fix: AppTable remounting children on table state updates#6331
Merged
Conversation
App wrappers (AppTable, AppCell, AppHeader, AppFooter) were memoized on the unstable `table` reference, causing them to be recreated every render. React then remounted their entire subtrees on every state change, destroying controlled input focus. Keep wrappers stable (created once) and read the current table from a ref updated each render. Fixes children remounting while preserving latest table state access.
2 tasks
Abyanzhafran
changed the title
Member
|
Awesome, I'll try this out soon |
|
View your CI Pipeline Execution ↗ for commit 9b6daf1
☁️ Nx Cloud last updated this comment at |
Contributor
📝 WalkthroughWalkthroughBoth ChangesCore stable App wrapper ref pattern
Row selection feature for all example frameworks
Sequence Diagram(s)sequenceDiagram
participant SelectCell
participant Context as useCellContext<br/>useTableContext
participant Subscribe
participant Atoms as table.atoms<br/>.rowSelection
participant Checkbox as IndeterminateCheckbox
SelectCell->>Context: read current cell/row
SelectCell->>Context: read table reference
SelectCell->>Subscribe: wrap render with subscription
Subscribe->>Atoms: observe row selection state
Atoms-->>Subscribe: emit updates on change
Subscribe->>Checkbox: render with row.getIsSelected()
Checkbox->>SelectCell: user toggles → row.getToggleSelectedHandler()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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.
Inline comments:
In `@examples/preact/composable-tables/src/components/cell-components.tsx`:
- Around line 27-32: The IndeterminateCheckbox component used for row selection
in the cell-components.tsx file is missing an accessible name, making it
inaccessible to assistive technologies. Add an aria-label attribute to the
IndeterminateCheckbox component that provides a descriptive name for the
checkbox, such as indicating that it selects the current row. This will ensure
screen readers announce the control's purpose to users.
In `@examples/preact/composable-tables/src/main.tsx`:
- Around line 39-55: The IndeterminateCheckbox component in the select-column
header (within personColumnHelper.display with id 'select') lacks an accessible
name for assistive technology. Add an aria-label prop to the
IndeterminateCheckbox to provide a descriptive label such as "Select all rows".
Apply the same fix to the analogous Products select-column header around lines
271-287 to ensure both select-all checkboxes are properly labeled for
accessibility.
In `@examples/react/composable-tables/src/components/cell-components.tsx`:
- Around line 27-32: The IndeterminateCheckbox component used for row selection
is missing an accessible name, making it inaccessible to assistive technologies.
Add an aria-label prop to the IndeterminateCheckbox component that clearly
describes its purpose, such as "Select row" or a more descriptive label that
indicates which row is being selected. This will ensure screen readers announce
the checkbox with meaningful context to users.
In `@examples/react/composable-tables/src/main.tsx`:
- Around line 40-56: The IndeterminateCheckbox components used in both the
select-column headers (in the personColumnHelper.display for Users and the
corresponding Products column helper) are missing accessible names, making them
unnamed to assistive technology. Add an aria-label prop to each
IndeterminateCheckbox instance that clearly describes its purpose, such as
"Select all rows" or "Toggle all rows selection", to ensure the checkboxes are
properly labeled for screen readers and other assistive technologies.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6be50bba-3190-496f-84ee-5b14299ba124
📒 Files selected for processing (10)
examples/preact/composable-tables/src/components/cell-components.tsxexamples/preact/composable-tables/src/components/indeterminate-checkbox.tsxexamples/preact/composable-tables/src/hooks/table.tsexamples/preact/composable-tables/src/index.cssexamples/preact/composable-tables/src/main.tsxexamples/react/composable-tables/src/components/cell-components.tsxexamples/react/composable-tables/src/components/indeterminate-checkbox.tsxexamples/react/composable-tables/src/hooks/table.tsexamples/react/composable-tables/src/index.cssexamples/react/composable-tables/src/main.tsx
✅ Files skipped from review due to trivial changes (2)
- examples/react/composable-tables/src/index.css
- examples/react/composable-tables/src/components/indeterminate-checkbox.tsx
Comment on lines
+27
to
+32
| <IndeterminateCheckbox | ||
| checked={row.getIsSelected()} | ||
| disabled={!row.getCanSelect()} | ||
| indeterminate={row.getIsSomeSelected()} | ||
| onChange={row.getToggleSelectedHandler()} | ||
| /> |
Contributor
There was a problem hiding this comment.
Add an accessible name to row selection checkboxes.
The row checkbox is rendered without a label/aria-label, so assistive technologies get an unnamed control.
Suggested patch
<IndeterminateCheckbox
+ aria-label={`Select row ${row.id}`}
checked={row.getIsSelected()}
disabled={!row.getCanSelect()}
indeterminate={row.getIsSomeSelected()}
onChange={row.getToggleSelectedHandler()}
/>📝 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
| <IndeterminateCheckbox | |
| checked={row.getIsSelected()} | |
| disabled={!row.getCanSelect()} | |
| indeterminate={row.getIsSomeSelected()} | |
| onChange={row.getToggleSelectedHandler()} | |
| /> | |
| <IndeterminateCheckbox | |
| aria-label={`Select row ${row.id}`} | |
| checked={row.getIsSelected()} | |
| disabled={!row.getCanSelect()} | |
| indeterminate={row.getIsSomeSelected()} | |
| onChange={row.getToggleSelectedHandler()} | |
| /> |
🤖 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 `@examples/preact/composable-tables/src/components/cell-components.tsx` around
lines 27 - 32, The IndeterminateCheckbox component used for row selection in the
cell-components.tsx file is missing an accessible name, making it inaccessible
to assistive technologies. Add an aria-label attribute to the
IndeterminateCheckbox component that provides a descriptive name for the
checkbox, such as indicating that it selects the current row. This will ensure
screen readers announce the control's purpose to users.
Comment on lines
+39
to
+55
| personColumnHelper.display({ | ||
| id: 'select', | ||
| header: ({ table }) => ( | ||
| // Subscribe keeps the select-all checkbox in sync with selection state (see SelectCell) | ||
| <Subscribe source={table.atoms.rowSelection}> | ||
| {() => ( | ||
| <IndeterminateCheckbox | ||
| checked={table.getIsAllRowsSelected()} | ||
| indeterminate={table.getIsSomeRowsSelected()} | ||
| onChange={table.getToggleAllRowsSelectedHandler()} | ||
| /> | ||
| )} | ||
| </Subscribe> | ||
| ), | ||
| // Cell uses the pre-bound SelectCell component via AppCell | ||
| cell: ({ cell }) => <cell.SelectCell />, | ||
| }), |
Contributor
There was a problem hiding this comment.
Add an accessible name to the select-all checkbox.
Both header-level selection checkboxes are missing a label/aria-label, which leaves the control unnamed in assistive tech.
Suggested patch
<IndeterminateCheckbox
+ aria-label="Select all rows"
checked={table.getIsAllRowsSelected()}
indeterminate={table.getIsSomeRowsSelected()}
onChange={table.getToggleAllRowsSelectedHandler()}
/>Apply the same addition to both Users and Products select-column headers.
Also applies to: 271-287
🤖 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 `@examples/preact/composable-tables/src/main.tsx` around lines 39 - 55, The
IndeterminateCheckbox component in the select-column header (within
personColumnHelper.display with id 'select') lacks an accessible name for
assistive technology. Add an aria-label prop to the IndeterminateCheckbox to
provide a descriptive label such as "Select all rows". Apply the same fix to the
analogous Products select-column header around lines 271-287 to ensure both
select-all checkboxes are properly labeled for accessibility.
Comment on lines
+27
to
+32
| <IndeterminateCheckbox | ||
| checked={row.getIsSelected()} | ||
| disabled={!row.getCanSelect()} | ||
| indeterminate={row.getIsSomeSelected()} | ||
| onChange={row.getToggleSelectedHandler()} | ||
| /> |
Contributor
There was a problem hiding this comment.
Add an accessible name to row selection checkboxes.
The row checkbox is rendered without a label/aria-label, so assistive technologies get an unnamed control.
Suggested patch
<IndeterminateCheckbox
+ aria-label={`Select row ${row.id}`}
checked={row.getIsSelected()}
disabled={!row.getCanSelect()}
indeterminate={row.getIsSomeSelected()}
onChange={row.getToggleSelectedHandler()}
/>📝 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
| <IndeterminateCheckbox | |
| checked={row.getIsSelected()} | |
| disabled={!row.getCanSelect()} | |
| indeterminate={row.getIsSomeSelected()} | |
| onChange={row.getToggleSelectedHandler()} | |
| /> | |
| <IndeterminateCheckbox | |
| aria-label={`Select row ${row.id}`} | |
| checked={row.getIsSelected()} | |
| disabled={!row.getCanSelect()} | |
| indeterminate={row.getIsSomeSelected()} | |
| onChange={row.getToggleSelectedHandler()} | |
| /> |
🤖 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 `@examples/react/composable-tables/src/components/cell-components.tsx` around
lines 27 - 32, The IndeterminateCheckbox component used for row selection is
missing an accessible name, making it inaccessible to assistive technologies.
Add an aria-label prop to the IndeterminateCheckbox component that clearly
describes its purpose, such as "Select row" or a more descriptive label that
indicates which row is being selected. This will ensure screen readers announce
the checkbox with meaningful context to users.
Comment on lines
+40
to
+56
| personColumnHelper.display({ | ||
| id: 'select', | ||
| header: ({ table }) => ( | ||
| // Subscribe works around React Compiler memoization (see SelectCell) | ||
| <Subscribe source={table.atoms.rowSelection}> | ||
| {() => ( | ||
| <IndeterminateCheckbox | ||
| checked={table.getIsAllRowsSelected()} | ||
| indeterminate={table.getIsSomeRowsSelected()} | ||
| onChange={table.getToggleAllRowsSelectedHandler()} | ||
| /> | ||
| )} | ||
| </Subscribe> | ||
| ), | ||
| // Cell uses the pre-bound SelectCell component via AppCell | ||
| cell: ({ cell }) => <cell.SelectCell />, | ||
| }), |
Contributor
There was a problem hiding this comment.
Add an accessible name to the select-all checkbox.
Both header-level selection checkboxes are missing a label/aria-label, which leaves the control unnamed in assistive tech.
Suggested patch
<IndeterminateCheckbox
+ aria-label="Select all rows"
checked={table.getIsAllRowsSelected()}
indeterminate={table.getIsSomeRowsSelected()}
onChange={table.getToggleAllRowsSelectedHandler()}
/>Apply the same addition to both Users and Products select-column headers.
Also applies to: 268-284
🤖 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 `@examples/react/composable-tables/src/main.tsx` around lines 40 - 56, The
IndeterminateCheckbox components used in both the select-column headers (in the
personColumnHelper.display for Users and the corresponding Products column
helper) are missing accessible names, making them unnamed to assistive
technology. Add an aria-label prop to each IndeterminateCheckbox instance that
clearly describes its purpose, such as "Select all rows" or "Toggle all rows
selection", to ensure the checkboxes are properly labeled for screen readers and
other assistive technologies.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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.
Inline comments:
In `@examples/preact/composable-tables/src/main.tsx`:
- Around line 41-45: The AppTable selector is missing the rowSelection state
tracking, which causes the header checkbox components (IndeterminateCheckbox) to
not re-render when row selection changes, leaving them in a stale state. Locate
both AppTable selector definitions (one around pagination/sorting/columnFilters
configuration) and add rowSelection to the selector function by including
rowSelection: state.rowSelection alongside the existing pagination, sorting, and
columnFilters properties. This ensures the component re-executes when row
selection state changes, keeping the getIsAllRowsSelected() and
getIsSomeRowsSelected() checks current.
In `@examples/solid/composable-tables/src/App.tsx`:
- Around line 202-224: The footer rendering logic for the 'select' column
currently falls into the generic footer branch which renders FooterColumnId and
filtered indicators inappropriately for this checkbox utility column. Modify the
conditional logic to skip footer rendering for the 'select' column by updating
the condition that currently checks for columnId === 'actions' to also exclude
the 'select' column (either by adding columnId === 'select' to the existing null
return condition or by creating a separate check). Apply this same fix to all
occurrences where this footer rendering pattern appears, including the section
mentioned at lines 426-448.
In `@examples/solid/composable-tables/src/components/cell-components.tsx`:
- Around line 21-26: The IndeterminateCheckbox component used for row selection
is missing an aria-label attribute, which prevents assistive technologies from
understanding its purpose. Add an aria-label prop to the IndeterminateCheckbox
component that clearly describes its function, such as indicating that it
controls row selection. The label should be accessible and descriptive so screen
readers can properly announce the checkbox's purpose to users.
In `@examples/svelte/composable-tables/src/components/SelectCell.svelte`:
- Around line 25-31: The checkbox input element in the SelectCell.svelte
component lacks an aria-label attribute, making it inaccessible to assistive
technologies. Add an aria-label attribute to the input element (the one with
type="checkbox" that has the checked, disabled, and onchange attributes) with a
descriptive label that clearly indicates it selects the current row for screen
readers and other assistive devices.
In `@examples/svelte/composable-tables/src/components/SelectHeader.svelte`:
- Around line 24-31: Add an aria-label attribute to the checkbox input element
in the SelectHeader component to provide an accessible name for screen readers.
The checkbox currently lacks a label that assistive technologies can announce to
users. Include an aria-label with a descriptive text such as "Select all rows"
to clearly communicate the purpose of this control to users relying on assistive
technologies.
In `@examples/vue/composable-tables/src/components/header-components.ts`:
- Around line 12-16: The IndeterminateCheckbox component in SelectAllHeader is
missing an accessible name property. Add an aria-label prop to the
IndeterminateCheckbox component that clearly describes its purpose, such as
selecting or deselecting all rows in the table. This will ensure screen readers
and assistive technologies can properly announce the checkbox's function to
users.
- Around line 15-16: The SelectAllHeader checkbox click event is bubbling up to
the parent table header element and triggering the sort handler. Modify the
onChange handler of the IndeterminateCheckbox component to prevent event
propagation by wrapping the table.getToggleAllRowsSelectedHandler() call in a
handler function that calls event.stopPropagation() before invoking the toggle
handler, similar to the ColumnFilter pattern used elsewhere in this file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 14acac1c-2dc0-4f95-9e6d-5e7d2a447689
📒 Files selected for processing (32)
examples/angular/composable-tables/src/app/components/cell-components.tsexamples/angular/composable-tables/src/app/components/header-components.tsexamples/angular/composable-tables/src/app/components/products-table/products-table.htmlexamples/angular/composable-tables/src/app/components/products-table/products-table.tsexamples/angular/composable-tables/src/app/components/users-table/users-table.htmlexamples/angular/composable-tables/src/app/components/users-table/users-table.tsexamples/angular/composable-tables/src/app/table.tsexamples/angular/composable-tables/src/styles.cssexamples/lit/composable-tables/src/components/products-table.tsexamples/lit/composable-tables/src/components/users-table.tsexamples/lit/composable-tables/src/hooks/table.tsexamples/lit/composable-tables/src/index.cssexamples/preact/composable-tables/src/components/cell-components.tsxexamples/preact/composable-tables/src/main.tsxexamples/solid/composable-tables/src/App.tsxexamples/solid/composable-tables/src/components/cell-components.tsxexamples/solid/composable-tables/src/components/indeterminate-checkbox.tsxexamples/solid/composable-tables/src/hooks/table.tsexamples/solid/composable-tables/src/index.cssexamples/svelte/composable-tables/src/components/ProductsTable.svelteexamples/svelte/composable-tables/src/components/SelectCell.svelteexamples/svelte/composable-tables/src/components/SelectHeader.svelteexamples/svelte/composable-tables/src/components/UsersTable.svelteexamples/svelte/composable-tables/src/hooks/table.tsexamples/svelte/composable-tables/src/index.cssexamples/vue/composable-tables/src/components/IndeterminateCheckbox.vueexamples/vue/composable-tables/src/components/ProductsTable.vueexamples/vue/composable-tables/src/components/UsersTable.vueexamples/vue/composable-tables/src/components/cell-components.tsexamples/vue/composable-tables/src/components/header-components.tsexamples/vue/composable-tables/src/hooks/table.tsexamples/vue/composable-tables/src/index.css
✅ Files skipped from review due to trivial changes (4)
- examples/vue/composable-tables/src/index.css
- examples/solid/composable-tables/src/components/indeterminate-checkbox.tsx
- examples/vue/composable-tables/src/components/IndeterminateCheckbox.vue
- examples/angular/composable-tables/src/app/components/products-table/products-table.html
Comment on lines
+41
to
+45
| <IndeterminateCheckbox | ||
| checked={table.getIsAllRowsSelected()} | ||
| indeterminate={table.getIsSomeRowsSelected()} | ||
| onChange={table.getToggleAllRowsSelectedHandler()} | ||
| /> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate preact table hook implementation =="
fd -i 'createTableHook.tsx'
echo
echo "== Inspect AppTable/AppHeader subscription behavior in preact hook =="
rg -n -C4 'AppTable|AppHeader|selector|Subscribe|rowSelection|atoms\.rowSelection' packages/preact-table/src/createTableHook.tsx
echo
echo "== Inspect current file selectors and checkbox wiring =="
rg -n -C3 'AppTable|selector=\(|getIsAllRowsSelected|getIsSomeRowsSelected|getToggleAllRowsSelectedHandler|IndeterminateCheckbox' examples/preact/composable-tables/src/main.tsxRepository: TanStack/table
Length of output: 25838
🏁 Script executed:
# Get the full AppTable selector and header rendering structure
sed -n '104,244p' examples/preact/composable-tables/src/main.tsxRepository: TanStack/table
Length of output: 6014
🏁 Script executed:
# Also check the ProductTable section
sed -n '321,458p' examples/preact/composable-tables/src/main.tsxRepository: TanStack/table
Length of output: 5743
🏁 Script executed:
# Check if table.Subscribe is available and how it might be used for rowSelection
rg -n 'getIsAllRowsSelected|getIsSomeRowsSelected|rowSelection.*Subscribe' examples/preact/composable-tables/src/main.tsxRepository: TanStack/table
Length of output: 298
🏁 Script executed:
# Verify that AppHeader without selector doesn't have any Subscribe logic for rowSelection
sed -n '945,1023p' packages/preact-table/src/createTableHook.tsxRepository: TanStack/table
Length of output: 2565
Add rowSelection to the AppTable selector to keep the header checkboxes reactive.
The checkboxes at lines 41-45 and 268-272 read table.getIsAllRowsSelected() and table.getIsSomeRowsSelected() directly within the column header function. However, the AppTable selector (lines 107-109 and 323-325) does not include rowSelection, only pagination, sorting, and columnFilters. This means when row selection changes, AppTable won't re-render and the header function won't re-execute, leaving the checkbox state stale until another subscribed state updates. Add rowSelection to both AppTable selectors:
selector={(state) => ({
pagination: state.pagination,
sorting: state.sorting,
columnFilters: state.columnFilters,
rowSelection: state.rowSelection,
})}🤖 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 `@examples/preact/composable-tables/src/main.tsx` around lines 41 - 45, The
AppTable selector is missing the rowSelection state tracking, which causes the
header checkbox components (IndeterminateCheckbox) to not re-render when row
selection changes, leaving them in a stale state. Locate both AppTable selector
definitions (one around pagination/sorting/columnFilters configuration) and add
rowSelection to the selector function by including rowSelection:
state.rowSelection alongside the existing pagination, sorting, and columnFilters
properties. This ensures the component re-executes when row selection state
changes, keeping the getIsAllRowsSelected() and getIsSomeRowsSelected() checks
current.
Comment on lines
+202
to
+224
| {columnId === 'age' || | ||
| columnId === 'visits' || | ||
| columnId === 'progress' ? ( | ||
| <> | ||
| <footer.FooterSum /> | ||
| {hasFilter() && ( | ||
| <span class="filtered-indicator"> | ||
| {' '} | ||
| (filtered) | ||
| </span> | ||
| )} | ||
| </> | ||
| ) : columnId === 'actions' ? null : ( | ||
| <> | ||
| <footer.FooterColumnId /> | ||
| {hasFilter() && ( | ||
| <span class="filtered-indicator"> | ||
| {' '} | ||
| ✓ | ||
| </span> | ||
| )} | ||
| </> | ||
| )} |
Contributor
There was a problem hiding this comment.
Skip footer rendering for the select display column.
The new selection column currently falls into generic footer branches, so it can render FooterColumnId/filtered indicators for a checkbox utility column (e.g., “select”). This is a small UX correctness bug.
Suggested patch
@@
- ) : columnId === 'actions' ? null : (
+ ) : columnId === 'actions' ||
+ columnId === 'select' ? null : (
@@
- {columnId === 'price' ||
+ {columnId === 'select' ? null : columnId === 'price' ||
columnId === 'stock' ||
columnId === 'rating' ? (Also applies to: 426-448
🤖 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 `@examples/solid/composable-tables/src/App.tsx` around lines 202 - 224, The
footer rendering logic for the 'select' column currently falls into the generic
footer branch which renders FooterColumnId and filtered indicators
inappropriately for this checkbox utility column. Modify the conditional logic
to skip footer rendering for the 'select' column by updating the condition that
currently checks for columnId === 'actions' to also exclude the 'select' column
(either by adding columnId === 'select' to the existing null return condition or
by creating a separate check). Apply this same fix to all occurrences where this
footer rendering pattern appears, including the section mentioned at lines
426-448.
Comment on lines
+21
to
+26
| <IndeterminateCheckbox | ||
| checked={row.getIsSelected()} | ||
| disabled={!row.getCanSelect()} | ||
| indeterminate={row.getIsSomeSelected()} | ||
| onChange={row.getToggleSelectedHandler()} | ||
| /> |
Contributor
There was a problem hiding this comment.
Add an accessible name to the row selection checkbox.
The checkbox should include an aria-label so assistive technologies can identify its purpose.
Suggested patch
<IndeterminateCheckbox
+ aria-label={`Select row ${row.id}`}
checked={row.getIsSelected()}
disabled={!row.getCanSelect()}
indeterminate={row.getIsSomeSelected()}
onChange={row.getToggleSelectedHandler()}
/>📝 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
| <IndeterminateCheckbox | |
| checked={row.getIsSelected()} | |
| disabled={!row.getCanSelect()} | |
| indeterminate={row.getIsSomeSelected()} | |
| onChange={row.getToggleSelectedHandler()} | |
| /> | |
| <IndeterminateCheckbox | |
| aria-label={`Select row ${row.id}`} | |
| checked={row.getIsSelected()} | |
| disabled={!row.getCanSelect()} | |
| indeterminate={row.getIsSomeSelected()} | |
| onChange={row.getToggleSelectedHandler()} | |
| /> |
🤖 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 `@examples/solid/composable-tables/src/components/cell-components.tsx` around
lines 21 - 26, The IndeterminateCheckbox component used for row selection is
missing an aria-label attribute, which prevents assistive technologies from
understanding its purpose. Add an aria-label prop to the IndeterminateCheckbox
component that clearly describes its function, such as indicating that it
controls row selection. The label should be accessible and descriptive so screen
readers can properly announce the checkbox's purpose to users.
Comment on lines
+25
to
+31
| <input | ||
| type="checkbox" | ||
| checked={row.getIsSelected()} | ||
| disabled={!row.getCanSelect()} | ||
| use:setIndeterminate={!row.getIsSelected() && row.getIsSomeSelected()} | ||
| onchange={row.getToggleSelectedHandler()} | ||
| /> |
Contributor
There was a problem hiding this comment.
Add an accessible name to the row checkbox.
This checkbox is currently unnamed for assistive tech. Add an aria-label describing that it selects the current row.
Suggested patch
<input
type="checkbox"
+ aria-label={`Select row ${row.id}`}
checked={row.getIsSelected()}
disabled={!row.getCanSelect()}
use:setIndeterminate={!row.getIsSelected() && row.getIsSomeSelected()}
onchange={row.getToggleSelectedHandler()}
/>📝 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
| <input | |
| type="checkbox" | |
| checked={row.getIsSelected()} | |
| disabled={!row.getCanSelect()} | |
| use:setIndeterminate={!row.getIsSelected() && row.getIsSomeSelected()} | |
| onchange={row.getToggleSelectedHandler()} | |
| /> | |
| <input | |
| type="checkbox" | |
| aria-label={`Select row ${row.id}`} | |
| checked={row.getIsSelected()} | |
| disabled={!row.getCanSelect()} | |
| use:setIndeterminate={!row.getIsSelected() && row.getIsSomeSelected()} | |
| onchange={row.getToggleSelectedHandler()} | |
| /> |
🤖 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 `@examples/svelte/composable-tables/src/components/SelectCell.svelte` around
lines 25 - 31, The checkbox input element in the SelectCell.svelte component
lacks an aria-label attribute, making it inaccessible to assistive technologies.
Add an aria-label attribute to the input element (the one with type="checkbox"
that has the checked, disabled, and onchange attributes) with a descriptive
label that clearly indicates it selects the current row for screen readers and
other assistive devices.
Comment on lines
+24
to
+31
| <input | ||
| type="checkbox" | ||
| checked={table.getIsAllRowsSelected()} | ||
| use:setIndeterminate={!table.getIsAllRowsSelected() && | ||
| table.getIsSomeRowsSelected()} | ||
| onchange={table.getToggleAllRowsSelectedHandler()} | ||
| onclick={(e) => e.stopPropagation()} | ||
| /> |
Contributor
There was a problem hiding this comment.
Add an accessible name to the select-all checkbox.
The header checkbox should expose a clear accessible name (e.g., “Select all rows”).
Suggested patch
<input
type="checkbox"
+ aria-label="Select all rows"
checked={table.getIsAllRowsSelected()}
use:setIndeterminate={!table.getIsAllRowsSelected() &&
table.getIsSomeRowsSelected()}
onchange={table.getToggleAllRowsSelectedHandler()}
onclick={(e) => e.stopPropagation()}
/>📝 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
| <input | |
| type="checkbox" | |
| checked={table.getIsAllRowsSelected()} | |
| use:setIndeterminate={!table.getIsAllRowsSelected() && | |
| table.getIsSomeRowsSelected()} | |
| onchange={table.getToggleAllRowsSelectedHandler()} | |
| onclick={(e) => e.stopPropagation()} | |
| /> | |
| <input | |
| type="checkbox" | |
| aria-label="Select all rows" | |
| checked={table.getIsAllRowsSelected()} | |
| use:setIndeterminate={!table.getIsAllRowsSelected() && | |
| table.getIsSomeRowsSelected()} | |
| onchange={table.getToggleAllRowsSelectedHandler()} | |
| onclick={(e) => e.stopPropagation()} | |
| /> |
🤖 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 `@examples/svelte/composable-tables/src/components/SelectHeader.svelte` around
lines 24 - 31, Add an aria-label attribute to the checkbox input element in the
SelectHeader component to provide an accessible name for screen readers. The
checkbox currently lacks a label that assistive technologies can announce to
users. Include an aria-label with a descriptive text such as "Select all rows"
to clearly communicate the purpose of this control to users relying on assistive
technologies.
Comment on lines
+12
to
+16
| return h(IndeterminateCheckbox, { | ||
| checked: table.getIsAllRowsSelected(), | ||
| indeterminate: table.getIsSomeRowsSelected(), | ||
| onChange: table.getToggleAllRowsSelectedHandler(), | ||
| }) |
Contributor
There was a problem hiding this comment.
Pass an accessible name to the select-all checkbox component.
SelectAllHeader should provide an explicit accessible name to the rendered checkbox.
Suggested patch
return h(IndeterminateCheckbox, {
+ 'aria-label': 'Select all rows',
checked: table.getIsAllRowsSelected(),
indeterminate: table.getIsSomeRowsSelected(),
onChange: table.getToggleAllRowsSelectedHandler(),
})📝 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
| return h(IndeterminateCheckbox, { | |
| checked: table.getIsAllRowsSelected(), | |
| indeterminate: table.getIsSomeRowsSelected(), | |
| onChange: table.getToggleAllRowsSelectedHandler(), | |
| }) | |
| return h(IndeterminateCheckbox, { | |
| 'aria-label': 'Select all rows', | |
| checked: table.getIsAllRowsSelected(), | |
| indeterminate: table.getIsSomeRowsSelected(), | |
| onChange: table.getToggleAllRowsSelectedHandler(), | |
| }) |
🤖 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 `@examples/vue/composable-tables/src/components/header-components.ts` around
lines 12 - 16, The IndeterminateCheckbox component in SelectAllHeader is missing
an accessible name property. Add an aria-label prop to the IndeterminateCheckbox
component that clearly describes its purpose, such as selecting or deselecting
all rows in the table. This will ensure screen readers and assistive
technologies can properly announce the checkbox's function to users.
Comment on lines
+15
to
+16
| onChange: table.getToggleAllRowsSelectedHandler(), | ||
| }) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Inspect Vue table header click wiring:"
fd -i 'ProductsTable.vue' examples/vue/composable-tables/src/components --exec rg -n -C2 '`@click`|onClick|getToggleSortingHandler|toggleSorting'
fd -i 'UsersTable.vue' examples/vue/composable-tables/src/components --exec rg -n -C2 '`@click`|onClick|getToggleSortingHandler|toggleSorting'
echo
echo "Inspect SelectAllHeader implementation:"
rg -n -C3 'SelectAllHeader|stopPropagation|getToggleAllRowsSelectedHandler' examples/vue/composable-tables/src/components/header-components.tsRepository: TanStack/table
Length of output: 1476
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Locate and examine IndeterminateCheckbox.vue ==="
fd -i 'IndeterminateCheckbox.vue' examples/vue/composable-tables/src --exec cat -n {}
echo
echo "=== Check how SelectAllHeader is used in table headers ==="
rg -n 'SelectAllHeader' examples/vue/composable-tables/src/components --type vue --type ts -B2 -A2
echo
echo "=== Look at full header-components.ts file ==="
wc -l examples/vue/composable-tables/src/components/header-components.tsRepository: TanStack/table
Length of output: 970
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for SelectAllHeader usage ==="
rg -n 'SelectAllHeader' examples/vue/composable-tables/src/components -g '*.vue' -g '*.ts' -B3 -A3
echo
echo "=== Full header-components.ts file ==="
cat -n examples/vue/composable-tables/src/components/header-components.tsRepository: TanStack/table
Length of output: 5431
Add event propagation prevention to SelectAllHeader checkbox.
Clicking the checkbox will bubble the click event to the parent <th> element and trigger the sort handler in ProductsTable.vue and UsersTable.vue. The IndeterminateCheckbox needs to prevent this propagation. Either wrap it with event.stopPropagation() (following the ColumnFilter pattern in this file), or add the handler directly to the checkbox input:
Example fix
return h(IndeterminateCheckbox, {
checked: table.getIsAllRowsSelected(),
indeterminate: table.getIsSomeRowsSelected(),
onChange: (event: Event) => {
event.stopPropagation()
table.getToggleAllRowsSelectedHandler()(event)
},
})🤖 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 `@examples/vue/composable-tables/src/components/header-components.ts` around
lines 15 - 16, The SelectAllHeader checkbox click event is bubbling up to the
parent table header element and triggering the sort handler. Modify the onChange
handler of the IndeterminateCheckbox component to prevent event propagation by
wrapping the table.getToggleAllRowsSelectedHandler() call in a handler function
that calls event.stopPropagation() before invoking the toggle handler, similar
to the ColumnFilter pattern used elsewhere in this file.
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
2 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.
App wrappers (AppTable, AppCell, AppHeader, AppFooter) were memoized on the unstable
tablereference, causing them to be recreated every render. React then remounted their entire subtrees on every state change, destroying controlled input focus.Keep wrappers stable (created once) and read the current table from a ref updated each render. Fixes children remounting while preserving latest table state access.
Fixes : #6331
🎯 Changes
RCA: AppTable remounting children on every table state update
Problem
table.AppTable(andAppCell,AppHeader,AppFooter) created a new function reference on every render. React uses function identity to decide if a component should re-render or remount. A new function = unmount + remount the entire subtree, destroying controlled input focus.Root Cause
useTablereturns a freshtablereference every render (by design, for React Compiler compatibility)useAppTablefeeds a new options object literal touseTableevery render[table]as a dependency[table]changes every render, the wrappers were recreated every renderSolution
Decouple component identity (must be stable) from the table reference the component reads (must be current) using a ref pattern:
useRef✅ Checklist
pnpm test:pr.Summary by CodeRabbit
Bug Fixes
New Features