feat(Table): add indeterminate checkbox state support for select-all header#12411
feat(Table): add indeterminate checkbox state support for select-all header#12411rhamilto wants to merge 1 commit intopatternfly:mainfrom
Conversation
WalkthroughAdds an optional ChangesIndeterminate Checkbox State for Table Headers
Sequence DiagramsequenceDiagram
participant App as App State
participant Th as Th Component
participant Selectable as Selectable Decorator
participant SelectColumn as SelectColumn
participant CheckboxDOM as Checkbox Component
App->>Th: pass select props (isSelected, isIndeterminate, onSelect)
Th->>Selectable: extraParams={ isIndeterminate, ... }
Selectable->>SelectColumn: isIndeterminate={rowId === -1 ? isIndeterminate : undefined}
SelectColumn->>SelectColumn: construct checkboxProps (includes isChecked: null if isIndeterminate)
SelectColumn->>CheckboxDOM: render Checkbox with checkboxProps
CheckboxDOM-->>App: renders checked/indeterminate/unchecked
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related issues
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Preview: https://pf-react-pr-12411.surge.sh A11y report: https://pf-react-pr-12411-a11y.surge.sh |
…header
Add support for indeterminate state to the Table component's select-all
checkbox, following PatternFly's bulk selection design guidelines.
The select-all checkbox now supports three states:
- Unchecked: no items selected
- Indeterminate: some items selected (shows dash/minus icon)
- Checked: all items selected
## Changes
- Add `isIndeterminate?: boolean` property to `ThSelectType` interface
- Add `isIndeterminate?: boolean` to `IColumn` extraParams type
- Update `Th` component to pass `isIndeterminate` to the selectable decorator
- Update `selectable` decorator to pass indeterminate state to `SelectColumn`
- Update `SelectColumn` to use PatternFly Checkbox's native indeterminate support (isChecked: null)
- Update `TableSelectable` example to demonstrate indeterminate state
- Add new `TableSelectableIndeterminate` example showcasing the feature
## Implementation
The indeterminate state is implemented using the PatternFly Checkbox component's
native support by setting `isChecked: null` when `isIndeterminate` is true.
## Usage
\`\`\`typescript
const areSomeReposSelected = selectedRepoNames.length > 0 && selectedRepoNames.length < selectableRepos.length;
<Th
select={{
onSelect: (_event, isSelecting) => selectAllRepos(isSelecting),
isSelected: areAllReposSelected,
isIndeterminate: areSomeReposSelected
}}
aria-label="Row select"
/>
\`\`\`
Fixes patternfly#12404
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
3ab7def to
1b9740e
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/react-table/src/components/Table/SelectColumn.tsx (1)
47-61: 💤 Low value
checkboxPropsduplicates all four fields fromcommonProps— consider eliminating the separate object.PatternFly's
CheckboxsupportsisChecked?: boolean | null, wherenullmeans the checkbox will be indeterminate (partially checked). The override is correct, butcheckboxPropsreplicates all four fields (...props,id,ref,onChange) already present incommonProps. You can eliminate the duplication by building the conditional override directly at the call-site:♻️ Proposed refactor
- // PatternFly Checkbox supports indeterminate via isChecked: null - const checkboxProps = { - ...props, - id, - ref: inputRef, - onChange: handleChange, - ...(isIndeterminate && { isChecked: null }) - }; - const commonProps = { ...props, id, ref: inputRef, onChange: handleChange };- <Checkbox {...checkboxProps} /> + <Checkbox {...commonProps} {...(isIndeterminate && { isChecked: null })} />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react-table/src/components/Table/SelectColumn.tsx` around lines 47 - 61, Remove the duplicated object by using commonProps as the base and applying the indeterminate override only where Checkbox is rendered: delete the separate checkboxProps declaration and, at the Checkbox render site, spread commonProps and conditionally include isChecked: null when isIndeterminate is true (preserving ...props, id, ref: inputRef, onChange: handleChange from commonProps). Keep the identifier names isIndeterminate, commonProps and checkbox behavior consistent with PatternFly's Checkbox which accepts isChecked?: boolean | null.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/react-table/src/components/Table/SelectColumn.tsx`:
- Around line 47-61: Remove the duplicated object by using commonProps as the
base and applying the indeterminate override only where Checkbox is rendered:
delete the separate checkboxProps declaration and, at the Checkbox render site,
spread commonProps and conditionally include isChecked: null when
isIndeterminate is true (preserving ...props, id, ref: inputRef, onChange:
handleChange from commonProps). Keep the identifier names isIndeterminate,
commonProps and checkbox behavior consistent with PatternFly's Checkbox which
accepts isChecked?: boolean | null.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 407847d8-4ecd-4bbc-80dc-1d367e75dd63
📒 Files selected for processing (8)
packages/react-table/src/components/Table/SelectColumn.tsxpackages/react-table/src/components/Table/TableTypes.tsxpackages/react-table/src/components/Table/Th.tsxpackages/react-table/src/components/Table/base/types.tsxpackages/react-table/src/components/Table/examples/Table.mdpackages/react-table/src/components/Table/examples/TableSelectable.tsxpackages/react-table/src/components/Table/examples/TableSelectableIndeterminate.tsxpackages/react-table/src/components/Table/utils/decorators/selectable.tsx
✅ Files skipped from review due to trivial changes (2)
- packages/react-table/src/components/Table/examples/TableSelectable.tsx
- packages/react-table/src/components/Table/examples/Table.md
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/react-table/src/components/Table/utils/decorators/selectable.tsx
- packages/react-table/src/components/Table/Th.tsx
|
cc: @nicolethoen, this is needed to complete openshift/console#16203 |
Summary
Adds support for indeterminate state to the Table component's select-all checkbox in the header, following PatternFly's bulk selection design guidelines.
The select-all checkbox now properly supports three states:
Changes
isIndeterminate?: booleanproperty toThSelectTypeinterfaceselectabledecorator to pass indeterminate state toSelectColumnSelectColumncomponent to set indeterminate property via ref usinguseEffectThcomponent to passisIndeterminateto the selectable decoratorTableSelectableexample to demonstrate indeterminate stateTableSelectableIndeterminateexample showcasing the featureTechnical implementation
The indeterminate state on HTML checkboxes must be set via the DOM property (not as a React prop), so this is handled using a ref and
useEffectin theSelectColumncomponent.Usage example
Related issues
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation