From f35ba55521c2596d17f92125ebc052e8a5bf7484 Mon Sep 17 00:00:00 2001
From: Ben Pusey <141063311+Ben-Pusey-Bentley@users.noreply.github.com>
Date: Wed, 12 Jun 2024 09:54:17 -0400
Subject: [PATCH] Fixes regression with `Table` `selectSubRows` prop (#2098)

Updated selection cell checkbox logic to be based on the `selectSubRows` property, which it gets from the `CellProps` object.
---
 .changeset/old-cougars-check.md               |  5 +++
 .../core/Table/columns/selectionColumn.tsx    |  4 +--
 testing/e2e/app/routes/Table/route.tsx        |  2 ++
 testing/e2e/app/routes/Table/spec.ts          | 32 +++++++++++++++++++
 4 files changed, 41 insertions(+), 2 deletions(-)
 create mode 100644 .changeset/old-cougars-check.md

diff --git a/.changeset/old-cougars-check.md b/.changeset/old-cougars-check.md
new file mode 100644
index 00000000000..0a76c04f6d4
--- /dev/null
+++ b/.changeset/old-cougars-check.md
@@ -0,0 +1,5 @@
+---
+'@itwin/itwinui-react': patch
+---
+
+Fixed regression with `Table` component where `selectSubRows` property being set to `false` would cause parent rows to become unable to be unchecked.
diff --git a/packages/itwinui-react/src/core/Table/columns/selectionColumn.tsx b/packages/itwinui-react/src/core/Table/columns/selectionColumn.tsx
index ba6fa700650..a8bbcf7b0f0 100644
--- a/packages/itwinui-react/src/core/Table/columns/selectionColumn.tsx
+++ b/packages/itwinui-react/src/core/Table/columns/selectionColumn.tsx
@@ -74,7 +74,7 @@ export const SelectionColumn = <T extends Record<string, unknown>>(
         />
       );
     },
-    Cell: ({ row }: CellProps<T>) => (
+    Cell: ({ row, selectSubRows = true }: CellProps<T>) => (
       <Checkbox
         {...row.getToggleRowSelectedProps()}
         style={{}} // Removes pointer cursor as we have it in CSS and it is also showing pointer when disabled
@@ -82,7 +82,7 @@ export const SelectionColumn = <T extends Record<string, unknown>>(
         disabled={isDisabled?.(row.original)}
         onClick={(e) => e.stopPropagation()} // Prevents triggering on row click
         onChange={() => {
-          if (row.subRows.length > 0) {
+          if (row.subRows.length > 0 && selectSubRows) {
             //This code ignores any sub-rows that are not currently available(i.e disabled or filtered out).
             //If all available sub-rows are selected, then it deselects them all, otherwise it selects them all.
             row.toggleRowSelected(
diff --git a/testing/e2e/app/routes/Table/route.tsx b/testing/e2e/app/routes/Table/route.tsx
index 448c7b95e6c..99d151af53e 100644
--- a/testing/e2e/app/routes/Table/route.tsx
+++ b/testing/e2e/app/routes/Table/route.tsx
@@ -12,6 +12,7 @@ export default function Resizing() {
   const isSelectable = searchParams.get('isSelectable') === 'true';
   const subRows = searchParams.get('subRows') === 'true';
   const filter = searchParams.get('filter') === 'true';
+  const selectSubRows = !(searchParams.get('selectSubRows') === 'false');
 
   const data = subRows
     ? [
@@ -127,6 +128,7 @@ export default function Resizing() {
         isSelectable={isSelectable}
         isSortable
         columnResizeMode={columnResizeMode as 'fit' | 'expand' | undefined}
+        selectSubRows={selectSubRows}
       />
     </>
   );
diff --git a/testing/e2e/app/routes/Table/spec.ts b/testing/e2e/app/routes/Table/spec.ts
index 125b0c292cc..25cd98615e9 100644
--- a/testing/e2e/app/routes/Table/spec.ts
+++ b/testing/e2e/app/routes/Table/spec.ts
@@ -330,6 +330,38 @@ test.describe('Table row selection', () => {
     await expect(row22Checkbox).toBeChecked();
   });
 
+  test('parent checkbox should have normal checkbox behavior when selectSubRows is false', async ({
+    page,
+  }) => {
+    await page.goto(
+      '/Table?isSelectable=true&subRows=true&selectSubRows=false',
+    );
+
+    const row2 = page
+      .getByRole('row')
+      .filter({ has: page.getByRole('cell').getByText('2', { exact: true }) });
+    const row2SubRowExpander = row2.getByLabel('Toggle sub row');
+    await row2SubRowExpander.click();
+
+    const row21 = page
+      .getByRole('row')
+      .filter({
+        has: page.getByRole('cell').getByText('2.1', { exact: true }),
+      });
+    const row2Checkbox = row2.getByRole('checkbox');
+    const row21Checkbox = row21.getByRole('checkbox');
+
+    //Check the row
+    await row2Checkbox.click();
+    await expect(row2Checkbox).toBeChecked();
+    await expect(row21Checkbox).not.toBeChecked();
+
+    //Uncheck the row
+    await row2Checkbox.click();
+    await expect(row2Checkbox).not.toBeChecked();
+    await expect(row21Checkbox).not.toBeChecked();
+  });
+
   //#region Helpers for row selection tests
   const filter = async (page: Page) => {
     const filterButton = page.getByLabel('Filter');