fix: propagate dataview row selection state, add example - #869
fix: propagate dataview row selection state, add example#869rohanchkrabrty wants to merge 3 commits into
Conversation
Row selection changes never reached `useDataView()` consumers. The context value is memoized over deps that a selection change doesn't touch, and the TanStack table instance identity is stable, so `row.toggleSelected()` re-rendered the root but produced an identical context value. Checkboxes only caught up when something else invalidated the memo — a filter edit or a consumer re-render. Lift `rowSelection` into root state alongside `columnVisibility` so it is a real dependency of the context value, expose it on the context, and add an `onRowSelectionChange` prop for mirroring selection outside the tree. Also backfill `flatRows` on the filtered row model. DataView filters with `filterFromLeafRows` so groups survive when a child matches, and table-core's leaf-first implementation allocates that array but never pushes into it — leaving `table.getIsAllRowsSelected()`, `getIsSomeRowsSelected()` and `toggleAllRowsSelected()` dead whenever a filter or search was active. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Mirrors DataTable's pattern: an unmanaged `select` column (absent from `fields`, so it stays out of Display Properties and can't be filtered, sorted, or grouped) plus a FloatingActions bar reading selection from `useDataView()`. Notes cover what the docs example can't show inline: why `getRowId` matters (positional keys survive client sort/filter/search but not a data reorder or a grouping switch), why TanStack's `getToggleSelectedHandler` / `getToggleAllRowsSelectedHandler` don't fit a Base UI checkbox, and that grouping plus selection is still unsupported. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 44 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughDataView now controls TanStack row selection state, exposes it through Sequence Diagram(s)sequenceDiagram
participant CheckboxColumn
participant DataView
participant TanStackTable
participant SelectionBar
CheckboxColumn->>TanStackTable: Toggle row selection
TanStackTable->>DataView: Emit selection updater
DataView->>DataView: Resolve controlled rowSelection state
DataView->>SelectionBar: Expose selected rows through useDataView
SelectionBar->>DataView: Reset selection on dismiss
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@apps/www/src/content/docs/components/dataview/demo.ts`:
- Around line 343-350: Update the import list in the demo snippet to include the
DataViewListColumn type from `@raystack/apsara`, so the selectionColumn
declaration resolves correctly when the example is copied and type-checked.
In `@packages/raystack/components/data-view/data-view.tsx`:
- Around line 141-147: Update handleRowSelectionChange so the
setRowSelectionState updater only computes and returns the next selection state;
move consumer notification out of the updater by tracking the latest reported
selection and invoking onRowSelectionChange from an effect after the state
commit, while avoiding duplicate reports.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: cc082ee8-0417-48e4-b7b2-7068bd3811d9
📒 Files selected for processing (8)
apps/www/src/components/dataview-demo.tsxapps/www/src/components/demo/demo.tsxapps/www/src/content/docs/components/dataview/demo.tsapps/www/src/content/docs/components/dataview/index.mdxapps/www/src/content/docs/components/dataview/props.tspackages/raystack/components/data-view/data-view.tsxpackages/raystack/components/data-view/data-view.types.tsxpackages/raystack/components/data-view/utils/index.tsx
| code: `import { | ||
| Button, | ||
| Checkbox, | ||
| Chip, | ||
| DataView, | ||
| FloatingActions, | ||
| useDataView, | ||
| } from "@raystack/apsara"; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C2 'DataViewListColumn<Person>|DataViewListColumn' \
apps/www/src/content/docs/components/dataview/demo.ts \
packages/raystack/components/data-view/index.tsRepository: raystack/apsara
Length of output: 1092
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== demo snippet =="
sed -n '330,370p' apps/www/src/content/docs/components/dataview/demo.ts
echo
echo "== data-view index exports =="
sed -n '1,40p' packages/raystack/components/data-view/index.ts
echo
echo "== occurrences of DataViewListColumn import =="
rg -n 'Import\s*<\s*.*DataViewListColumn|dataViewListColumn|DataViewListColumn' apps/www/src/content docs packages --glob '!**/node_modules/**' | head -200Repository: raystack/apsara
Length of output: 4806
Import DataViewListColumn in the example.
selectionColumn is typed as DataViewListColumn<Person>, but that type is not imported from @raystack/apsara, so copied TSX fails type checking.
Proposed fix
import {
Button,
Checkbox,
Chip,
DataView,
+ type DataViewListColumn,
FloatingActions,
useDataView,
} from "`@raystack/apsara`";📝 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.
| code: `import { | |
| Button, | |
| Checkbox, | |
| Chip, | |
| DataView, | |
| FloatingActions, | |
| useDataView, | |
| } from "@raystack/apsara"; | |
| code: `import { | |
| Button, | |
| Checkbox, | |
| Chip, | |
| DataView, | |
| type DataViewListColumn, | |
| FloatingActions, | |
| useDataView, | |
| } from "`@raystack/apsara`"; |
🤖 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 `@apps/www/src/content/docs/components/dataview/demo.ts` around lines 343 -
350, Update the import list in the demo snippet to include the
DataViewListColumn type from `@raystack/apsara`, so the selectionColumn
declaration resolves correctly when the example is copied and type-checked.
| const handleRowSelectionChange = useCallback( | ||
| (value: Updater<RowSelectionState>) => { | ||
| setRowSelectionState(prev => { | ||
| const newValue = typeof value === 'function' ? value(prev) : value; | ||
| onRowSelectionChange?.(newValue); | ||
| return newValue; | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
rg -n -C3 'onRowSelectionChange|StrictMode|toggleSelected|setRowSelection' \
packages/raystack/components/data-view/__tests__/data-view.test.tsxRepository: raystack/apsara
Length of output: 153
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== locate files =="
fd -a 'data-view\.tsx$|data-view\.test\.tsx$' . | sed 's#^\./##' | head -50
echo "== outline target =="
ast-grep outline packages/raystack/components/data-view/data-view.tsx || true
echo "== relevant source lines =="
cat -n packages/raystack/components/data-view/data-view.tsx | sed -n '1,240p'
echo "== tests relevant mentions =="
python3 - <<'PY'
from pathlib import Path
p = Path('packages/raystack/components/data-view/__tests__/data-view.test.tsx')
if p.exists():
txt = p.read_text()
for term in ['onRowSelectionChange','StrictMode','toggleSelected','setRowSelection','selection']:
print(f'\\n-- {term} --')
try:
hits = list(p.rglob('*')) # only one file so rg would work; search text here
for i,line in enumerate(txt.splitlines(),1):
if term.lower() in line.lower():
print(f'{i}: {line}')
except Exception as e:
print('ERR', e)
else:
print('test file missing')
PYRepository: raystack/apsara
Length of output: 9993
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
import subprocess, tempfile, os, re
src = subprocess.check_output(['git','show','HEAD:packages/raystack/components/data-view/data-view.tsx'], text=True)
lines = src.splitlines()
selected = 89 if selected == 89 else None
# Show relevant later lines
for i,line in enumerate(lines[240:360], 241):
if i>330: break
print(f"{i}: {line}")
# Extract hook body and dependency usage patterns
for name in ['useCallback','useEffect','useRef','useState','useReactTable','rootProps']:
print(f"\n-- occurrences of {name} --")
for i,l in enumerate(lines,1):
if re.search(rf"\b{name}\b", l):
print(f"{i}: {l}")
PY
echo "== data-view test selection snippets =="
python3 - <<'PY'
from pathlib import Path
p = Path('packages/raystack/components/data-view/__tests__/data-view.test.tsx')
text = p.read_text()
for i,line in enumerate(text.splitlines(),1):
if i >= 1120 and i <= 1205:
print(f"{i}: {line}")
PYRepository: raystack/apsara
Length of output: 268
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== later DataViewRoot/source context =="
cat -n packages/raystack/components/data-view/data-view.tsx | sed -n '241,380p'
echo "== hook/context usages =="
rg -n -C2 'useCallback|useEffect|useRef|useState|useReactTable|onRowSelectionChange|onColumnVisibilityChange' packages/raystack/components/data-view/data-view.tsx packages/raystack/components/data-view/__tests__/data-view.test.tsx
echo "== selection-related test snippets =="
cat -n packages/raystack/components/data-view/__tests__/data-view.test.tsx | sed -n '1128,1205p'
echo "== package react versions from lock/package data =="
python3 - <<'PY'
from pathlib import Path
for p in [Path('package.json'), Path('pnpm-lock.yaml'), Path('yarn.lock')]:
if p.exists():
print(f"\n-- {p} --")
text = p.read_text(errors='ignore')
for needle in ['"react"', '"react-dom"', '`@types/react`']:
if needle in text:
for line in text.splitlines():
if needle in line:
print(line)
PYRepository: raystack/apsara
Length of output: 50371
Move onRowSelectionChange out of the state updater.
Calling a consumer callback inside the setRowSelectionState(prev => ...) function lets it run during renders or React 18/19 Strict Mode double-invocation paths, so consumers can receive duplicate or out-of-order selection updates. Track the last reported selection and report from an effect after the commit/update path.
Proposed fix
const [rowSelection, setRowSelectionState] = useState<RowSelectionState>({});
+const lastReportedRowSelection = useRef(rowSelection);
const handleRowSelectionChange = useCallback(
(value: Updater<RowSelectionState>) => {
setRowSelectionState(prev => {
const newValue = typeof value === 'function' ? value(prev) : value;
- onRowSelectionChange?.(newValue);
return newValue;
});
},
- [onRowSelectionChange]
+ []
);
+
+useEffect(() => {
+ if (lastReportedRowSelection.current === rowSelection) return;
+ lastReportedRowSelection.current = rowSelection;
+ onRowSelectionChange?.(rowSelection);
+}, [rowSelection, onRowSelectionChange]);📝 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.
| const handleRowSelectionChange = useCallback( | |
| (value: Updater<RowSelectionState>) => { | |
| setRowSelectionState(prev => { | |
| const newValue = typeof value === 'function' ? value(prev) : value; | |
| onRowSelectionChange?.(newValue); | |
| return newValue; | |
| }); | |
| const handleRowSelectionChange = useCallback( | |
| (value: Updater<RowSelectionState>) => { | |
| setRowSelectionState(prev => { | |
| const newValue = typeof value === 'function' ? value(prev) : value; | |
| return newValue; | |
| }); | |
| }, | |
| [] | |
| ); | |
| useEffect(() => { | |
| if (lastReportedRowSelection.current === rowSelection) return; | |
| lastReportedRowSelection.current = rowSelection; | |
| onRowSelectionChange?.(rowSelection); | |
| }, [rowSelection, onRowSelectionChange]); |
🤖 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/raystack/components/data-view/data-view.tsx` around lines 141 - 147,
Update handleRowSelectionChange so the setRowSelectionState updater only
computes and returns the next selection state; move consumer notification out of
the updater by tracking the latest reported selection and invoking
onRowSelectionChange from an effect after the state commit, while avoiding
duplicate reports.
Group rows are synthesized by `groupData` and carry no consumer id, so `getRowId` returned undefined for them and TanStack fell back to a positional id — which collided with data row ids. With ids `1,2,3` and grouping on, row ids came out as `0=GROUP 1=Ada 2=Grace 1=GROUP 3=Susan`, so clicking Ada resolved to the Design group row and cascaded selection to Susan. React also warned about duplicate keys. Buckets are now keyed `__group__:<group_key>`, leaves delegate to `getRowId`, and without one we reproduce TanStack's positional default (`'0'`, and `'<parent>.<index>'` for sub-rows) since supplying a resolver replaces it. Group rows are also unselectable, so `rowSelection` holds one key per data row and select-all doesn't flag the bands. Grouping and selection now compose: the docs example drops `hideGrouping`, counts off `getSelectedRowModel().flatRows` (`.rows` only walks selected top-level rows, which is empty when grouping pushes data a level down), and guards `indeterminate` with `!getIsAllRowsSelected()`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 participant
Summary
useDataView()consumers: the context value is memoized over deps a selection change doesn't touch and the TanStack table instance identity is stable, sorow.toggleSelected()re-rendered the root but produced an identical context value — checkboxes only caught up when a filter edit or consumer re-render invalidated the memo.rowSelectionis now root state alongsidecolumnVisibility.flatRowson the filtered row model. DataView filters withfilterFromLeafRowsso groups survive when a child matches, and table-core's leaf-first implementation allocates that array but never fills it, leavinggetIsAllRowsSelected(),getIsSomeRowsSelected()andtoggleAllRowsSelected()dead whenever a filter or search was active.onRowSelectionChangeprop on the root for mirroring selection outside the tree, and exposerowSelection/setRowSelectionon the context.selectcolumn (absent fromfields, so it stays out of Display Properties and can't be filtered, sorted, or grouped) plus aFloatingActionsbar reading selection fromuseDataView().getRowIdactually matters (positional keys survive client sort/filter/search but not a data reorder or a grouping switch), why TanStack'sgetToggleSelectedHandler/getToggleAllRowsSelectedHandlerdon't fit a Base UI checkbox, and that grouping plus selection is still unsupported because group header rows share the row-id space.