Repository navigation
Conversation
…amples The React, Vue, Svelte and Angular table examples moved each row with translateY inside a div sized to getTotalSize(). The <table> itself only grew to the height of the rendered rows, so a sticky header scrolled away once the rows were translated past the end of the table. Render a spacer row above and below the visible rows instead, so the table is as tall as all of its rows, and make the header sticky. Also collapse the default border spacing so rows are exactly the estimated 34px, set explicit row heights in the Vue example, and turn off overflow-anchor on the scroll container. Fixes TanStack#585
|
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Spacer-row rendering examples/angular/table/src/app/app.component.ts, examples/react/table/src/main.tsx, examples/react/table/src/index.css, examples/svelte/table/src/App.svelte, examples/vue/table/src/App.vue, examples/vue/table/src/style.css |
The examples calculate spacer heights from virtual row positions and render the visible rows between spacer rows. They remove the fixed-height wrapper and translated row positioning. Table styles add sticky headers, collapsed borders, and disabled scroll anchoring. Explanatory text describes the spacer-row approach. |
Priority:
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Bug fix · Severity of issue fixed: Low
Suggested reviewers: kouts, garrettld, marcusvirg
Merge Risk: 
990cd
-
No concrete merge-blocking issue remains in the updated table examples.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (4 skipped: 4 … Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Title check The title clearly and concisely identifies the main change: replacing translateY positioning with spacer rows in the table examples. Description check The description includes the required Changes, Checklist, and Release Impact sections. It explains the motivation, implementation details, testing results, and why pnpm run test:pr was not run. The un… Linked Issues check The active directly linked issue is #585. The React example now renders top and bottom spacer rows from the virtual range, removes translated rows and the fixed-height wrapper, and keeps the table siz…Out of Scope Changes check The changes to Vue, Svelte, and Angular apply the same table-sizing fix to equivalent virtual table examples. Their spacer rows, sticky headers, border handling, and scroll-anchor handling are directl… Full details: Description check
-
Explanation
The description includes the required Changes, Checklist, and Release Impact sections. It explains the motivation, implementation details, testing results, and why pnpm run test:pr was not run. The unchecked test requirement is documented with related checks and an unrelated failure on main.
Full details: Linked Issues check
-
Explanation
The active directly linked issue is
#585. The React example now renders top and bottom spacer rows from the virtual range, removes translated rows and the fixed-height wrapper, and keeps the table sized to the virtualized content. The React CSS adds a sticky header with a background, collapsed borders, and disabled scroll anchoring. These changes address the sticky-header and row-border requirements in#585. The PR summary also reports automated browser checks at multiple scroll positions, including the end of the table.
Full details: Out of Scope Changes check
-
Explanation
The changes to Vue, Svelte, and Angular apply the same table-sizing fix to equivalent virtual table examples. Their spacer rows, sticky headers, border handling, and scroll-anchor handling are directly connected to the
#585defect pattern. The updated explanatory text supports the changed implementation. No unrelated published code or unrelated feature changes are shown, and the Marko example remains unchanged.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (4 skipped: 4 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 help to get the list of available commands.
🎯 Changes
Fixes #585.
The React, Vue, Svelte and Angular table examples moved each row with
translateY(start - index * size)inside adivsized togetTotalSize(). The<table>was only as tall as the rendered rows (roughly 560 to 2,200px, against 1,700,000px of rows), so most rows were drawn outside it. That's why a sticky header scrolls away and row borders disappear, as described in the issue.Following the approach suggested in the issue thread and in #476, the examples now render a spacer row above and below the visible rows:
virtualRows[0].start. Bottom spacer:getTotalSize() - lastRow.end. Each spacer is a single<td colSpan>.divand the transforms are removed, so the table is exactly as tall as all of its rows.theadisposition: sticky; top: 0with a background.border-collapse: collapseon the table. With the default 2px border spacing each row took 36px while the virtualizer assumed 34px.overflow-anchor: noneon the scroll container, so scroll anchoring doesn't adjustscrollTopwhen the spacer heights change.height: virtualRow.sizelike the other examples. Before, they rendered at their natural height (around 20px), which left blank space at the bottom of the viewport.box-sizing: border-box, so the spacer cells setpadding: 0to keep their height exact.The Marko table example already sizes its table to the total size, so I didn't change it. There are no Solid or Lit table examples. Most of the diff is re-indentation from removing the wrapper
div(git diff -wis 169+/56-).Testing: I ran each example in both the dev server and the production build. I checked each one with a headless Playwright script in Chromium, Firefox and WebKit at scroll offsets 0, 3,400, 100,017, 850,000, 1,234,567 and the max. At each offset the script checked that:
(id - 1) * 34pxfrom the top of thetbodyIt also wheel-scrolled (
scrollToponly increased and the header stayed pinned), sorted by ID, and jumped to the end. All four examples passed in all three browsers. Onmainthe same script fails on table height and the sticky header.✅ Checklist
pnpm run test:pr. (test:sherif,test:docsand thebuildtargets for the four examples pass.test:knipfails for me on an unmodifiedmaincheckout too, flaggingscripts/verify-*.tsandmarkdown-link-extractor, so it's unrelated to this change.)🚀 Release Impact
Happy to take suggested changes, for example putting the sticky styles on
thinstead ofthead, or splitting the non-React examples into a separate PR.Summary by CodeRabbit