Fix table.getIsSomeRowsSelected when filtered out rows are selected #6080
+4
−5
Conversation
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
…e when filtered out rows are selected
WalkthroughRefactors table.getIsSomeRowsSelected() to base its indeterminate-state calculation on selected rows within the current filtered (visible) rows instead of the total selection across all rows. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Table
participant FilteredRows as Filtered Row Model
participant HeaderCheckbox as Header Checkbox
User->>Table: Apply filters / select rows
Table->>FilteredRows: getFilteredRowModel().flatRows
Table->>Table: selectedVisible = visibleRows.filter(row.getIsSelected())
alt Indeterminate condition
Table->>HeaderCheckbox: some(selectedVisible) && !all(selectedVisible)
note right of HeaderCheckbox: Checkbox state = indeterminate
else None selected
Table->>HeaderCheckbox: 0 selected
note right of HeaderCheckbox: Checkbox unchecked
else All selected
Table->>HeaderCheckbox: all visible selected
note right of HeaderCheckbox: Checkbox checked
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNone found. Possibly related issues
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/table-core/src/features/RowSelection.ts(1 hunks)
Comment on lines
+434
to
439
| const visibleRows = table.getFilteredRowModel().flatRows | ||
| const selectedVisibleRows = visibleRows.filter(row => row.getIsSelected()) | ||
| return ( | ||
| totalSelected > 0 && | ||
| totalSelected < table.getFilteredRowModel().flatRows.length | ||
| selectedVisibleRows.length > 0 && | ||
| selectedVisibleRows.length < visibleRows.length | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Good fix; also exclude non-selectable rows and account for partial (subrow) selection to avoid new inconsistencies.
This change correctly scopes the calculation to the filtered (visible) set, fixing #6079. However:
- It counts against all visible rows, including those that cannot be selected. This can incorrectly set the header to indeterminate when all selectable rows are selected but some visible rows are not selectable.
- It ignores rows that are “partially selected” via subrows (getIsSomeSelected), which page-level logic already handles.
Align this with getIsSomePageRowsSelected by considering only selectable rows and including partial selection. Suggested diff:
- const visibleRows = table.getFilteredRowModel().flatRows
- const selectedVisibleRows = visibleRows.filter(row => row.getIsSelected())
- return (
- selectedVisibleRows.length > 0 &&
- selectedVisibleRows.length < visibleRows.length
- )
+ const visibleSelectableRows = table
+ .getFilteredRowModel()
+ .flatRows.filter(row => row.getCanSelect())
+ if (!visibleSelectableRows.length) return false
+ if (table.getIsAllRowsSelected()) return false
+ return visibleSelectableRows.some(
+ row => row.getIsSelected() || row.getIsSomeSelected()
+ )This keeps behavior consistent with page-level helpers and prevents regressions when non-selectable rows are present in the filtered view.
I can add unit tests covering:
- Selected rows filtered out vs. visible subset.
- Presence of non-selectable rows alongside selectable ones.
- Partial (subrow) selection affecting the indeterminate state.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const visibleRows = table.getFilteredRowModel().flatRows | |
| const selectedVisibleRows = visibleRows.filter(row => row.getIsSelected()) | |
| return ( | |
| totalSelected > 0 && | |
| totalSelected < table.getFilteredRowModel().flatRows.length | |
| selectedVisibleRows.length > 0 && | |
| selectedVisibleRows.length < visibleRows.length | |
| ) | |
| const visibleSelectableRows = table | |
| .getFilteredRowModel() | |
| .flatRows.filter(row => row.getCanSelect()) | |
| if (!visibleSelectableRows.length) return false | |
| if (table.getIsAllRowsSelected()) return false | |
| return visibleSelectableRows.some( | |
| row => row.getIsSelected() || row.getIsSomeSelected() | |
| ) |
🤖 Prompt for AI Agents
In packages/table-core/src/features/RowSelection.ts around lines 434 to 439, the
current visible-row indeterminate logic counts all visible rows and ignores
partial/subrow selection, which can mark the header indeterminate when only
non-selectable rows remain or miss when subrows are partially selected; update
the calculation to first filter visibleRows to only those with
row.getCanSelect(), then compute selectedVisibleRows as rows where
row.getIsSelected() OR row.getIsSomeSelected(), and finally return true when
selectedVisibleRows.length > 0 and selectedVisibleRows.length <
selectableVisibleRows.length so the indeterminate state matches
getIsSomePageRowsSelected and excludes non-selectable rows.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This pull request is for fixing an issue where table.getIsSomeRowsSelected occasionally returns an incorrect value when filtered out rows are selected.
Fixes #6079
Summary by CodeRabbit
Bug Fixes
Chores