From 4c23736813f93fc7586e03ecad5e42f6b08c480d Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Thu, 19 Oct 2023 10:17:41 -0500 Subject: [PATCH] fix: Fix select all with disabled rows. --- src/components/Table/GridTable.test.tsx | 58 +++++++++++++++++-- .../Table/components/SelectToggle.tsx | 4 +- src/components/Table/utils/RowState.ts | 13 ++--- src/inputs/Checkbox.test.tsx | 5 ++ src/inputs/CheckboxBase.tsx | 2 +- 5 files changed, 65 insertions(+), 17 deletions(-) diff --git a/src/components/Table/GridTable.test.tsx b/src/components/Table/GridTable.test.tsx index f4e1cc85e..f4bea1c19 100644 --- a/src/components/Table/GridTable.test.tsx +++ b/src/components/Table/GridTable.test.tsx @@ -72,10 +72,6 @@ const nestedColumns: GridColumn[] = [ }, selectColumn({ totals: emptyCell, - header: (data, { row }) => ({ content: }), - child: (data, { row }) => ({ content: }), }), { totals: emptyCell, @@ -1647,6 +1643,57 @@ describe("GridTable", () => { expect(api.current!.getSelectedRowIds()).toEqual([]); }); + it("can select all with disabled children", async () => { + // Given a parent with two children + const rows: GridDataRow[] = [ + simpleHeader, + { + ...{ kind: "parent", id: "p1", data: { name: "parent 1" } }, + children: [ + { kind: "child", id: "p1c1", data: { name: "child p1c1" } }, + // And the 2nd one is disabled + { kind: "child", id: "p1c2", data: { name: "child p1c2" }, selectable: false }, + ], + }, + ]; + + const api: MutableRefObject | undefined> = { current: undefined }; + function Test() { + const _api = useGridTableApi(); + api.current = _api; + return api={_api} columns={nestedColumns} rows={rows} />; + } + const r = await render(); + + // And all three rows are initially rendered + expect(cell(r, 1, 2)).toHaveTextContent("parent 1"); + expect(cell(r, 2, 2)).toHaveTextContent("child p1c1"); + expect(cell(r, 3, 2)).toHaveTextContent("child p1c2"); + + // When we select all + click(cell(r, 0, 1).children[0] as any); + // Then the 'All', Parent, and 1st child rows are shown as selected + expect(cellAnd(r, 0, 1, "select")).toBeChecked(); + expect(cellAnd(r, 1, 1, "select")).toBeChecked(); + expect(cellAnd(r, 2, 1, "select")).toBeChecked(); + // But the 2nd child is not + expect(cellAnd(r, 3, 1, "select")).toBeDisabled(); + // And the api can fetch them + expect(api.current!.getSelectedRowIds()).toEqual(["p1", "p1c1"]); + expect(api.current!.getSelectedRowIds("child")).toEqual(["p1c1"]); + + // And when we unselect all + click(cell(r, 0, 1).children[0] as any); + // Then all rows are shown as unselected + expect(cellAnd(r, 0, 1, "select")).not.toBeChecked(); + expect(cellAnd(r, 1, 1, "select")).not.toBeChecked(); + expect(cellAnd(r, 2, 1, "select")).not.toBeChecked(); + expect(cellAnd(r, 3, 1, "select")).not.toBeChecked(); + expect(cellAnd(r, 3, 1, "select")).toBeDisabled(); + // And they are no longer selected + expect(api.current!.getSelectedRowIds()).toEqual([]); + }); + it("fires props.onSelect", async () => { // Given a parent with a child const actions: string[] = []; @@ -3566,7 +3613,8 @@ describe("GridTable", () => { // When deselecting the visible row click(r.select_2); - // Then the parent row is now unchecked (respecting only the matched rows), but the header is still partially selected as it takes into consideration the kept selected rows + // Then the parent row is now unchecked (respecting only the matched rows), but the header is still partially + // selected as it takes into consideration the kept selected rows expect(cellAnd(r, 0, 1, "select")).toHaveAttribute("data-indeterminate", "true"); expect(cellAnd(r, 2, 1, "select")).not.toBeChecked(); diff --git a/src/components/Table/components/SelectToggle.tsx b/src/components/Table/components/SelectToggle.tsx index cc578e512..fcc0ceee4 100644 --- a/src/components/Table/components/SelectToggle.tsx +++ b/src/components/Table/components/SelectToggle.tsx @@ -19,9 +19,7 @@ export function SelectToggle({ id, disabled }: SelectToggleProps) { checkboxOnly={true} disabled={disabled} label="Select" - onChange={(selected) => { - tableState.selectRow(id, selected); - }} + onChange={(selected) => tableState.selectRow(id, selected)} selected={selected} /> ); diff --git a/src/components/Table/utils/RowState.ts b/src/components/Table/utils/RowState.ts index be439b782..701469b62 100644 --- a/src/components/Table/utils/RowState.ts +++ b/src/components/Table/utils/RowState.ts @@ -44,11 +44,7 @@ export class RowState { removed: false | "soft" | "hard" = false; private isCalculatingDirectMatch = false; - constructor( - private states: RowStates, - public parent: RowState | undefined, - row: GridDataRow, - ) { + constructor(private states: RowStates, public parent: RowState | undefined, row: GridDataRow) { this.row = row; this.selected = !!row.initSelected; this.collapsed = states.storage.wasCollapsed(row.id) ?? !!row.initCollapsed; @@ -127,7 +123,7 @@ export class RowState { // client-side filter changes, a child reappears, and we need to transition to partial-ness. if (this.isParent) { // Use visibleChildren b/c if filters are hiding some of our children, we still want to show fully selected - const children = this.visibleChildren; + const children = this.visibleChildren.filter((c) => c.row.selectable !== false); const allChecked = children.every((child) => child.selectedState === "checked"); const allUnchecked = children.every((child) => child.selectedState === "unchecked"); return children.length === 0 ? "unchecked" : allChecked ? "checked" : allUnchecked ? "unchecked" : "partial"; @@ -143,7 +139,7 @@ export class RowState { */ get selectedStateForHeader(): SelectedState { if (this.children) { - const children = this.visibleChildren; + const children = this.visibleChildren.filter((c) => c.row.selectable !== false || c.isParent); const allChecked = children.every((child) => child.selectedStateForHeader === "checked"); const allUnchecked = children.every((child) => child.selectedStateForHeader === "unchecked"); // For the header purposes, if this is a selectable-row (i.e. not inferSelectedState) make sure @@ -302,7 +298,8 @@ export class RowState { * they want the row to be selectable. */ private get isParent(): boolean { - return !!this.children && this.children.length > 0 && this.inferSelectedState; + // Check for KEPT_GROUP b/c it has `this.children = []` but we synthesize its children in `visibleChildren` + return !!this.children && (this.children.length > 0 || this.row.id === KEPT_GROUP) && this.inferSelectedState; } private get isPinned(): boolean { diff --git a/src/inputs/Checkbox.test.tsx b/src/inputs/Checkbox.test.tsx index a86b8b0bd..f881d25f0 100644 --- a/src/inputs/Checkbox.test.tsx +++ b/src/inputs/Checkbox.test.tsx @@ -13,6 +13,11 @@ describe("Checkbox", () => { expect(r.customId).toBeTruthy(); }); + it("sets disabled", async () => { + const r = await render(); + expect(r.test).toBeDisabled(); + }); + it("treats indeterminate as false", async () => { // Given an indeterminate checkbox const onChange = jest.fn(); diff --git a/src/inputs/CheckboxBase.tsx b/src/inputs/CheckboxBase.tsx index 39f7a7222..1e5d729ab 100644 --- a/src/inputs/CheckboxBase.tsx +++ b/src/inputs/CheckboxBase.tsx @@ -63,7 +63,7 @@ export function CheckboxBase(props: CheckboxBaseProps) { - + {!checkboxOnly && ( // Use a mtPx(-2) to better align the label with the checkbox. // Not using align-items: center as the checkbox would align with all content below, where we really want it to stay only aligned with the label