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
concord-consortium · Jul 20, 2024 · b39f8a9 · b39f8a9
1 parent 128ece4
commit b39f8a9
Show file tree

Hide file tree

Showing 5 changed files with 62 additions and 32 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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 (
-      <IndexCell caseId={__id__} index={index} collapsedCases={collapsedCases} />
+      <IndexCell caseId={__id__} index={index} collapsedCases={collapsedCaseCount} onClick={handleClick} />
     )
   }, [caseMetadata, data])
   const indexColumn = useRef<TColumn | undefined>()
@@ -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<HTMLButtonElement | null>(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 (
       <div className={classes}>
         <DragIndicator />
       </div>
     )
-  } else {
+  }
+
+  // collapsed row
+  const casesStr = t(collapsedCases === 1 ? "DG.DataContext.singleCaseName" : "DG.DataContext.pluralCaseName")
+  if (collapsedCases) {
     return (
-      <Menu isLazy>
-        <MenuButton ref={setMenuButtonRef} className={classes} data-testid="codap-index-content-button"
-                    onKeyDown={handleKeyDown} aria-describedby="sr-index-menu-instructions">
-          {collapsedCases != null
-            ? `${collapsedCases} ${casesStr}`
-            : index != null ? `${index + 1}` : ""}
-        </MenuButton>
-        <VisuallyHidden id="sr-index-menu-instructions">
-          Press Enter to open the menu.
-        </VisuallyHidden>
-        {portalElt && createPortal(<IndexMenuList caseId={caseId} index={index}/>, portalElt)}
-      </Menu>
+      <div className={classes} data-testid="codap-index-content-button" onClick={onClick}>
+        {`${collapsedCases} ${casesStr}`}
+      </div>
     )
   }
+
+  // normal index row
+  return (
+    <Menu isLazy>
+      <MenuButton ref={setMenuButtonRef} className={classes} data-testid="codap-index-content-button"
+                  onKeyDown={handleKeyDown} aria-describedby="sr-index-menu-instructions">
+        {index != null ? `${index + 1}` : ""}
+      </MenuButton>
+      <VisuallyHidden id="sr-index-menu-instructions">
+        Press Enter to open the menu.
+      </VisuallyHidden>
+      {portalElt && createPortal(<IndexMenuList caseId={caseId} index={index}/>, portalElt)}
+    </Menu>
+  )
 }