feat: add DataView component#822
Open
rohanchkrabrty wants to merge 2 commits into
Open
Conversation
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughThis pull request introduces a complete DataView component system for Raystack. DataView is a unified data primitive that owns query state (filters, sort, grouping, search) while supporting swappable renderers and a client/server mode toggle. The system includes a List renderer for table/list variants, hooks for state management, utilities for query translation and grouping, filter operations with type-specific predicates, comprehensive UI subcomponents (filters, search, toolbar, display controls), extensive CSS styling for grid-based layout with virtualization support, a full test suite covering all behaviors, documentation, and example components. The package exports are updated to serve DataView from the new location. Suggested reviewers
🚥 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. 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: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/www/src/components/theme-switcher/theme-toggle.tsx (1)
10-18:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGuard the toggle path until
themeis resolved.The icon fallback is safe now, but the click handler still treats
undefinedas"dark"and sets"light". That makes the first click during hydration a no-op or a wrong-direction toggle.Suggested fix
export default function ThemeToggle(props: HTMLAttributes<HTMLElement>) { const { setTheme, theme } = useTheme(); // `theme` can briefly be undefined or an unexpected value during hydration // (next-themes resolves async). Fall back so rendering doesn't crash. const Icon = ICONS_MAP[theme as keyof typeof ICONS_MAP] ?? Sun; + const canToggle = theme === 'light' || theme === 'dark'; return ( <IconButton aria-label='Toggle Theme' - onClick={() => setTheme({ theme: theme === 'light' ? 'dark' : 'light' })} + onClick={() => { + if (!canToggle) return; + setTheme({ theme: theme === 'light' ? 'dark' : 'light' }); + }} size={3} {...props} > <Icon /> </IconButton>🤖 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/components/theme-switcher/theme-toggle.tsx` around lines 10 - 18, The click handler uses theme directly and can mis-toggle while next-themes is still resolving; update the onClick for IconButton to guard against an unresolved theme by first checking that theme is a valid 'light' or 'dark' string (from useTheme) and returning early or disabling the handler when not; keep the existing ICONS_MAP/Icon fallback for rendering but only call setTheme({ theme: theme === 'light' ? 'dark' : 'light' }) inside the guarded branch so undefined won't be treated as 'dark' and cause a wrong/no-op toggle.
🧹 Nitpick comments (3)
packages/raystack/index.tsx (1)
33-42: 💤 Low valueConsider exporting advanced types for extensibility.
The main package exports only a subset of types available in
data-view/index.ts. While this keeps the API surface smaller, consumers building custom renderers or advanced integrations may need access toDataViewContextType,ViewSpec,GroupByResolver, and the subcomponent prop types.Currently, consumers would need to import these from deep paths (e.g.,
@raystack/apsara/components/data-view), which can be fragile across versions.Consider either:
- Exporting additional types commonly needed for advanced use cases
- Documenting the intended import strategy for advanced consumers
- Creating a separate
/advancedexport path if you want to signal these are power-user APIs🤖 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/index.tsx` around lines 33 - 42, The package root currently re-exports core DataView symbols from ./components/data-view but not advanced types; update packages/raystack/index.tsx to also re-export the advanced types from components/data-view (e.g., DataViewContextType, ViewSpec, GroupByResolver, and any subcomponent prop types like DataViewListColumnProps/DataViewFieldProps) so consumers don’t need deep imports, or alternatively add a clear `/advanced` export that re-exports those same symbols from ./components/data-view to signal power-user APIs; ensure the exported names match the type identifiers in components/data-view for compatibility.packages/raystack/components/data-view/__tests__/debug.test.tsx (1)
1-6: ⚡ Quick winDrop the skipped debug suite before merge.
describe.skip('debug')is just a local bisect artifact and adds dead noise to test discovery and maintenance.🤖 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/__tests__/debug.test.tsx` around lines 1 - 6, Remove the temporary skipped debug test suite to avoid dead test noise: delete the describe.skip('debug', ...) block (the anonymous suite and its noop it) from debug.test.tsx, or replace it with a meaningful test if intended; specifically remove the describe.skip('debug') and the inner it('noop', () => {}) so the file contains no leftover bisect artifacts.packages/raystack/components/data-view/__tests__/data-view.test.tsx (1)
635-657: ⚡ Quick winThis test doesn't cover the reset path it names.
The case says "Clear Filters button resets filters", but it never renders the toolbar/reset control or performs the interaction; it only proves the empty-state branch. Either rename it to match the current assertion, or drive the actual clear-filters flow and verify rows come back.
🤖 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/__tests__/data-view.test.tsx` around lines 635 - 657, The test named "Clear Filters button resets filters" never exercises the reset flow; update it to render the DataView toolbar controls and simulate the user clicking the clear action, then assert rows reappear. Specifically, render the same DataView (with DataView.List, DataView.EmptyState, mockData/mockFields/mockColumns/defaultSort and initial query.filters), use userEvent to find and click the "Clear Filters" button (or the toolbar control that clears filters), then assert the empty state is gone and table rows from mockData are present (e.g., expect screen.getByText(...) or getAllByRole('row') to show data). Alternatively, if you prefer the current assertion, rename the test title to reflect it only checks the empty state branch instead of the reset behavior.
🤖 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/components/dataview-demo.tsx`:
- Around line 266-291: The demo DataViewEmptyZeroDemo currently uses a single
boolean state filtered that never changes and cannot express both "empty" (no
rows but an active query) and "zero" (no rows and no active query); replace the
boolean with a tri-state (e.g., mode variable with values like 'populated' |
'empty' | 'zero') or split into two separate examples, then update
DataViewEmptyZeroDemo to set data and query based on that mode (for 'populated'
use data=people and no query, for 'empty' use data=[] plus
query={filters:[{name:'name',operator:'eq',value:'Nobody'}]}, for 'zero' use
data=[] and query=undefined), and wire the UI control (buttons/RadioGroup) to
call setMode so the demo can render all three states; keep references to
DataView, DataView.EmptyState, and DataView.ZeroState intact.
- Around line 305-340: The custom card renderer currently maps over ctx.data
(bypassing table state) so toolbar filters/sort/grouping don't apply; update the
DataView.Custom render to iterate over table.getRowModel().rows instead, using
each row (e.g., const p = row.original as Person) and row.id for the key, and
keep the DataView.DisplayAccess/accessorKey usage unchanged so cards reflect the
actual row model produced by the table.
In `@apps/www/src/content/docs/components/dataview/index.mdx`:
- Around line 79-80: Update the "Empty state" bullet in the dataview docs:
remove "sort" from the explanation so it reads that no rows are visible because
filters/search exclude them, and keep the note that the toolbar stays visible;
edit the line with the "**Empty state**" bullet accordingly in
apps/www/src/content/docs/components/dataview/index.mdx.
In `@apps/www/src/content/docs/components/dataview/props.ts`:
- Around line 76-112: The docs mirror is missing several exported props present
in the real types; update the definitions here to match
packages/raystack/components/data-view/data-view.types.tsx by adding the missing
fields: on DataViewField add dataType, defaultFilterValue, filterProps and
groupCountMap (or groupLabelsMap rename if applicable); on DataViewListProps add
classNames; on DataViewListColumn add classNames and styles; and extend
DataViewQuery.filters with the extra filter value fields (e.g.
value/values/defaultValue/defaultValues depending on type). Ensure the property
names and types match the source file (DataViewField, DataViewListProps,
DataViewListColumn, DataViewQuery) so the generated API table documents all
customization points.
In `@packages/raystack/components/data-view/components/empty-state.tsx`:
- Around line 9-10: The JSDoc for the forView prop is misleading: it currently
says it restricts to a view's `name` but the code compares forView against
`activeView` (which represents the view `value`). Update the comment for the
forView prop to state it matches the view `value` (activeView) instead of
`name`, and ensure any related docs/comments near the comparison logic that
references `activeView` are consistent with this change (look for the forView
prop and the activeView comparison).
In `@packages/raystack/components/data-view/components/filters.tsx`:
- Around line 80-87: DataViewFiltersProps declares classNames.addFilter but the
component never forwards it to the AddFilter control or its trigger; either
remove classNames.addFilter from the public interface or wire it through. Update
the component rendering where AddFilter (and its trigger usage) is created to
pass props.classNames?.addFilter to the AddFilter component/trigger (or map into
the trigger's className prop), referencing DataViewFiltersProps, AddFilter and
the trigger prop so the passed-in class is applied; if you choose to remove it,
delete classNames.addFilter from DataViewFiltersProps and any related
docs/usage.
- Around line 45-49: The icon-only filter trigger rendered when
appliedFiltersSet.size > 0 uses IconButton with only <FilterIcon />, which is
inaccessible to screen readers; update the IconButton in the branch that returns
the icon to include an accessible name (e.g., aria-label="Open filters" or
aria-labelledby pointing to a visible label) or wrap it with a Tooltip that
provides an accessible label, ensuring the IconButton (component IconButton and
the <FilterIcon /> usage) has an aria-label/aria-labelledby so screen readers
can announce its purpose.
In `@packages/raystack/components/data-view/components/list.tsx`:
- Around line 139-158: The computed groupHeaderList.start currently sums the
estimated effectiveRowHeight which drifts when rows are re-measured; update
groupHeaderList to compute each group's start using actual measured heights
instead of only effectiveRowHeight (e.g., use the virtualizer/row measurement
data or a per-row size field if available) — for each group at index i, compute
start by summing rows[0..i-1].size (falling back to effectiveRowHeight when size
is undefined) plus GROUP_HEADER_HEIGHT where applicable; this ensures
useStickyGroupAnchor sees real offsets and fixes the sticky group anchoring
drift.
- Around line 375-388: The row div with onRowClick is currently mouse-only; make
it keyboard-activatable by adding tabIndex={0} and an onKeyDown handler that
invokes onRowClick(row.original) when Enter or Space is pressed (call
event.preventDefault() for Space to avoid page scroll). Keep the existing role
prop (ariaRole === 'table' ? 'row' : 'listitem') and existing onClick, but add
tabIndex and onKeyDown to the same element (the div that currently has
ref={measure}, data-index, className and onClick) so keyboard users can focus
and activate the row.
In `@packages/raystack/components/data-view/components/search.tsx`:
- Around line 7-13: DataViewSearchProps currently extends ComponentProps<typeof
Search> but must omit controlled props that DataViewSearch always overrides;
update the type to extend Omit<ComponentProps<typeof Search>, 'onChange' |
'value' | 'onClear'> so consumer-provided onChange/value/onClear cannot be
passed (leave disabled handling as-is since DataViewSearch respects disabled via
disabled ?? ...); update the DataViewSearchProps declaration (and any related
usages) to use this Omit type and keep the autoDisableInZeroState flag.
In `@packages/raystack/components/data-view/data-view.tsx`:
- Around line 182-188: The table is passing the caller's getRowId directly but
grouping injects GroupedData header rows (from getSubRows) that lack a normal
id; wrap or replace the getRowId passed into useReactTable so it first detects
synthetic group rows (e.g., by checking for (row as GroupedData<TData>).subRows
or a missing id) and returns a deterministic internal id for headers (like
`__group-${groupKey}` or based on group index/key), and only calls the original
getRowId for real data rows; update the useReactTable call to pass this guarded
getRowId so header rows never produce undefined/duplicate ids.
- Around line 112-118: The current implementation seeds columnVisibility once
from getInitialColumnVisibility(fields) into initialColumnVisibility and never
re-seeds, so per-view/render overrides of defaultHidden are ignored; change the
logic to either (A) maintain visibility keyed by an active view id (e.g., a
visibilityMap: Record<viewId, VisibilityState>) and read/write via
setColumnVisibilityState for the current view, or (B) add a useEffect that
watches the effective fields/render override and re-seeds columnVisibility by
calling getInitialColumnVisibility(fields) (or the renderer-specific field list)
when that set changes; update references to initialColumnVisibility,
getInitialColumnVisibility, columnVisibility and setColumnVisibilityState
accordingly so switching renderers/views applies the view-specific defaultHidden
values.
In `@packages/raystack/components/data-view/hooks/useFilters.tsx`:
- Around line 20-26: The defaultValue initialization in useFilters.tsx
incorrectly falls through to an empty string for FilterType.multiselect, causing
later normalization (which expects an array and calls .map) to throw; update the
ternary to explicitly return an empty array for FilterType.multiselect (i.e.,
handle FilterType.multiselect branch separately and return []), keeping existing
branches for FilterType.date and FilterType.select and using
field.defaultFilterValue when present so defaultValue is always the correct type
for the filter.
In `@packages/raystack/components/data-view/hooks/useInfiniteScroll.tsx`:
- Around line 53-58: The IntersectionObserver can fire multiple times before the
parent re-renders so add an internal latch in useInfiniteScroll to prevent
duplicate calls: introduce a ref like internalLoadingRef, check it alongside
isLoadingRef inside the observer callback (return if either is true), set
internalLoadingRef.current = true immediately before calling
onLoadMoreRef.current(), and clear internalLoadingRef.current when loading
completes (e.g., in an effect that watches isLoadingRef.current becoming false
or by clearing after the onLoadMore promise resolves if onLoadMoreRef returns
one); update symbols referenced: observer callback, isLoadingRef, onLoadMoreRef,
and useInfiniteScroll.
In `@packages/raystack/components/data-view/utils/index.tsx`:
- Around line 24-37: The columnFilters construction is currently only excluding
empty strings so empty arrays (e.g., cleared multiselects) still become active
filters; update the predicate used on query.filters (the filter inside the
columnFilters, and similarly the other occurrences at the other ranges) to treat
arrays as inactive by checking if Array.isArray(data.value) ? data.value.length
> 0 : data.value !== '' (and preserve the special-case for FilterType.date using
dayjs); keep the rest of the mapping logic (valueObject creation and returning {
value: valueObject, id: data?.name }) the same.
- Around line 139-148: The isFilterChanged function currently compares filter
values by reference (oldFilterMap.get(key) !== value) which falsely marks
multiselect arrays as changed; update isFilterChanged (and/or generateFilterMap
consumer logic) to perform value-aware comparisons: if both old and new values
are arrays (multiselect), compare their contents (e.g., same length and same
items irrespective of order or by sorting and comparing elements) or use a
stable deep-equality check for objects/arrays; keep scalar comparisons as-is and
ensure you reference isFilterChanged and generateFilterMap/InternalFilter when
making the change.
- Around line 261-305: When building internalFilters in the filters.map block,
handle the serialized select "empty" operator emitted by
transformToDataViewQuery(): detect when operator === 'empty' (in the same area
where you handle 'ilike') and replace it with the internal operator that your
select filter operations expect (e.g., map to 'is_empty' or whatever key exists
in filterOperationsMap.select) so preloaded/shared queries with empty-select
filters resolve correctly; update the transformedFilter assignment (the variable
named transformedFilter inside the filters.map function) to perform this mapping
before returning the InternalFilter.
---
Outside diff comments:
In `@apps/www/src/components/theme-switcher/theme-toggle.tsx`:
- Around line 10-18: The click handler uses theme directly and can mis-toggle
while next-themes is still resolving; update the onClick for IconButton to guard
against an unresolved theme by first checking that theme is a valid 'light' or
'dark' string (from useTheme) and returning early or disabling the handler when
not; keep the existing ICONS_MAP/Icon fallback for rendering but only call
setTheme({ theme: theme === 'light' ? 'dark' : 'light' }) inside the guarded
branch so undefined won't be treated as 'dark' and cause a wrong/no-op toggle.
---
Nitpick comments:
In `@packages/raystack/components/data-view/__tests__/data-view.test.tsx`:
- Around line 635-657: The test named "Clear Filters button resets filters"
never exercises the reset flow; update it to render the DataView toolbar
controls and simulate the user clicking the clear action, then assert rows
reappear. Specifically, render the same DataView (with DataView.List,
DataView.EmptyState, mockData/mockFields/mockColumns/defaultSort and initial
query.filters), use userEvent to find and click the "Clear Filters" button (or
the toolbar control that clears filters), then assert the empty state is gone
and table rows from mockData are present (e.g., expect screen.getByText(...) or
getAllByRole('row') to show data). Alternatively, if you prefer the current
assertion, rename the test title to reflect it only checks the empty state
branch instead of the reset behavior.
In `@packages/raystack/components/data-view/__tests__/debug.test.tsx`:
- Around line 1-6: Remove the temporary skipped debug test suite to avoid dead
test noise: delete the describe.skip('debug', ...) block (the anonymous suite
and its noop it) from debug.test.tsx, or replace it with a meaningful test if
intended; specifically remove the describe.skip('debug') and the inner
it('noop', () => {}) so the file contains no leftover bisect artifacts.
In `@packages/raystack/index.tsx`:
- Around line 33-42: The package root currently re-exports core DataView symbols
from ./components/data-view but not advanced types; update
packages/raystack/index.tsx to also re-export the advanced types from
components/data-view (e.g., DataViewContextType, ViewSpec, GroupByResolver, and
any subcomponent prop types like DataViewListColumnProps/DataViewFieldProps) so
consumers don’t need deep imports, or alternatively add a clear `/advanced`
export that re-exports those same symbols from ./components/data-view to signal
power-user APIs; ensure the exported names match the type identifiers in
components/data-view for compatibility.
🪄 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
Run ID: 137af1f9-91e4-4f73-a539-5e3c2ab3a267
📒 Files selected for processing (37)
apps/www/src/app/examples/dataview-beta/page.tsxapps/www/src/components/dataview-demo.tsxapps/www/src/components/demo/demo.tsxapps/www/src/components/theme-switcher/theme-toggle.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/__tests__/data-view.test.tsxpackages/raystack/components/data-view/__tests__/debug.test.tsxpackages/raystack/components/data-view/__tests__/filter-operations.test.tspackages/raystack/components/data-view/components/custom.tsxpackages/raystack/components/data-view/components/display-access.tsxpackages/raystack/components/data-view/components/display-controls.tsxpackages/raystack/components/data-view/components/display-properties.tsxpackages/raystack/components/data-view/components/empty-state.tsxpackages/raystack/components/data-view/components/filters.tsxpackages/raystack/components/data-view/components/grouping.tsxpackages/raystack/components/data-view/components/list.tsxpackages/raystack/components/data-view/components/ordering.tsxpackages/raystack/components/data-view/components/search.tsxpackages/raystack/components/data-view/components/toolbar.tsxpackages/raystack/components/data-view/components/view-switcher.tsxpackages/raystack/components/data-view/components/zero-state.tsxpackages/raystack/components/data-view/context.tsxpackages/raystack/components/data-view/data-view.module.csspackages/raystack/components/data-view/data-view.tsxpackages/raystack/components/data-view/data-view.types.tsxpackages/raystack/components/data-view/hooks/useDataView.tsxpackages/raystack/components/data-view/hooks/useElementHeight.tsxpackages/raystack/components/data-view/hooks/useFilters.tsxpackages/raystack/components/data-view/hooks/useInfiniteScroll.tsxpackages/raystack/components/data-view/hooks/useStickyGroupAnchor.tsxpackages/raystack/components/data-view/hooks/useVirtualRows.tsxpackages/raystack/components/data-view/index.tspackages/raystack/components/data-view/utils/filter-operations.tsxpackages/raystack/components/data-view/utils/index.tsxpackages/raystack/index.tsx
Comment on lines
+266
to
+291
| export function DataViewEmptyZeroDemo() { | ||
| const [filtered, setFiltered] = useState(false); | ||
| return ( | ||
| <Flex direction='column' gap={4} width='full'> | ||
| <div style={{ height: 400 }}> | ||
| <DataView | ||
| data={filtered ? [] : people} | ||
| fields={fields} | ||
| defaultSort={defaultSort} | ||
| query={ | ||
| filtered | ||
| ? { filters: [{ name: 'name', operator: 'eq', value: 'Nobody' }] } | ||
| : undefined | ||
| } | ||
| > | ||
| <DataView.Toolbar> | ||
| <DataView.Filters /> | ||
| </DataView.Toolbar> | ||
| <DataView.List variant='table' columns={tableColumns} /> | ||
| <DataView.EmptyState> | ||
| <Text>No people match your filters.</Text> | ||
| </DataView.EmptyState> | ||
| <DataView.ZeroState> | ||
| <Text>Nothing here yet.</Text> | ||
| </DataView.ZeroState> | ||
| </DataView> |
Contributor
There was a problem hiding this comment.
This example never demonstrates both states.
filtered is never updated, so the demo only renders the populated table today. Even if you wire a toggle later, this boolean can only express the empty state; zero state also needs data={[]} with no active query. Please switch this to a small tri-state control or split it into separate fixed examples.
🤖 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/components/dataview-demo.tsx` around lines 266 - 291, The demo
DataViewEmptyZeroDemo currently uses a single boolean state filtered that never
changes and cannot express both "empty" (no rows but an active query) and "zero"
(no rows and no active query); replace the boolean with a tri-state (e.g., mode
variable with values like 'populated' | 'empty' | 'zero') or split into two
separate examples, then update DataViewEmptyZeroDemo to set data and query based
on that mode (for 'populated' use data=people and no query, for 'empty' use
data=[] plus query={filters:[{name:'name',operator:'eq',value:'Nobody'}]}, for
'zero' use data=[] and query=undefined), and wire the UI control
(buttons/RadioGroup) to call setMode so the demo can render all three states;
keep references to DataView, DataView.EmptyState, and DataView.ZeroState intact.
Comment on lines
+305
to
+340
| <DataView.Custom> | ||
| {ctx => ( | ||
| <Flex gap={3} wrap='wrap' style={{ padding: 'var(--rs-space-4)' }}> | ||
| {(ctx.data as Person[]).map(p => ( | ||
| <Flex | ||
| key={p.id} | ||
| direction='column' | ||
| gap={2} | ||
| style={{ | ||
| padding: 'var(--rs-space-4)', | ||
| border: '0.5px solid var(--rs-color-border-base-primary)', | ||
| borderRadius: 'var(--rs-radius-3)', | ||
| width: 220 | ||
| }} | ||
| > | ||
| <DataView.DisplayAccess accessorKey='name'> | ||
| <Text weight='medium'>{p.name}</Text> | ||
| </DataView.DisplayAccess> | ||
| <DataView.DisplayAccess accessorKey='email'> | ||
| <Text size='small' variant='secondary'> | ||
| {p.email} | ||
| </Text> | ||
| </DataView.DisplayAccess> | ||
| <Flex gap={2}> | ||
| <DataView.DisplayAccess accessorKey='team'> | ||
| <Badge variant='neutral'>{p.team}</Badge> | ||
| </DataView.DisplayAccess> | ||
| <DataView.DisplayAccess accessorKey='status'> | ||
| <Badge variant='success'>{p.status}</Badge> | ||
| </DataView.DisplayAccess> | ||
| </Flex> | ||
| </Flex> | ||
| ))} | ||
| </Flex> | ||
| )} | ||
| </DataView.Custom> |
Contributor
There was a problem hiding this comment.
Drive the custom cards from the row model, not ctx.data.
This renderer is bypassing the queried row set, so filters/search/sort/grouping in the toolbar won't affect the cards. Use table.getRowModel().rows here, like the main example page does.
Suggested fix
- <DataView.Custom>
- {ctx => (
+ <DataView.Custom>
+ {({ table }) => (
<Flex gap={3} wrap='wrap' style={{ padding: 'var(--rs-space-4)' }}>
- {(ctx.data as Person[]).map(p => (
- <Flex
- key={p.id}
+ {table
+ .getRowModel()
+ .rows.filter(row => !row.subRows?.length)
+ .map(row => {
+ const p = row.original;
+ return (
+ <Flex
+ key={row.id}
direction='column'
gap={2}
style={{
padding: 'var(--rs-space-4)',
border: '0.5px solid var(--rs-color-border-base-primary)',
borderRadius: 'var(--rs-radius-3)',
width: 220
}}
>
<DataView.DisplayAccess accessorKey='name'>
<Text weight='medium'>{p.name}</Text>
</DataView.DisplayAccess>
<DataView.DisplayAccess accessorKey='email'>
<Text size='small' variant='secondary'>
{p.email}
</Text>
</DataView.DisplayAccess>
<Flex gap={2}>
<DataView.DisplayAccess accessorKey='team'>
<Badge variant='neutral'>{p.team}</Badge>
</DataView.DisplayAccess>
<DataView.DisplayAccess accessorKey='status'>
<Badge variant='success'>{p.status}</Badge>
</DataView.DisplayAccess>
</Flex>
</Flex>
- ))}
+ );
+ })}
</Flex>
)}
</DataView.Custom>🤖 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/components/dataview-demo.tsx` around lines 305 - 340, The custom
card renderer currently maps over ctx.data (bypassing table state) so toolbar
filters/sort/grouping don't apply; update the DataView.Custom render to iterate
over table.getRowModel().rows instead, using each row (e.g., const p =
row.original as Person) and row.id for the key, and keep the
DataView.DisplayAccess/accessorKey usage unchanged so cards reflect the actual
row model produced by the table.
Comment on lines
+79
to
+80
| - **Zero state** — no data, no active query. The "first-use" surface. Toolbar is hidden automatically. | ||
| - **Empty state** — no rows visible because filters/search/sort exclude them all. Toolbar stays visible so the user can correct it. |
Contributor
There was a problem hiding this comment.
Remove sorting from the empty-state explanation.
Sorting only reorders rows; it does not drive the row count to zero. Mentioning it here makes the empty-state rules inaccurate.
🤖 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/index.mdx` around lines 79 -
80, Update the "Empty state" bullet in the dataview docs: remove "sort" from the
explanation so it reads that no rows are visible because filters/search exclude
them, and keep the note that the toolbar stays visible; edit the line with the
"**Empty state**" bullet accordingly in
apps/www/src/content/docs/components/dataview/index.mdx.
Comment on lines
+76
to
+112
| export interface DataViewField { | ||
| /** Key into the row object. */ | ||
| accessorKey: string; | ||
|
|
||
| /** Human-readable label. */ | ||
| label: string; | ||
|
|
||
| /** Optional icon (e.g. for grouping menu). */ | ||
| icon?: ReactNode; | ||
|
|
||
| /** Allow filtering on this field. */ | ||
| filterable?: boolean; | ||
|
|
||
| /** Filter input type. */ | ||
| filterType?: 'string' | 'number' | 'date' | 'select' | 'multiselect'; | ||
|
|
||
| /** Options when filterType is select/multiselect. */ | ||
| filterOptions?: Array<{ label: string; value: string }>; | ||
|
|
||
| /** Allow sorting. */ | ||
| sortable?: boolean; | ||
|
|
||
| /** Allow grouping. */ | ||
| groupable?: boolean; | ||
|
|
||
| /** Allow toggling visibility. */ | ||
| hideable?: boolean; | ||
|
|
||
| /** Hide this field by default. */ | ||
| defaultHidden?: boolean; | ||
|
|
||
| /** Show item count next to the group header label. */ | ||
| showGroupCount?: boolean; | ||
|
|
||
| /** Override group bucket labels (key → label). */ | ||
| groupLabelsMap?: Record<string, string>; | ||
| } |
Contributor
There was a problem hiding this comment.
Sync this docs mirror with the exported DataView types.
This file drops several public props that exist in packages/raystack/components/data-view/data-view.types.tsx — for example DataViewField.dataType/defaultFilterValue/filterProps/groupCountMap, DataViewListProps.classNames, DataViewListColumn.classNames/styles, and the extra filter value fields on DataViewQuery.filters. The generated API table will under-document supported customization points.
Also applies to: 114-173, 230-241
🤖 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/props.ts` around lines 76 -
112, The docs mirror is missing several exported props present in the real
types; update the definitions here to match
packages/raystack/components/data-view/data-view.types.tsx by adding the missing
fields: on DataViewField add dataType, defaultFilterValue, filterProps and
groupCountMap (or groupLabelsMap rename if applicable); on DataViewListProps add
classNames; on DataViewListColumn add classNames and styles; and extend
DataViewQuery.filters with the extra filter value fields (e.g.
value/values/defaultValue/defaultValues depending on type). Ensure the property
names and types match the source file (DataViewField, DataViewListProps,
DataViewListColumn, DataViewQuery) so the generated API table documents all
customization points.
Comment on lines
+9
to
+10
| /** Restrict to a specific view's `name`. */ | ||
| forView?: string; |
Contributor
There was a problem hiding this comment.
Align forView docs with the actual match key.
Line 9 says forView targets view name, but Line 27 compares it with activeView (which is view value in this API). Please update wording to avoid misuse.
Also applies to: 25-27
🤖 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/components/empty-state.tsx` around
lines 9 - 10, The JSDoc for the forView prop is misleading: it currently says it
restricts to a view's `name` but the code compares forView against `activeView`
(which represents the view `value`). Update the comment for the forView prop to
state it matches the view `value` (activeView) instead of `name`, and ensure any
related docs/comments near the comparison logic that references `activeView` are
consistent with this change (look for the forView prop and the activeView
comparison).
Comment on lines
+20
to
+26
| const defaultValue = | ||
| field.defaultFilterValue ?? | ||
| (filterType === FilterType.date | ||
| ? new Date() | ||
| : filterType === FilterType.select | ||
| ? options[0]?.value | ||
| : ''); |
Contributor
There was a problem hiding this comment.
Initialize multiselect filters with an array, not ''.
FilterType.multiselect currently falls through to '', but the normalization path later treats multiselect values as arrays and calls .map(...). Adding a multiselect filter before making a selection will therefore throw at runtime.
Suggested fix
const defaultValue =
field.defaultFilterValue ??
(filterType === FilterType.date
? new Date()
: filterType === FilterType.select
? options[0]?.value
+ : filterType === FilterType.multiselect
+ ? []
: '');📝 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 defaultValue = | |
| field.defaultFilterValue ?? | |
| (filterType === FilterType.date | |
| ? new Date() | |
| : filterType === FilterType.select | |
| ? options[0]?.value | |
| : ''); | |
| const defaultValue = | |
| field.defaultFilterValue ?? | |
| (filterType === FilterType.date | |
| ? new Date() | |
| : filterType === FilterType.select | |
| ? options[0]?.value | |
| : filterType === FilterType.multiselect | |
| ? [] | |
| : ''); |
🤖 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/hooks/useFilters.tsx` around lines 20
- 26, The defaultValue initialization in useFilters.tsx incorrectly falls
through to an empty string for FilterType.multiselect, causing later
normalization (which expects an array and calls .map) to throw; update the
ternary to explicitly return an empty array for FilterType.multiselect (i.e.,
handle FilterType.multiselect branch separately and return []), keeping existing
branches for FilterType.date and FilterType.select and using
field.defaultFilterValue when present so defaultValue is always the correct type
for the filter.
Comment on lines
+53
to
+58
| const observer = new IntersectionObserver( | ||
| entries => { | ||
| const entry = entries[0]; | ||
| if (!entry?.isIntersecting) return; | ||
| if (isLoadingRef.current) return; | ||
| onLoadMoreRef.current?.(); |
Contributor
There was a problem hiding this comment.
Add an internal latch before calling onLoadMore.
isLoadingRef only protects you after the parent re-renders. If the sentinel stays intersecting, the observer can fire again in that gap and enqueue duplicate loadMore calls for the same cursor/page.
Proposed fix
export function useInfiniteScroll({
enabled,
sentinelRef,
scrollRef,
isLoading,
onLoadMore,
rootMargin = 200
}: UseInfiniteScrollParams) {
const onLoadMoreRef = useRef(onLoadMore);
const isLoadingRef = useRef(isLoading);
+ const pendingRef = useRef(false);
onLoadMoreRef.current = onLoadMore;
isLoadingRef.current = isLoading;
+
+ useEffect(() => {
+ if (!isLoading) pendingRef.current = false;
+ }, [isLoading]);
useEffect(() => {
if (!enabled) return;
const sentinel = sentinelRef.current;
if (!sentinel) return;
@@
const observer = new IntersectionObserver(
entries => {
const entry = entries[0];
if (!entry?.isIntersecting) return;
- if (isLoadingRef.current) return;
+ if (isLoadingRef.current || pendingRef.current) return;
+ pendingRef.current = true;
onLoadMoreRef.current?.();
},🤖 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/hooks/useInfiniteScroll.tsx` around
lines 53 - 58, The IntersectionObserver can fire multiple times before the
parent re-renders so add an internal latch in useInfiniteScroll to prevent
duplicate calls: introduce a ref like internalLoadingRef, check it alongside
isLoadingRef inside the observer callback (return if either is true), set
internalLoadingRef.current = true immediately before calling
onLoadMoreRef.current(), and clear internalLoadingRef.current when loading
completes (e.g., in an effect that watches isLoadingRef.current becoming false
or by clearing after the onLoadMore promise resolves if onLoadMoreRef returns
one); update symbols referenced: observer callback, isLoadingRef, onLoadMoreRef,
and useInfiniteScroll.
Comment on lines
+24
to
+37
| const columnFilters = | ||
| query.filters | ||
| ?.filter(data => { | ||
| if (data._type === FilterType.date) return dayjs(data.value).isValid(); | ||
| if (data.value !== '') return true; | ||
| return false; | ||
| }) | ||
| ?.map(data => { | ||
| const valueObject = | ||
| data._type === FilterType.date | ||
| ? { date: data.value } | ||
| : { value: data.value }; | ||
| return { value: valueObject, id: data?.name }; | ||
| }) || []; |
Contributor
There was a problem hiding this comment.
Treat empty multiselect arrays as inactive filters.
These checks only exclude '', so [] survives as an active filter. In client mode that turns a cleared multiselect into a filter that matches nothing; it also survives query serialization/change detection and can flip zero-state into empty-state.
Please use the same non-empty predicate here for strings and arrays.
Suggested fix
+const hasFilterValue = (value: unknown) => {
+ if (Array.isArray(value)) return value.length > 0;
+ return value !== '';
+};
+
export function queryToTableState(query: InternalQuery): Partial<TableState> {
const columnFilters =
query.filters
?.filter(data => {
if (data._type === FilterType.date) return dayjs(data.value).isValid();
- if (data.value !== '') return true;
+ if (hasFilterValue(data.value)) return true;
return false;
})
@@
const generateFilterMap = (
filters: InternalFilter[] = []
): Map<string, any> => {
return new Map(
filters
- ?.filter(data => data._type === FilterType.select || data.value !== '')
+ ?.filter(
+ data => data._type === FilterType.select || hasFilterValue(data.value)
+ )
.map(({ name, operator, value }) => [`${name}-${operator}`, value])
);
};
@@
const sanitizedFilters =
filters
?.filter(data => {
if (data._type === FilterType.select) return true;
if (data._type === FilterType.date) return dayjs(data.value).isValid();
- if (data.value !== '') return true;
+ if (hasFilterValue(data.value)) return true;
return false;
})Also applies to: 125-133, 222-243
🤖 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/utils/index.tsx` around lines 24 - 37,
The columnFilters construction is currently only excluding empty strings so
empty arrays (e.g., cleared multiselects) still become active filters; update
the predicate used on query.filters (the filter inside the columnFilters, and
similarly the other occurrences at the other ranges) to treat arrays as inactive
by checking if Array.isArray(data.value) ? data.value.length > 0 : data.value
!== '' (and preserve the special-case for FilterType.date using dayjs); keep the
rest of the mapping logic (valueObject creation and returning { value:
valueObject, id: data?.name }) the same.
Comment on lines
+139
to
+148
| const isFilterChanged = ( | ||
| oldFilters: InternalFilter[] = [], | ||
| newFilters: InternalFilter[] = [] | ||
| ): boolean => { | ||
| const oldFilterMap = generateFilterMap(oldFilters); | ||
| const newFilterMap = generateFilterMap(newFilters); | ||
| if (oldFilterMap.size !== newFilterMap.size) return true; | ||
| return [...newFilterMap].some( | ||
| ([key, value]) => oldFilterMap.get(key) !== value | ||
| ); |
Contributor
There was a problem hiding this comment.
Don't compare multiselect values by array identity.
oldFilterMap.get(key) !== value is only reliable for scalars. For multiselect filters, two arrays with identical selections but different references will always read as changed, which can trigger redundant query updates/fetches.
Suggested fix
const isFilterChanged = (
oldFilters: InternalFilter[] = [],
newFilters: InternalFilter[] = []
): boolean => {
const oldFilterMap = generateFilterMap(oldFilters);
const newFilterMap = generateFilterMap(newFilters);
if (oldFilterMap.size !== newFilterMap.size) return true;
return [...newFilterMap].some(
- ([key, value]) => oldFilterMap.get(key) !== value
+ ([key, value]) =>
+ JSON.stringify(oldFilterMap.get(key)) !== JSON.stringify(value)
);
};📝 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 isFilterChanged = ( | |
| oldFilters: InternalFilter[] = [], | |
| newFilters: InternalFilter[] = [] | |
| ): boolean => { | |
| const oldFilterMap = generateFilterMap(oldFilters); | |
| const newFilterMap = generateFilterMap(newFilters); | |
| if (oldFilterMap.size !== newFilterMap.size) return true; | |
| return [...newFilterMap].some( | |
| ([key, value]) => oldFilterMap.get(key) !== value | |
| ); | |
| const isFilterChanged = ( | |
| oldFilters: InternalFilter[] = [], | |
| newFilters: InternalFilter[] = [] | |
| ): boolean => { | |
| const oldFilterMap = generateFilterMap(oldFilters); | |
| const newFilterMap = generateFilterMap(newFilters); | |
| if (oldFilterMap.size !== newFilterMap.size) return true; | |
| return [...newFilterMap].some( | |
| ([key, value]) => | |
| JSON.stringify(oldFilterMap.get(key)) !== JSON.stringify(value) | |
| ); | |
| }; |
🤖 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/utils/index.tsx` around lines 139 -
148, The isFilterChanged function currently compares filter values by reference
(oldFilterMap.get(key) !== value) which falsely marks multiselect arrays as
changed; update isFilterChanged (and/or generateFilterMap consumer logic) to
perform value-aware comparisons: if both old and new values are arrays
(multiselect), compare their contents (e.g., same length and same items
irrespective of order or by sorting and comparing elements) or use a stable
deep-equality check for objects/arrays; keep scalar comparisons as-is and ensure
you reference isFilterChanged and generateFilterMap/InternalFilter when making
the change.
Comment on lines
+261
to
+305
| const internalFilters: InternalFilter[] = filters.map(filter => { | ||
| const { | ||
| operator, | ||
| value, | ||
| stringValue, | ||
| numberValue, | ||
| boolValue, | ||
| ...filterRest | ||
| } = filter; | ||
|
|
||
| let transformedFilter = { | ||
| operator: operator as FilterOperatorTypes, | ||
| value, | ||
| ...(stringValue !== undefined && { stringValue }), | ||
| ...(numberValue !== undefined && { numberValue }), | ||
| ...(boolValue !== undefined && { boolValue }) | ||
| }; | ||
|
|
||
| if (operator === 'ilike' && stringValue) { | ||
| if (stringValue.startsWith('%') && stringValue.endsWith('%')) { | ||
| transformedFilter = { | ||
| operator: 'contains', | ||
| value: stringValue.slice(1, -1) | ||
| }; | ||
| } else if (stringValue.endsWith('%')) { | ||
| transformedFilter = { | ||
| operator: 'starts_with', | ||
| value: stringValue.slice(0, -1) | ||
| }; | ||
| } else if (stringValue.startsWith('%')) { | ||
| transformedFilter = { | ||
| operator: 'ends_with', | ||
| value: stringValue.slice(1) | ||
| }; | ||
| } else { | ||
| transformedFilter = { operator: 'contains', value: stringValue }; | ||
| } | ||
| } | ||
|
|
||
| return { | ||
| ...filterRest, | ||
| ...transformedFilter, | ||
| _type: undefined, | ||
| _dataType: undefined | ||
| } as InternalFilter; |
Contributor
There was a problem hiding this comment.
Reverse the serialized empty operator before storing the internal query.
transformToDataViewQuery() emits operator: 'empty' for select filters, but this reverse path never maps it back. That leaves the internal query holding an operator that has no entry in filterOperationsMap.select, so preloaded/shared queries using an empty-select filter won't work in client mode.
Suggested fix
+import { EmptyFilterValue, FilterOperatorTypes, FilterType } from '~/types/filters';
@@
- if (operator === 'ilike' && stringValue) {
+ if (operator === 'empty') {
+ transformedFilter = {
+ operator: 'eq',
+ value: EmptyFilterValue
+ };
+ } else if (operator === 'ilike' && stringValue) {
if (stringValue.startsWith('%') && stringValue.endsWith('%')) {
transformedFilter = {
operator: 'contains',🤖 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/utils/index.tsx` around lines 261 -
305, When building internalFilters in the filters.map block, handle the
serialized select "empty" operator emitted by transformToDataViewQuery(): detect
when operator === 'empty' (in the same area where you handle 'ilike') and
replace it with the internal operator that your select filter operations expect
(e.g., map to 'is_empty' or whatever key exists in filterOperationsMap.select)
so preloaded/shared queries with empty-select filters resolve correctly; update
the transformedFilter assignment (the variable named transformedFilter inside
the filters.map function) to perform this mapping before returning the
InternalFilter.
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
1 participant
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
DataView, a unified data primitive whose query state (filters, sort, group, search) drives swappable renderers —DataView.List(table + list presentations) andDataView.Custom(escape hatch for cards, kanban, etc.) — with client and server modes.Search,Filters,DisplayControls) plus a single global column-visibility model exposed viaDisplayAccess, andEmptyState/ZeroStatesibling surfaces.DisplayControlspopover (shown whenviews.length > 1), with optional per-viewleadingIconandhideViewSwitcher/hideOrdering/hideGrouping/hideDisplayPropertiestoggles.