feat(Table): add indeterminate checkbox state support for select-all header#12411
Merged
thatblindgeye merged 2 commits intopatternfly:mainfrom May 8, 2026
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR adds an optional ChangesIndeterminate Checkbox State Support Indeterminate header select DAG
Sequence Diagram(s)sequenceDiagram
participant User
participant Th
participant selectable
participant SelectColumn
participant Checkbox
User->>Th: user toggles selection
Th->>selectable: pass column.extraParams (isIndeterminate, isSelected, ...)
selectable->>SelectColumn: props (isIndeterminate only for header)
SelectColumn->>Checkbox: render checkboxProps (isChecked:null when indeterminate)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
Collaborator
|
Preview: https://pf-react-pr-12411.surge.sh A11y report: https://pf-react-pr-12411-a11y.surge.sh |
rhamilto
force-pushed
the
feat/table-indeterminate-checkbox
branch
from
3ab7def to
1b9740e
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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
Member
Author
|
cc: @nicolethoen, this is needed to complete openshift/console#16203 |
rhamilto
force-pushed
the
feat/table-indeterminate-checkbox
branch
from
1b9740e to
a99a368
Compare
…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)
- 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>
rhamilto
force-pushed
the
feat/table-indeterminate-checkbox
branch
from
a99a368 to
6308f98
Compare
Collaborator
|
This update looks good to me. Not sure why the "a" row in our examples has a disabled check but not a blocker for you. |
Member
Author
|
@nicolethoen, who else can review? |
thatblindgeye
requested changes
May 8, 2026
|
|
||
| ### Selectable with indeterminate state | ||
|
|
||
| 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. |
Contributor
There was a problem hiding this comment.
Just to match the tone we typically try to have for example descriptions:
Suggested change
| 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. | |
| 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. |
kmcfaul
approved these changes
May 8, 2026
Collaborator
wise-king-sullyman
left a comment
wise-king-sullyman
left a comment
There was a problem hiding this comment.
Thanks for this work! Couple small requests:
- 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>
Member
Author
|
@wise-king-sullyman, @thatblindgeye, thanks for the review. Comments addressed. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/react-table/src/components/Table/__tests__/Th.test.tsx (1)
66-71: ⚡ Quick winAdd explicit unchecked assertion in the indeterminate test.
Line 70 verifies mixed state, but adding
not.toBeChecked()would lock the intended header semantics more tightly.Proposed test tweak
test('Renders checkbox with indeterminate state when isIndeterminate is true', () => { render(<Th select={{ onSelect: jest.fn(), isSelected: false, isIndeterminate: true }} aria-label="Select all" />); const checkbox = screen.getByRole('checkbox') as HTMLInputElement; expect(checkbox.indeterminate).toBe(true); + expect(checkbox).not.toBeChecked(); });🤖 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/__tests__/Th.test.tsx` around lines 66 - 71, Update the test "Renders checkbox with indeterminate state when isIndeterminate is true" in Th.test.tsx to also assert the checkbox is not checked: after getting the checkbox (screen.getByRole('checkbox')) add an expectation using expect(checkbox).not.toBeChecked() so the Th component's header semantics (indeterminate but unchecked) are explicitly verified alongside checkbox.indeterminate for the select prop used in <Th select={{ ... isIndeterminate: true }} />.
🤖 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/__tests__/Th.test.tsx`:
- Around line 66-71: Update the test "Renders checkbox with indeterminate state
when isIndeterminate is true" in Th.test.tsx to also assert the checkbox is not
checked: after getting the checkbox (screen.getByRole('checkbox')) add an
expectation using expect(checkbox).not.toBeChecked() so the Th component's
header semantics (indeterminate but unchecked) are explicitly verified alongside
checkbox.indeterminate for the select prop used in <Th select={{ ...
isIndeterminate: true }} />.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b0902b39-4905-41d0-a250-54f914329065
📒 Files selected for processing (3)
packages/react-table/src/components/Table/SelectColumn.tsxpackages/react-table/src/components/Table/__tests__/Th.test.tsxpackages/react-table/src/components/Table/examples/Table.md
✅ Files skipped from review due to trivial changes (1)
- packages/react-table/src/components/Table/examples/Table.md
thatblindgeye
approved these changes
May 8, 2026
Collaborator
|
Your changes have been released in:
Thanks for your contribution! 🎉 |
rhamilto
added a commit
to rhamilto/console
that referenced
this pull request
May 8, 2026
…eckbox support Upgrade to @patternfly/react-table@6.5.0-prerelease.77 which includes native indeterminate checkbox support via PR patternfly/patternfly-react#12411. Replace custom DOM manipulation hook with inline useEffect that sets checkbox indeterminate state directly. This avoids React controlled/uncontrolled input warnings that occur when passing isIndeterminate as a prop. Changes: - Upgrade PatternFly packages to v6.5 prerelease with peer dependency resolutions - Delete useIndeterminateCheckbox custom hook - Add inline useEffect in ConsoleDataView for indeterminate state via DOM manipulation - Fix icon-utils to handle both IconDefinition and IconData formats from PF v6.5 - Add Boolean coercion for checkbox isSelected prop to prevent controlled warnings - Wrap getDataViewRows in useCallback for performance - Remove @ts-expect-error comments now that NotificationBadge types variant prop - Document known reselect dev warning from legacy connect() HOC in kinds.ts Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 toThSelectTypeinterfaceisIndeterminate?: booleantoIColumnextraParams typeThcomponent to passisIndeterminateto the selectable decoratorselectabledecorator to pass indeterminate state toSelectColumnSelectColumnto use PatternFly Checkbox's native indeterminate support (isChecked: null)TableSelectableIndeterminateexample showcasing the featureTechnical implementation
The indeterminate state is implemented using the PatternFly Checkbox component's native support by setting
isChecked: nullwhenisIndeterminateis true. This leverages PatternFly's built-in indeterminate checkbox handling.Usage example
Related issues
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit