Skip to content

Commit a03fbd1

Browse files
rhamiltoclaude
andcommitted
refactor(Table): address PR feedback for indeterminate checkbox
- Refactor SelectColumn to reduce duplication by building checkboxProps from commonProps - Update example description to match PatternFly tone and emphasize accessibility - Add comprehensive unit tests for isIndeterminate prop in Th component Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 6308f98 commit a03fbd1

3 files changed

Lines changed: 29 additions & 10 deletions

File tree

packages/react-table/src/components/Table/SelectColumn.tsx

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -44,22 +44,18 @@ export const SelectColumn: React.FunctionComponent<SelectColumnProps> = ({
4444
onSelect && onSelect(event);
4545
};
4646

47-
// PatternFly Checkbox supports indeterminate via isChecked: null
48-
const checkboxProps = {
49-
...props,
50-
id,
51-
ref: inputRef,
52-
onChange: handleChange,
53-
...(isIndeterminate && { isChecked: null })
54-
};
55-
5647
const commonProps = {
5748
...props,
5849
id,
5950
ref: inputRef,
6051
onChange: handleChange
6152
};
6253

54+
const checkboxProps = {
55+
...commonProps,
56+
...(isIndeterminate && { isChecked: null })
57+
};
58+
6359
const content = (
6460
<Fragment>
6561
{selectVariant === RowSelectVariant.checkbox ? (

packages/react-table/src/components/Table/__tests__/Th.test.tsx

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,3 +54,26 @@ test('Additional content renders after children when additionalContent is passed
5454
expect(thChildren.item(0)?.textContent).toEqual('Test');
5555
expect(thChildren.item(1)?.textContent).toEqual('Extra');
5656
});
57+
58+
test('Renders checkbox without indeterminate state by default', () => {
59+
render(<Th select={{ onSelect: jest.fn(), isSelected: false }} aria-label="Select all" />);
60+
61+
const checkbox = screen.getByRole('checkbox') as HTMLInputElement;
62+
expect(checkbox).not.toBeChecked();
63+
expect(checkbox.indeterminate).toBe(false);
64+
});
65+
66+
test('Renders checkbox with indeterminate state when isIndeterminate is true', () => {
67+
render(<Th select={{ onSelect: jest.fn(), isSelected: false, isIndeterminate: true }} aria-label="Select all" />);
68+
69+
const checkbox = screen.getByRole('checkbox') as HTMLInputElement;
70+
expect(checkbox.indeterminate).toBe(true);
71+
});
72+
73+
test('Renders checked checkbox when isSelected is true and isIndeterminate is false', () => {
74+
render(<Th select={{ onSelect: jest.fn(), isSelected: true, isIndeterminate: false }} aria-label="Select all" />);
75+
76+
const checkbox = screen.getByRole('checkbox') as HTMLInputElement;
77+
expect(checkbox).toBeChecked();
78+
expect(checkbox.indeterminate).toBe(false);
79+
});

packages/react-table/src/components/Table/examples/Table.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ checking checkboxes will check intermediate rows' checkboxes.
160160

161161
### Selectable with indeterminate state
162162

163-
This example demonstrates the indeterminate state support for the select-all checkbox. When some (but not all) rows are selected, the header checkbox displays a dash/minus icon to indicate partial selection.
163+
To indicate partial selection, use `isIndeterminate` on the header's `select` prop. When some (but not all) selectable rows are selected, the header checkbox will convey this information to assistive technologies and also display a dash instead of a checkmark.
164164

165165
```ts file="TableSelectableIndeterminate.tsx"
166166

0 commit comments

Comments
 (0)