-
Notifications
You must be signed in to change notification settings - Fork 13
fix: propagate dataview row selection state, add example #869
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3,8 +3,8 @@ | |||||||||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||||||||||
| getCoreRowModel, | ||||||||||||||||||||||||||||||||||||||||||||||
| getExpandedRowModel, | ||||||||||||||||||||||||||||||||||||||||||||||
| getFilteredRowModel, | ||||||||||||||||||||||||||||||||||||||||||||||
| getSortedRowModel, | ||||||||||||||||||||||||||||||||||||||||||||||
| RowSelectionState, | ||||||||||||||||||||||||||||||||||||||||||||||
| Updater, | ||||||||||||||||||||||||||||||||||||||||||||||
| useReactTable, | ||||||||||||||||||||||||||||||||||||||||||||||
| VisibilityState | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -34,11 +34,14 @@ import { | |||||||||||||||||||||||||||||||||||||||||||||
| } from './data-view.types'; | ||||||||||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||||||||||
| hasActiveQuery as computeHasActiveQuery, | ||||||||||||||||||||||||||||||||||||||||||||||
| createRowIdResolver, | ||||||||||||||||||||||||||||||||||||||||||||||
| fieldsToColumnDefs, | ||||||||||||||||||||||||||||||||||||||||||||||
| getDefaultTableQuery, | ||||||||||||||||||||||||||||||||||||||||||||||
| getFilteredRowModelWithFlatRows, | ||||||||||||||||||||||||||||||||||||||||||||||
| getInitialColumnVisibility, | ||||||||||||||||||||||||||||||||||||||||||||||
| groupData, | ||||||||||||||||||||||||||||||||||||||||||||||
| hasQueryChanged, | ||||||||||||||||||||||||||||||||||||||||||||||
| isGroupRowData, | ||||||||||||||||||||||||||||||||||||||||||||||
| queryToTableState, | ||||||||||||||||||||||||||||||||||||||||||||||
| transformToDataViewQuery | ||||||||||||||||||||||||||||||||||||||||||||||
| } from './utils'; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -57,6 +60,7 @@ function DataViewRoot<TData>({ | |||||||||||||||||||||||||||||||||||||||||||||
| onLoadMore, | ||||||||||||||||||||||||||||||||||||||||||||||
| onRowClick, | ||||||||||||||||||||||||||||||||||||||||||||||
| onColumnVisibilityChange, | ||||||||||||||||||||||||||||||||||||||||||||||
| onRowSelectionChange, | ||||||||||||||||||||||||||||||||||||||||||||||
| getRowId, | ||||||||||||||||||||||||||||||||||||||||||||||
| views, | ||||||||||||||||||||||||||||||||||||||||||||||
| defaultView, | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -130,6 +134,21 @@ function DataViewRoot<TData>({ | |||||||||||||||||||||||||||||||||||||||||||||
| [onColumnVisibilityChange] | ||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| // Lifted so a selection change invalidates the memoized context value; the | ||||||||||||||||||||||||||||||||||||||||||||||
| // table instance identity is stable and can't do it. | ||||||||||||||||||||||||||||||||||||||||||||||
| const [rowSelection, setRowSelectionState] = useState<RowSelectionState>({}); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| const handleRowSelectionChange = useCallback( | ||||||||||||||||||||||||||||||||||||||||||||||
| (value: Updater<RowSelectionState>) => { | ||||||||||||||||||||||||||||||||||||||||||||||
| setRowSelectionState(prev => { | ||||||||||||||||||||||||||||||||||||||||||||||
| const newValue = typeof value === 'function' ? value(prev) : value; | ||||||||||||||||||||||||||||||||||||||||||||||
| onRowSelectionChange?.(newValue); | ||||||||||||||||||||||||||||||||||||||||||||||
| return newValue; | ||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+141
to
+147
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 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 Calling a consumer callback inside the 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||
| [onRowSelectionChange] | ||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| const [tableQuery, setTableQuery] = | ||||||||||||||||||||||||||||||||||||||||||||||
| useState<InternalQuery>(defaultTableQuery); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -181,24 +200,32 @@ function DataViewRoot<TData>({ | |||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| }, [tableQuery, onTableQueryChange, mode]); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| const resolveRowId = useMemo(() => createRowIdResolver(getRowId), [getRowId]); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| const table = useReactTable({ | ||||||||||||||||||||||||||||||||||||||||||||||
| data: groupedData as unknown as TData[], | ||||||||||||||||||||||||||||||||||||||||||||||
| columns: columnDefs, | ||||||||||||||||||||||||||||||||||||||||||||||
| getRowId, | ||||||||||||||||||||||||||||||||||||||||||||||
| getRowId: resolveRowId, | ||||||||||||||||||||||||||||||||||||||||||||||
| // Group rows render without cells, so they have no checkbox — keeping them | ||||||||||||||||||||||||||||||||||||||||||||||
| // unselectable keeps `rowSelection` one key per data row. | ||||||||||||||||||||||||||||||||||||||||||||||
| enableRowSelection: row => !isGroupRowData(row.original), | ||||||||||||||||||||||||||||||||||||||||||||||
| getCoreRowModel: getCoreRowModel(), | ||||||||||||||||||||||||||||||||||||||||||||||
| getExpandedRowModel: getExpandedRowModel(), | ||||||||||||||||||||||||||||||||||||||||||||||
| getSubRows: row => (row as unknown as GroupedData<TData>)?.subRows || [], | ||||||||||||||||||||||||||||||||||||||||||||||
| getSortedRowModel: mode === 'server' ? undefined : getSortedRowModel(), | ||||||||||||||||||||||||||||||||||||||||||||||
| getFilteredRowModel: mode === 'server' ? undefined : getFilteredRowModel(), | ||||||||||||||||||||||||||||||||||||||||||||||
| getFilteredRowModel: | ||||||||||||||||||||||||||||||||||||||||||||||
| mode === 'server' ? undefined : getFilteredRowModelWithFlatRows(), | ||||||||||||||||||||||||||||||||||||||||||||||
| manualSorting: mode === 'server', | ||||||||||||||||||||||||||||||||||||||||||||||
| manualFiltering: mode === 'server', | ||||||||||||||||||||||||||||||||||||||||||||||
| onColumnVisibilityChange: handleColumnVisibilityChange, | ||||||||||||||||||||||||||||||||||||||||||||||
| onRowSelectionChange: handleRowSelectionChange, | ||||||||||||||||||||||||||||||||||||||||||||||
| globalFilterFn: mode === 'server' ? undefined : 'auto', | ||||||||||||||||||||||||||||||||||||||||||||||
| initialState: { columnVisibility: initialColumnVisibility }, | ||||||||||||||||||||||||||||||||||||||||||||||
| filterFromLeafRows: true, | ||||||||||||||||||||||||||||||||||||||||||||||
| state: { | ||||||||||||||||||||||||||||||||||||||||||||||
| ...reactTableState, | ||||||||||||||||||||||||||||||||||||||||||||||
| columnVisibility, | ||||||||||||||||||||||||||||||||||||||||||||||
| rowSelection, | ||||||||||||||||||||||||||||||||||||||||||||||
| expanded: | ||||||||||||||||||||||||||||||||||||||||||||||
| group_by && group_by !== defaultGroupOption.id ? true : undefined | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -260,6 +287,8 @@ function DataViewRoot<TData>({ | |||||||||||||||||||||||||||||||||||||||||||||
| shouldShowFilters, | ||||||||||||||||||||||||||||||||||||||||||||||
| columnVisibility, | ||||||||||||||||||||||||||||||||||||||||||||||
| setColumnVisibility: handleColumnVisibilityChange, | ||||||||||||||||||||||||||||||||||||||||||||||
| rowSelection, | ||||||||||||||||||||||||||||||||||||||||||||||
| setRowSelection: handleRowSelectionChange, | ||||||||||||||||||||||||||||||||||||||||||||||
| views, | ||||||||||||||||||||||||||||||||||||||||||||||
| activeView, | ||||||||||||||||||||||||||||||||||||||||||||||
| setActiveView, | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -287,6 +316,8 @@ function DataViewRoot<TData>({ | |||||||||||||||||||||||||||||||||||||||||||||
| shouldShowFilters, | ||||||||||||||||||||||||||||||||||||||||||||||
| columnVisibility, | ||||||||||||||||||||||||||||||||||||||||||||||
| handleColumnVisibilityChange, | ||||||||||||||||||||||||||||||||||||||||||||||
| rowSelection, | ||||||||||||||||||||||||||||||||||||||||||||||
| handleRowSelectionChange, | ||||||||||||||||||||||||||||||||||||||||||||||
| views, | ||||||||||||||||||||||||||||||||||||||||||||||
| activeView, | ||||||||||||||||||||||||||||||||||||||||||||||
| setActiveView, | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: raystack/apsara
Length of output: 1092
🏁 Script executed:
Repository: raystack/apsara
Length of output: 4806
Import
DataViewListColumnin the example.selectionColumnis typed asDataViewListColumn<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
🤖 Prompt for AI Agents