From b39f8a92fed82c81f5605e0d9a16674e54f8e588 Mon Sep 17 00:00:00 2001 From: Kirk Swenson Date: Sat, 20 Jul 2024 10:41:22 -0700 Subject: [PATCH] fix: collapsed case rows shouldn't show index menu [PT-187986400] (#1364) * fix: collapsed case rows don't show index menu * chore: improve test coverage --- v3/cypress/e2e/hierarchical-table.spec.ts | 5 +- v3/cypress/fixtures/hierarchical.json | 3 - v3/cypress/support/elements/table-tile.ts | 12 ++-- v3/src/components/case-table/case-table.scss | 3 +- .../case-table/use-index-column.tsx | 71 +++++++++++++------ 5 files changed, 62 insertions(+), 32 deletions(-) diff --git a/v3/cypress/e2e/hierarchical-table.spec.ts b/v3/cypress/e2e/hierarchical-table.spec.ts index 581a46dbd2..fde6c1019e 100644 --- a/v3/cypress/e2e/hierarchical-table.spec.ts +++ b/v3/cypress/e2e/hierarchical-table.spec.ts @@ -11,7 +11,7 @@ context("hierarchical collections", () => { }) hierarchical.tests.forEach((h) => { // FIXME: enable skipped tests - const itOrSkip = h.skip ? it.skip : it + const itOrSkip = h.skip ? it.skip : h.only ? it.only : it itOrSkip(`${h.testName}`, () => { const collections = h.collections collections.forEach((collection, index) => { @@ -34,6 +34,9 @@ context("hierarchical collections", () => { table.collapseAllGroups(collection.index+1) table.getNumOfRows(collection.index+1).should("contain", collection.cases+2) table.verifyCollapsedRows(collection.childCases, collection.index+1) + // clicking on collapsed row should select cases within it along with parent case + table.getIndexCellInRow(2, collection.index+1).click() + table.getSelectedRow(2, collection.index).should("exist") table.expandAllGroups(collection.index+1) table.getNumOfRows(collection.index+1).should("contain", collection.totalChildren+2) }) diff --git a/v3/cypress/fixtures/hierarchical.json b/v3/cypress/fixtures/hierarchical.json index 902d1ab69f..fc50051efd 100644 --- a/v3/cypress/fixtures/hierarchical.json +++ b/v3/cypress/fixtures/hierarchical.json @@ -38,7 +38,6 @@ }, { "testName": "Move multiple categorical attributes to a parent collection", - "skip": true, "collections": [ { "name": "Diets (5 cases)", @@ -61,7 +60,6 @@ }, { "testName": "Move numeric and categorical attributes to a parent collection", - "skip": true, "collections": [ { "name": "Habitats (20 cases)", @@ -84,7 +82,6 @@ }, { "testName": "Move categorical attributes to multilevel hierarchical collections", - "skip": true, "collections": [ { "name": "Diets (3 cases)", diff --git a/v3/cypress/support/elements/table-tile.ts b/v3/cypress/support/elements/table-tile.ts index e08364ed47..3f762248e1 100644 --- a/v3/cypress/support/elements/table-tile.ts +++ b/v3/cypress/support/elements/table-tile.ts @@ -42,13 +42,17 @@ export const TableTileElements = { // getColumnHeaderTooltip() { // return cy.get("[data-testid=case-table-attribute-tooltip]") // }, - getIndexRow(rowNum: number, collectionIndex = 1) { + getSelectedRow(rowNum: number, collectionIndex = 1) { + return this.getCollection(collectionIndex).find(`[data-testid=collection-table-grid] + [role=row][aria-rowindex="${rowNum}"][aria-selected="true"]`) + }, + getIndexCellInRow(rowNum: number, collectionIndex = 1) { return this.getCollection(collectionIndex).find(`[data-testid=collection-table-grid] [role=row][aria-rowindex="${rowNum}"] [data-testid=codap-index-content-button]`) }, openIndexMenuForRow(rowNum: number, collectionIndex = 1) { - this.getIndexRow(rowNum, collectionIndex).click("top") + this.getIndexCellInRow(rowNum, collectionIndex).click("top") }, getIndexMenu() { return cy.get("[data-testid=index-menu-list]") @@ -311,7 +315,7 @@ export const TableTileElements = { this.getCollapseAllGroupsButton(collectionIndex).click() }, getCollapsedIndex(rowIndex, collectionIndex = 1) { - return this.getIndexRow(rowIndex, collectionIndex) + return this.getIndexCellInRow(rowIndex, collectionIndex) }, getRowExpandCollapseButton(rowIndex, collectionIndex = 1) { return this.getCollection(collectionIndex).find(".spacer-mid-layer .expand-collapse-button img") @@ -325,7 +329,7 @@ export const TableTileElements = { }, verifyCollapsedRows(childCases, collectionIndex = 1) { for (let childCaseIndex = 0; childCaseIndex < childCases.length; childCaseIndex++) { - this.getIndexRow(childCaseIndex+2, collectionIndex).then(indexCell => { + this.getIndexCellInRow(childCaseIndex+2, collectionIndex).then(indexCell => { expect(childCases).to.include(indexCell.text()) }) } diff --git a/v3/src/components/case-table/case-table.scss b/v3/src/components/case-table/case-table.scss index 845175479c..e64d6f09f8 100644 --- a/v3/src/components/case-table/case-table.scss +++ b/v3/src/components/case-table/case-table.scss @@ -259,7 +259,8 @@ $table-body-font-size: 8pt; justify-content: center; &.collapsed { - text-align: left + margin-left: 4px; + justify-content: flex-start; } &.input-row { diff --git a/v3/src/components/case-table/use-index-column.tsx b/v3/src/components/case-table/use-index-column.tsx index ce24f4d759..3b009f1b6f 100644 --- a/v3/src/components/case-table/use-index-column.tsx +++ b/v3/src/components/case-table/use-index-column.tsx @@ -12,7 +12,7 @@ import { useDataSetContext } from "../../hooks/use-data-set-context" import { ICollectionModel } from "../../models/data/collection" import { IDataSet } from "../../models/data/data-set" import { symIndex, symParent } from "../../models/data/data-set-types" -import { getCollectionAttrs } from "../../models/data/data-set-utils" +import { getCollectionAttrs, selectCases, setSelectedCases } from "../../models/data/data-set-utils" import { ISharedCaseMetadata } from "../../models/shared/shared-case-metadata" import { t } from "../../utilities/translation/translate" @@ -44,12 +44,27 @@ export const useIndexColumn = () => { const RenderIndexCell = useCallback(({ row }: TRenderCellProps) => { const { __id__, [symIndex]: _index, [symParent]: parentId } = row const index = _index != null ? _index : data?.caseIndexFromID(__id__) - const collapsedCases = (data && parentId && caseMetadata?.isCollapsed(parentId)) - ? data.caseGroupMap.get(parentId)?.childCaseIds?.length ?? - data.caseGroupMap.get(parentId)?.childItemIds.length - : undefined + const collapsedCases = data && parentId && caseMetadata?.isCollapsed(parentId) + ? data.caseGroupMap.get(parentId)?.childCaseIds ?? [] + : [] + const collapsedCaseCount = collapsedCases.length + + function handleClick(e: React.MouseEvent) { + if (parentId && collapsedCaseCount) { + const wereSelected = collapsedCases.every(caseId => data?.isCaseSelected(caseId)) + const extend = e.metaKey || e.shiftKey + if (extend) { + selectCases([parentId], data, !wereSelected) + } + else if (!wereSelected) { + setSelectedCases([parentId], data) + } + e.stopPropagation() + } + } + return ( - + ) }, [caseMetadata, data]) const indexColumn = useRef() @@ -75,13 +90,13 @@ export const useIndexColumn = () => { return indexColumn.current } -interface ICellProps { +interface IIndexCellProps { caseId: string index?: number collapsedCases?: number - onClick?: (caseId: string, evt: React.MouseEvent) => void + onClick?: (evt: React.MouseEvent) => void } -export function IndexCell({ caseId, index, collapsedCases, onClick }: ICellProps) { +export function IndexCell({ caseId, index, collapsedCases, onClick }: IIndexCellProps) { const [menuButton, setMenuButton] = useState(null) const cellElt: HTMLDivElement | null = menuButton?.closest(".rdg-cell") ?? null // Find the parent CODAP component to display the index menu above the grid @@ -118,27 +133,37 @@ export function IndexCell({ caseId, index, collapsedCases, onClick }: ICellProps const isInputRow = caseId === kInputRowKey const classes = clsx("codap-index-content", { collapsed: collapsedCases != null, "input-row": isInputRow }) - const casesStr = t(collapsedCases === 1 ? "DG.DataContext.singleCaseName" : "DG.DataContext.pluralCaseName") + + // input row if (isInputRow) { return (
) - } else { + } + + // collapsed row + const casesStr = t(collapsedCases === 1 ? "DG.DataContext.singleCaseName" : "DG.DataContext.pluralCaseName") + if (collapsedCases) { return ( - - - {collapsedCases != null - ? `${collapsedCases} ${casesStr}` - : index != null ? `${index + 1}` : ""} - - - Press Enter to open the menu. - - {portalElt && createPortal(, portalElt)} - +
+ {`${collapsedCases} ${casesStr}`} +
) } + + // normal index row + return ( + + + {index != null ? `${index + 1}` : ""} + + + Press Enter to open the menu. + + {portalElt && createPortal(, portalElt)} + + ) }