Skip to content

Commit

Permalink
Fixes regression with Table selectSubRows prop (#2098)
Browse files Browse the repository at this point in the history
Updated selection cell checkbox logic to be based on the `selectSubRows` property, which it gets from the `CellProps` object.
  • Loading branch information
Ben-Pusey-Bentley authored Jun 12, 2024
1 parent 2e33e14 commit f35ba55
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 2 deletions.
5 changes: 5 additions & 0 deletions .changeset/old-cougars-check.md
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,15 @@ 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
title='' // Removes default title that comes from react-table
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(
Expand Down
2 changes: 2 additions & 0 deletions testing/e2e/app/routes/Table/route.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
? [
Expand Down Expand Up @@ -127,6 +128,7 @@ export default function Resizing() {
isSelectable={isSelectable}
isSortable
columnResizeMode={columnResizeMode as 'fit' | 'expand' | undefined}
selectSubRows={selectSubRows}
/>
</>
);
Expand Down
32 changes: 32 additions & 0 deletions testing/e2e/app/routes/Table/spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down

0 comments on commit f35ba55

Please sign in to comment.