Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
620 changes: 620 additions & 0 deletions apps/www/src/app/examples/dataview/page.tsx

Large diffs are not rendered by default.

596 changes: 596 additions & 0 deletions docs/rfcs/002-unified-dataview-component.md

Large diffs are not rendered by default.

407 changes: 407 additions & 0 deletions packages/raystack/components/data-view-beta/components/content.tsx

Large diffs are not rendered by default.

44 changes: 44 additions & 0 deletions packages/raystack/components/data-view-beta/components/custom.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
'use client';

import { ReactNode, useEffect } from 'react';
import { DataViewContextType, DataViewField } from '../data-view.types';
import { useDataView } from '../hooks/useDataView';

export interface DataViewCustomProps<TData> {
/** Multi-view name. When set, the renderer gates itself on the active view. */
name?: string;
/** Optional view-scoped field override. Full replacement of root `fields` for this view's active session. */
fields?: DataViewField<TData>[];
/**
* Render prop that receives the full DataView context (table, fields,
* tableQuery, hasData, isEmptyState, etc.) and returns the rendered view.
* Pair with `<DataView.DisplayAccess>` to keep field visibility in sync with
* the single Display Properties toggle.
*/
children: (context: DataViewContextType<TData>) => ReactNode;
}

/**
* Escape-hatch renderer for free-form views (cards, kanban, map, etc.).
* Consumes the DataView context and hands it to a render prop.
*/
export function DataViewCustom<TData>({
name,
fields: fieldsOverride,
children
}: DataViewCustomProps<TData>) {
const ctx = useDataView<TData>();
const { activeView, registerFieldsForView } = ctx;

useEffect(() => {
if (!name || !fieldsOverride) return;
return registerFieldsForView(name, fieldsOverride);
}, [name, fieldsOverride, registerFieldsForView]);
Comment on lines +33 to +36
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check whether registerFieldsForView is wrapped in useCallback in the provider
rg -n "registerFieldsForView" --type=tsx --type=ts -A 3 -B 3

Repository: raystack/apsara

Length of output: 87


🏁 Script executed:

# Search for registerFieldsForView with corrected flags
rg -n "registerFieldsForView" -A 3 -B 3 --type ts

Repository: raystack/apsara

Length of output: 5403


🏁 Script executed:

# Also search more broadly without type restriction to find all occurrences
rg -n "registerFieldsForView" -A 3 -B 3

Repository: raystack/apsara

Length of output: 5403


🏁 Script executed:

# Locate the DataViewCustom component file mentioned in the review
find . -name "custom.tsx" -path "*/data-view-beta/*"

Repository: raystack/apsara

Length of output: 127


🏁 Script executed:

cat -n packages/raystack/components/data-view-beta/data-view.tsx | sed -n '85,105p'

Repository: raystack/apsara

Length of output: 831


🏁 Script executed:

cat -n packages/raystack/components/data-view-beta/data-view.tsx | sed -n '91,110p'

Repository: raystack/apsara

Length of output: 791


🏁 Script executed:

cat -n packages/raystack/components/data-view-beta/components/custom.tsx | head -50

Repository: raystack/apsara

Length of output: 1913


fieldsOverride array reference instability will cause the effect to re-fire on every parent render.

fieldsOverride is DataViewField<TData>[]. Arrays are compared by reference in JavaScript, so passing an inline array literal — the natural usage pattern:

<DataView.Custom name="cards" fields={[{ accessorKey: 'name', ... }]}>

…creates a new array on every parent render. React's useEffect sees a changed dependency and re-runs, calling registerFieldsForView on every render cycle. This causes unnecessary field registration and cleanup cycles.

The fix is to require consumers to memoize fieldsOverride. Add a prominent JSDoc warning:

 export interface DataViewCustomProps<TData> {
   /** Multi-view name. When set, the renderer gates itself on the active view. */
   name?: string;
-  /** Optional view-scoped field override. Full replacement of root `fields` for this view's active session. */
+  /**
+   * Optional view-scoped field override. Full replacement of root `fields` for this view's active session.
+   * **Must be referentially stable** (e.g. defined outside the render function or wrapped in `useMemo`)
+   * to prevent the registration effect from re-firing on every render.
+   */
   fields?: DataViewField<TData>[];

Alternatively, guard the effect with a ref-based deep equality check to skip redundant registrations when field definitions haven't semantically changed:

♻️ Alternative fix using a ref guard
+import { ReactNode, useEffect, useRef } from 'react';
+import { isEqual } from 'some-deep-equal-utility'; // or JSON.stringify comparison
 
 export function DataViewCustom<TData>({
   name,
   fields: fieldsOverride,
   children
 }: DataViewCustomProps<TData>) {
   const ctx = useDataView<TData>();
   const { activeView, registerFieldsForView } = ctx;
+  const prevFieldsRef = useRef<DataViewField<TData>[] | undefined>(undefined);
 
   useEffect(() => {
     if (!name || !fieldsOverride) return;
+    if (isEqual(prevFieldsRef.current, fieldsOverride)) return;
+    prevFieldsRef.current = fieldsOverride;
     return registerFieldsForView(name, fieldsOverride);
   }, [name, fieldsOverride, registerFieldsForView]);
🤖 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-beta/components/custom.tsx` around
lines 33 - 36, The effect depends on the array prop fieldsOverride which will
re-trigger on every parent render if passed inline; either require consumers to
provide a stable/memoized array (add a prominent JSDoc on the fieldsOverride
prop in DataView.Custom saying “memoize this with useMemo or keep stable
reference”) or add a ref-based guard inside the useEffect in this file: keep
previousFields in a useRef, perform a deep-equality check (or stable key
compare) between previousFields.current and fieldsOverride and only call
registerFieldsForView(name, fieldsOverride) (and update previousFields.current)
when they differ; reference fieldsOverride, useEffect, registerFieldsForView and
the DataView.Custom fields prop when making the change.


const isActive = !name || activeView === undefined || activeView === name;
if (!isActive) return null;

return <>{children(ctx)}</>;
}

DataViewCustom.displayName = 'DataView.Custom';
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
'use client';

import { ReactNode } from 'react';
import { useDataView } from '../hooks/useDataView';

export interface DataViewDisplayAccessProps {
/** Field (column) accessor key. Gates rendering on the column's current visibility state. */
accessorKey: string;
children: ReactNode;
/** Rendered when the referenced field is currently hidden. Defaults to null. */
fallback?: ReactNode;
}

/**
* Gates children on the current column visibility state from DataView context.
* Use inside free-form renderers (Timeline bars, custom renderers, cell overrides)
* so the single DisplayControls toggle reaches the same visibility story that
* Table/List rows get through their column specs.
*/
export function DisplayAccess({
accessorKey,
children,
fallback = null
}: DataViewDisplayAccessProps) {
const { table } = useDataView();
const column = table?.getColumn(accessorKey);
// If the column doesn't exist, default to visible so consumers can wrap JSX
// in DisplayAccess without worrying about typos silently breaking the render.
const isVisible = column ? column.getIsVisible() : true;
return <>{isVisible ? children : fallback}</>;
}

DisplayAccess.displayName = 'DataView.DisplayAccess';
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
'use client';

import { Chip } from '../../chip';
import { Flex } from '../../flex';
import { Text } from '../../text';
import { DataViewField } from '../data-view.types';
import { useDataView } from '../hooks/useDataView';

export function DisplayProperties<TData>({
fields
}: {
fields: DataViewField<TData>[];
}) {
const { table } = useDataView<TData>();
const hidableFields = fields?.filter(f => f.hideable) ?? [];

return (
<Flex direction='column' gap={3}>
<Text>Display Properties</Text>
<Flex gap={3} wrap='wrap'>
{hidableFields.map(field => {
const column = table.getColumn(field.accessorKey);
const isVisible = column ? column.getIsVisible() : true;
return (
<Chip
key={field.accessorKey}
variant='outline'
size='small'
color={isVisible ? 'accent' : 'neutral'}
onClick={() => column?.toggleVisibility()}
>
{field.label}
</Chip>
);
})}
</Flex>
</Flex>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
'use client';

import { MixerHorizontalIcon } from '@radix-ui/react-icons';

import { isValidElement, ReactNode } from 'react';
import { Button } from '../../button';
import { Flex } from '../../flex';
import { Popover } from '../../popover';
import styles from '../data-view.module.css';
import { defaultGroupOption, SortOrdersValues } from '../data-view.types';
import { useDataView } from '../hooks/useDataView';
import { DisplayProperties } from './display-properties';
import { Grouping } from './grouping';
import { Ordering } from './ordering';
import { ViewSwitcher } from './view-switcher';

interface DisplaySettingsProps {
trigger?: ReactNode;
}

export function DisplaySettings<TData>({
trigger = (
<Button
variant='outline'
color='neutral'
size='small'
leadingIcon={<MixerHorizontalIcon />}
>
Display
</Button>
)
}: DisplaySettingsProps) {
const {
fields,
updateTableQuery,
tableQuery,
defaultSort,
onDisplaySettingsReset,
views
} = useDataView<TData>();

const sortableColumns = (fields ?? [])
.filter(f => f.sortable)
.map(f => ({
label: f.label,
id: f.accessorKey
}));

function onSortChange(columnId: string, order: SortOrdersValues) {
updateTableQuery(query => {
return {
...query,
sort: [{ name: columnId, order }]
};
});
}

function onGroupChange(columnId: string) {
updateTableQuery(query => {
return {
...query,
group_by: [columnId]
};
});
}

function onGroupRemove() {
updateTableQuery(query => {
return {
...query,
group_by: []
};
});
}

function onReset() {
onDisplaySettingsReset();
}

const showViewSwitcher = (views?.length ?? 0) > 1;

return (
<Popover>
<Popover.Trigger
render={isValidElement(trigger) ? trigger : <button>{trigger}</button>}
/>
<Popover.Content
className={styles['display-popover-content']}
align='end'
>
<Flex direction='column'>
{showViewSwitcher ? (
<Flex className={styles['display-popover-properties-container']}>
<ViewSwitcher />
</Flex>
) : null}
<Flex
direction='column'
className={styles['display-popover-properties-container']}
gap={5}
>
<Ordering
columnList={sortableColumns}
onChange={onSortChange}
value={tableQuery?.sort?.[0] || defaultSort}
/>
<Grouping
fields={fields ?? []}
onRemove={onGroupRemove}
onChange={onGroupChange}
value={tableQuery?.group_by?.[0] || defaultGroupOption.id}
/>
Comment on lines +107 to +112
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Filter grouping options by groupable.

Grouping currently receives every field, so non-groupable fields can still be selected. Mirror the sortable filtering and pass only fields.filter(f => f.groupable).

Suggested fix
+  const groupableFields = (fields ?? []).filter(f => f.groupable);
+
   return (
@@
             <Grouping
-              fields={fields ?? []}
+              fields={groupableFields}
               onRemove={onGroupRemove}
               onChange={onGroupChange}
               value={tableQuery?.group_by?.[0] || defaultGroupOption.id}
📝 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
<Grouping
fields={fields ?? []}
onRemove={onGroupRemove}
onChange={onGroupChange}
value={tableQuery?.group_by?.[0] || defaultGroupOption.id}
/>
const groupableFields = (fields ?? []).filter(f => f.groupable);
<Grouping
fields={groupableFields}
onRemove={onGroupRemove}
onChange={onGroupChange}
value={tableQuery?.group_by?.[0] || defaultGroupOption.id}
/>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/raystack/components/data-view/components/display-settings.tsx`
around lines 98 - 103, Grouping is being passed all fields so non-groupable
options appear; update the props to pass only groupable fields by replacing the
fields prop on the Grouping component with fields?.filter(f => f.groupable) (use
the same null-safe pattern used elsewhere), keeping value
(tableQuery?.group_by?.[0] || defaultGroupOption.id) and handlers
onRemove/onChange unchanged so Grouping only receives groupable fields.

</Flex>
<Flex className={styles['display-popover-properties-container']}>
<DisplayProperties fields={fields ?? []} />
</Flex>
<Flex
justify='end'
className={styles['display-popover-reset-container']}
>
<Button variant='text' onClick={onReset} color='neutral'>
Reset to default
</Button>
</Flex>
</Flex>
</Popover.Content>
</Popover>
);
}

DisplaySettings.displayName = 'DataView.DisplayControls';
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
'use client';

import { cx } from 'class-variance-authority';
import { ReactNode } from 'react';
import styles from '../data-view.module.css';
import { useDataView } from '../hooks/useDataView';

export interface DataViewEmptyStateProps {
/** Restrict to a specific view's `name`. When set, the EmptyState only renders if both `isEmptyState` is true AND the active view matches. */
forView?: string;
className?: string;
children: ReactNode;
}

/**
* Renders its children when the current data + query result in an empty state
* (i.e., a query is active but no rows match). Reads `isEmptyState` from
* DataView context, so the empty/zero distinction is computed in one place.
*/
export function DataViewEmptyState({
forView,
className,
children
}: DataViewEmptyStateProps) {
const { isEmptyState, activeView } = useDataView();
if (!isEmptyState) return null;
if (forView && activeView !== forView) return null;
return (
<div className={cx(styles.dataStateContainer, className)}>{children}</div>
);
}

DataViewEmptyState.displayName = 'DataView.EmptyState';
Loading
Loading