feat (DataTable): add anchored header for groups#686
Merged
paanSinghCoder merged 5 commits intomainfrom Mar 12, 2026
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR introduces a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
paanSinghCoder
changed the title
![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: 1
🧹 Nitpick comments (4)
packages/raystack/components/data-table/data-table.module.css (1)
172-179: Fixed header height assumption may be fragile.The
top: var(--rs-space-10)assumes a fixed table header height. If the header height varies (e.g., multi-line headers, different font sizes), the sticky section header may not align correctly under it. Consider whether this could be made more dynamic or documented as a constraint.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/raystack/components/data-table/data-table.module.css` around lines 172 - 179, .stickySectionHeader currently uses a fixed top: var(--rs-space-10) which breaks if the table header height changes; update the approach so the sticky section uses a dynamic header height: either introduce and use a dedicated CSS custom property (e.g. top: var(--table-header-height, var(--rs-space-10))) and ensure the table header sets --table-header-height to its computed height, or add a small script that measures the actual header element height (querySelector for the table header element) and sets the .stickySectionHeader style.top (or document.documentElement style.setProperty('--table-header-height', `${height}px`)) on mount and window resize; reference .stickySectionHeader and the new --table-header-height variable (or the header measurement logic) when applying the change.packages/raystack/components/data-table/components/virtualized-content.tsx (3)
341-354: Consider extracting duplicated group label rendering.The sticky anchor content (lines 347-352) duplicates the rendering logic from
VirtualGroupHeader(lines 74-78). Consider extracting the shared label+badge rendering into a reusable component:♻️ Suggested extraction
// Extract shared rendering function GroupLabelContent({ data }: { data: GroupedData<unknown> }) { return ( <Flex gap={3} align='center'> {data.label} {data.showGroupCount ? ( <Badge variant='neutral'>{data.count}</Badge> ) : null} </Flex> ); } // Then use in both places: // In VirtualGroupHeader: <div role='row' className={styles.virtualSectionHeader} style={style}> <GroupLabelContent data={data} /> </div> // In sticky anchor: {stickyGroupHeader && isGrouped && stickyGroup && ( <div role='row' className={styles.stickyGroupAnchor} style={{ top: headerHeight }}> <GroupLabelContent data={stickyGroup} /> </div> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/raystack/components/data-table/components/virtualized-content.tsx` around lines 341 - 354, The sticky group anchor duplicates the label+badge render logic found in VirtualGroupHeader; extract that shared markup into a small presentational component (e.g., GroupLabelContent) that accepts the grouped data shape and renders the label and optional Badge, then replace the inline JSX in both VirtualGroupHeader (use GroupLabelContent with the existing data prop) and the sticky anchor block (use GroupLabelContent with stickyGroup) so both places reference the same component and remove the duplicated code.
267-278: Performance:virtualizerin dependency array causes callback recreation on every render.
useVirtualizerreturns a new object reference each render, so includingvirtualizerin the dependency array meansupdateStickyGroupis recreated every render. This cascades tohandleVirtualScrolland theuseLayoutEffectat line 305-307, causing unnecessary work.Consider reading the virtualizer via a ref or accessing
getVirtualItems()directly within the scroll handler without memoizing this function separately:♻️ Suggested refactor using ref pattern
+ const virtualizerRef = useRef(virtualizer); + virtualizerRef.current = virtualizer; + const updateStickyGroup = useCallback(() => { if (!stickyGroupHeader || !isGrouped || groupHeaderList.length === 0) { setStickyGroup(null); return; } - const items = virtualizer.getVirtualItems(); + const items = virtualizerRef.current.getVirtualItems(); const firstIndex = items[0]?.index ?? 0; const current = groupHeaderList .filter(g => g.index <= firstIndex) .pop()?.data; setStickyGroup(current ?? null); - }, [stickyGroupHeader, isGrouped, groupHeaderList, virtualizer]); + }, [stickyGroupHeader, isGrouped, groupHeaderList]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/raystack/components/data-table/components/virtualized-content.tsx` around lines 267 - 278, The callback updateStickyGroup is being recreated every render because virtualizer (from useVirtualizer) is a new object reference each time; remove virtualizer from the dependency array and instead read virtualizer.getVirtualItems() via a stable ref or directly inside the scroll handler so updateStickyGroup only depends on stable values (stickyGroupHeader, isGrouped, groupHeaderList). Concretely: create a ref (e.g., virtualizerRef) and assign the virtualizer to it after initialization, update updateStickyGroup to call virtualizerRef.current?.getVirtualItems() rather than referencing virtualizer in the closure, and update any consumers (handleVirtualScroll and the useLayoutEffect) to use the ref so the callbacks are no longer recreated each render.
305-307: Minor: Redundant dependencies in effect.
groupHeaderListandisGroupedare already captured inupdateStickyGroup's dependency array, making them redundant here. If the callback recreation issue from the previous comment is addressed, you can simplify:✨ Suggested simplification
useLayoutEffect(() => { if (stickyGroupHeader) updateStickyGroup(); - }, [stickyGroupHeader, updateStickyGroup, groupHeaderList, isGrouped]); + }, [stickyGroupHeader, updateStickyGroup]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/raystack/components/data-table/components/virtualized-content.tsx` around lines 305 - 307, The useLayoutEffect dependency array includes redundant entries: remove groupHeaderList and isGrouped from the dependencies since they are already captured by the updateStickyGroup callback; keep stickyGroupHeader and updateStickyGroup in the useLayoutEffect dependency list (and ensure updateStickyGroup's own dependencies correctly include groupHeaderList and isGrouped) so the effect only runs when stickyGroupHeader or the callback changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/www/src/content/docs/components/datatable/props.ts`:
- Around line 39-43: Update the JSDoc for the grouping sticky-label prop in
apps/www/src/content/docs/components/datatable/props.ts so the human-readable
sentence matches the actual default: remove or correct the "(default)" token in
the phrase "When true (default)" and instead state the behavior with the
true/false semantics and keep the `@defaultValue` false unchanged; ensure the
description reads something like "When true, the current group label sticks
under the table header while scrolling (anchor group title). `@defaultValue`
false" so docs and implementation align.
---
Nitpick comments:
In `@packages/raystack/components/data-table/components/virtualized-content.tsx`:
- Around line 341-354: The sticky group anchor duplicates the label+badge render
logic found in VirtualGroupHeader; extract that shared markup into a small
presentational component (e.g., GroupLabelContent) that accepts the grouped data
shape and renders the label and optional Badge, then replace the inline JSX in
both VirtualGroupHeader (use GroupLabelContent with the existing data prop) and
the sticky anchor block (use GroupLabelContent with stickyGroup) so both places
reference the same component and remove the duplicated code.
- Around line 267-278: The callback updateStickyGroup is being recreated every
render because virtualizer (from useVirtualizer) is a new object reference each
time; remove virtualizer from the dependency array and instead read
virtualizer.getVirtualItems() via a stable ref or directly inside the scroll
handler so updateStickyGroup only depends on stable values (stickyGroupHeader,
isGrouped, groupHeaderList). Concretely: create a ref (e.g., virtualizerRef) and
assign the virtualizer to it after initialization, update updateStickyGroup to
call virtualizerRef.current?.getVirtualItems() rather than referencing
virtualizer in the closure, and update any consumers (handleVirtualScroll and
the useLayoutEffect) to use the ref so the callbacks are no longer recreated
each render.
- Around line 305-307: The useLayoutEffect dependency array includes redundant
entries: remove groupHeaderList and isGrouped from the dependencies since they
are already captured by the updateStickyGroup callback; keep stickyGroupHeader
and updateStickyGroup in the useLayoutEffect dependency list (and ensure
updateStickyGroup's own dependencies correctly include groupHeaderList and
isGrouped) so the effect only runs when stickyGroupHeader or the callback
changes.
In `@packages/raystack/components/data-table/data-table.module.css`:
- Around line 172-179: .stickySectionHeader currently uses a fixed top:
var(--rs-space-10) which breaks if the table header height changes; update the
approach so the sticky section uses a dynamic header height: either introduce
and use a dedicated CSS custom property (e.g. top: var(--table-header-height,
var(--rs-space-10))) and ensure the table header sets --table-header-height to
its computed height, or add a small script that measures the actual header
element height (querySelector for the table header element) and sets the
.stickySectionHeader style.top (or document.documentElement
style.setProperty('--table-header-height', `${height}px`)) on mount and window
resize; reference .stickySectionHeader and the new --table-header-height
variable (or the header measurement logic) when applying the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cb45d7be-42cc-4ee9-9ce5-155e6f27122c
📒 Files selected for processing (9)
apps/www/src/app/examples/datatable/page.tsxapps/www/src/content/docs/components/datatable/index.mdxapps/www/src/content/docs/components/datatable/props.tspackages/raystack/components/data-table/__tests__/data-table.test.tsxpackages/raystack/components/data-table/components/content.tsxpackages/raystack/components/data-table/components/virtualized-content.tsxpackages/raystack/components/data-table/data-table.module.csspackages/raystack/components/data-table/data-table.tsxpackages/raystack/components/data-table/data-table.types.tsx
rsbh
approved these changes
Mar 12, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/www/src/content/docs/components/datatable/index.mdx (1)
219-230: Make the example show the anchored header immediately.Right now the snippet only flips
stickyGroupHeader; it never enters a grouped state, so readers still need extra setup before anything sticky appears. If this section is meant to build on thecategorygrouping example above, seedquery.group_byhere (or show the grouping controls) so the behavior is visible on first render.📝 One way to make the example self-demonstrating
<DataTable data={data} columns={columns} + query={{ group_by: ["category"] }} defaultSort={{ name: "name", order: "asc" }} - stickyGroupHeader={true} + stickyGroupHeader >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/www/src/content/docs/components/datatable/index.mdx` around lines 219 - 230, The example toggles stickyGroupHeader but never enables grouping, so nothing appears anchored; update the DataTable instantiation (the snippet using DataTable, DataTable.Content, DataTable.VirtualizedContent and stickyGroupHeader) to include a grouping configuration—for example set query.group_by to the grouping key used elsewhere (e.g., "category") or pre-seed the data with a grouped field—so the table renders in a grouped state on first render and the anchored group header is visible immediately.packages/raystack/components/data-table/data-table.module.css (1)
173-176: Avoid a fixed sticky offset here.Line 175 assumes the header is always
var(--rs-space-10)tall. Since consumers can restyle header cells, this can drift under supported customizations and either overlap the header or leave a visible gap. Prefer sourcing the offset from a measured or CSS-variable header height instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/raystack/components/data-table/data-table.module.css` around lines 173 - 176, The .stickySectionHeader rule uses a fixed top: var(--rs-space-10) which can break when consumers restyle headers; change this to derive the offset from a header-height CSS variable or a measured value at runtime instead. Update the .stickySectionHeader selector to use a fallback CSS var (e.g. top: var(--data-table-header-height, var(--rs-space-10))) and/or add code in the DataTable component (e.g. on mount in the component that renders the header) to measure the actual header element height and set a --data-table-header-height custom property (or set style.top) so the sticky offset always matches the rendered header height. Ensure you reference .stickySectionHeader and the header element/class used by the DataTable when implementing the measurement and setting the variable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/raystack/components/data-table/data-table.tsx`:
- Around line 51-52: In the props destructuring for the DataTable component, add
the missing comma between stickyGroupHeader = false and getRowId to fix the
syntax error; locate the destructured parameter where stickyGroupHeader and
getRowId are listed (in data-table.tsx) and insert the comma so the
destructuring reads with stickyGroupHeader = false, getRowId, ensuring the
component compiles.
---
Nitpick comments:
In `@apps/www/src/content/docs/components/datatable/index.mdx`:
- Around line 219-230: The example toggles stickyGroupHeader but never enables
grouping, so nothing appears anchored; update the DataTable instantiation (the
snippet using DataTable, DataTable.Content, DataTable.VirtualizedContent and
stickyGroupHeader) to include a grouping configuration—for example set
query.group_by to the grouping key used elsewhere (e.g., "category") or pre-seed
the data with a grouped field—so the table renders in a grouped state on first
render and the anchored group header is visible immediately.
In `@packages/raystack/components/data-table/data-table.module.css`:
- Around line 173-176: The .stickySectionHeader rule uses a fixed top:
var(--rs-space-10) which can break when consumers restyle headers; change this
to derive the offset from a header-height CSS variable or a measured value at
runtime instead. Update the .stickySectionHeader selector to use a fallback CSS
var (e.g. top: var(--data-table-header-height, var(--rs-space-10))) and/or add
code in the DataTable component (e.g. on mount in the component that renders the
header) to measure the actual header element height and set a
--data-table-header-height custom property (or set style.top) so the sticky
offset always matches the rendered header height. Ensure you reference
.stickySectionHeader and the header element/class used by the DataTable when
implementing the measurement and setting the variable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6465f69f-daf1-42e2-b5af-1a556e9fd935
📒 Files selected for processing (8)
apps/www/src/content/docs/components/datatable/index.mdxapps/www/src/content/docs/components/datatable/props.tspackages/raystack/components/data-table/__tests__/data-table.test.tsxpackages/raystack/components/data-table/components/content.tsxpackages/raystack/components/data-table/components/virtualized-content.tsxpackages/raystack/components/data-table/data-table.module.csspackages/raystack/components/data-table/data-table.tsxpackages/raystack/components/data-table/data-table.types.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/raystack/components/data-table/data-table.types.tsx
- packages/raystack/components/data-table/tests/data-table.test.tsx
- packages/raystack/components/data-table/components/virtualized-content.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
feat (data-table): add anchored group header. Toggled with
stickyGroupHeader. DefaultfalseTest it here: https://apsara-git-feat-anchored-group-header-raystack.vercel.app/examples/datatable
Screen.Recording.2026-03-10.at.4.36.37.PM.mov
Type of Change
How Has This Been Tested?
[Describe the tests that you ran to verify your changes]
Checklist:
Screenshots (if appropriate):
[Add screenshots here]
Related Issues
[Link any related issues here using #issue-number]
Summary by CodeRabbit
New Features
Documentation
Style