Add virtual scrolling to StreamsTable for large stream lists.#557
Merged
ritik4ever merged 1 commit intoJun 1, 2026
Merged
Conversation
Window row rendering with @tanstack/react-virtual keeps DOM size bounded when 500+ streams are loaded, preserving keyboard focus and expandable timeline rows.
|
@testersweb0-bug is attempting to deploy a commit to the ritik4ever's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@testersweb0-bug Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
📝 WalkthroughWalkthroughThis PR implements virtual scrolling for StreamsTable using ChangesVirtual Scrolling Implementation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 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. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/src/components/StreamsTable.test.tsx (1)
95-143: ⚡ Quick winAdd a threshold-boundary test for virtualization toggle (49 vs 50 rows).
Current tests validate the large-list path, but not the boundary where virtualization should switch on/off. A focused boundary test would protect the contract from regressions with minimal cost.
Suggested test addition
describe("StreamsTable virtual scrolling", () => { + it("does not virtualize below threshold and virtualizes at threshold", () => { + const belowThreshold = Array.from({ length: 49 }, (_, i) => + createMockStream(String(i + 1).padStart(4, "0")), + ); + const atThreshold = Array.from({ length: 50 }, (_, i) => + createMockStream(String(i + 1).padStart(4, "0")), + ); + + const view = render(<StreamsTable {...defaultProps} streams={belowThreshold} />); + expect( + screen.getAllByRole("checkbox", { name: /^Select stream / }).length, + ).toBe(49); + + setScrollViewport(screen.getByTestId("streams-table-scroll"), 400); + view.rerender(<StreamsTable {...defaultProps} streams={atThreshold} />); + expect( + screen.getAllByRole("checkbox", { name: /^Select stream / }).length, + ).toBeLessThan(50); + });🤖 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 `@frontend/src/components/StreamsTable.test.tsx` around lines 95 - 143, Add a boundary test that verifies virtualization toggles between 49 and 50 rows: create two datasets using createMockStream (length 49 and length 50), render StreamsTable with each, call setScrollViewport(screen.getByTestId("streams-table-scroll"), 400) and rerender as the large-list test does, then assert for 49 items that the number of rendered checkbox rows (screen.getAllByRole("checkbox", { name: /^Select stream / })) equals 49 (no virtualization) and for 50 items that the rendered count is less than 50 and <= Math.ceil(400 / 52) + STREAMS_TABLE_VIRTUAL_OVERSCAN + 2 (virtualized); add this as a new it block in StreamsTable.test.tsx.
🤖 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 `@frontend/src/components/StreamsTable.tsx`:
- Around line 294-299: Clamp paddingBottom to a non-negative value to prevent
negative spacing when measured heights exceed estimates: compute the existing
paddingBottom using resolvedVirtualRows and rowVirtualizer.getTotalSize() (as
currently done) and then replace it with Math.max(0, paddingBottom) (or
equivalent) before it’s used in rendering; update the variable named
paddingBottom in StreamsTable (where resolvedVirtualRows and
rowVirtualizer.getTotalSize() are referenced) so any temporary negative result
becomes 0.
---
Nitpick comments:
In `@frontend/src/components/StreamsTable.test.tsx`:
- Around line 95-143: Add a boundary test that verifies virtualization toggles
between 49 and 50 rows: create two datasets using createMockStream (length 49
and length 50), render StreamsTable with each, call
setScrollViewport(screen.getByTestId("streams-table-scroll"), 400) and rerender
as the large-list test does, then assert for 49 items that the number of
rendered checkbox rows (screen.getAllByRole("checkbox", { name: /^Select stream
/ })) equals 49 (no virtualization) and for 50 items that the rendered count is
less than 50 and <= Math.ceil(400 / 52) + STREAMS_TABLE_VIRTUAL_OVERSCAN + 2
(virtualized); add this as a new it block in StreamsTable.test.tsx.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1aa41fcd-22fc-4ba7-8409-0338898bbc7b
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
frontend/package.jsonfrontend/src/components/StreamsTable.test.tsxfrontend/src/components/StreamsTable.tsxfrontend/src/index.css
Comment on lines
+294
to
+299
| const paddingTop = | ||
| resolvedVirtualRows.length > 0 ? resolvedVirtualRows[0].start : 0; | ||
| const paddingBottom = | ||
| resolvedVirtualRows.length > 0 | ||
| ? rowVirtualizer.getTotalSize() - resolvedVirtualRows[resolvedVirtualRows.length - 1].end | ||
| : 0; |
There was a problem hiding this comment.
Guard against negative paddingBottom.
If measured row heights exceed estimates during recalculation, getTotalSize() could temporarily be smaller than the last row's end, resulting in negative padding. CSS treats negative heights as 0, but explicit clamping is clearer and avoids layout edge cases.
🛡️ Proposed fix
const paddingBottom =
resolvedVirtualRows.length > 0
- ? rowVirtualizer.getTotalSize() - resolvedVirtualRows[resolvedVirtualRows.length - 1].end
+ ? Math.max(0, rowVirtualizer.getTotalSize() - resolvedVirtualRows[resolvedVirtualRows.length - 1].end)
: 0;📝 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 paddingTop = | |
| resolvedVirtualRows.length > 0 ? resolvedVirtualRows[0].start : 0; | |
| const paddingBottom = | |
| resolvedVirtualRows.length > 0 | |
| ? rowVirtualizer.getTotalSize() - resolvedVirtualRows[resolvedVirtualRows.length - 1].end | |
| : 0; | |
| const paddingTop = | |
| resolvedVirtualRows.length > 0 ? resolvedVirtualRows[0].start : 0; | |
| const paddingBottom = | |
| resolvedVirtualRows.length > 0 | |
| ? Math.max(0, rowVirtualizer.getTotalSize() - resolvedVirtualRows[resolvedVirtualRows.length - 1].end) | |
| : 0; |
🤖 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 `@frontend/src/components/StreamsTable.tsx` around lines 294 - 299, Clamp
paddingBottom to a non-negative value to prevent negative spacing when measured
heights exceed estimates: compute the existing paddingBottom using
resolvedVirtualRows and rowVirtualizer.getTotalSize() (as currently done) and
then replace it with Math.max(0, paddingBottom) (or equivalent) before it’s used
in rendering; update the variable named paddingBottom in StreamsTable (where
resolvedVirtualRows and rowVirtualizer.getTotalSize() are referenced) so any
temporary negative result becomes 0.
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.
Summary
Fixes #448 by adding virtual scrolling to
StreamsTableusing@tanstack/react-virtual. When 50+ streams are loaded, only rows in the visible viewport plus 5 overscan rows are mounted in the DOM, eliminating scroll jank and DOM bloat with 500+ streams.@tanstack/react-virtualwith a bounded scroll container and sticky table headerSTREAMS_TABLE_VIRTUAL_OVERSCAN = 5for testabilityAcceptance criteria
StreamRowinstances with 500 streamsTest plan
npm test -- --run src/components/StreamsTable.test.tsx— all 6 tests passStreamRowinstances instead of 500Summary by CodeRabbit
New Features
Tests