fix: smooth horizontal scrolling in wide data tables - #1544
Conversation
Fixes #1476 (duplicate: #1514). Three defects in center-column virtualization made horizontal scrolling in wide tables jumpy, made the last column unreachable, and snapped the viewport left when clicking a cell: - The virtualization window was derived from a raw scroll-position state that was updated behind a requestAnimationFrame hop, so every scrolled pixel re-rendered the whole grid while the mounted window lagged the real scroll position, showing blank columns and jumpy repaints. The window is now computed synchronously in the scroll/resize handlers and stored in state only when the set of mounted columns actually changes. - The focused-cell auto-scroll effect re-ran on every scroll update while the focused cell's element was not rendered (focused column outside the virtualization window, or focused row not on the current page), re-applying a column-aligned scrollLeft each time and fighting user scrolling. It now runs at most once per focused-cell change and falls back to any rendered cell in the focused row for the vertical reveal. - The window was computed from scrollLeft shifted left by the pinned-column width, under-rendering columns at the right viewport edge. Sticky pinned columns occupy as much viewport as they occupy row start, so container scrollLeft maps 1:1 onto center-column offsets. Verified in the ppg demo against the 61-column all_data_types table: the window follows scrolling to the very end, the last column renders fully, and clicking or focusing cells no longer moves the viewport. Regression tests cover the auto-scroll fight, animation-frame-free window updates, and end-of-grid window coverage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 41 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughUpdated wide-table column virtualization to compute synchronously from scroll, resize, and layout changes while storing only changed virtualization windows. Added virtualization window constants and equality helpers, corrected maximum-scroll coverage, and adjusted focused-cell auto-scroll to run once per focus change with a rendered-row fallback. Added interaction and virtualization tests plus documentation and a patch changeset. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
|
Compute preview deployed. Branch: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 2
🤖 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 `@ui/studio/grid/DataGrid.tsx`:
- Around line 1810-1815: Update the focused-cell auto-scroll effect around
autoScrolledFocusedCellRef so it does not mark a focus change handled until the
container, column, and usable viewport are available. Preserve the
once-per-focus behavior after scrolling is actionable, while allowing hidden or
not-yet-laid-out grids to retry when column/layout readiness changes.
- Around line 1209-1211: Update the centerVirtualizationInputsKey construction
to serialize each column’s id and size as structured tuples, including
leftPinnedWidth and rightPinnedWidth, instead of joining delimiter-based
strings. Ensure distinct column/width inputs always produce distinct keys so
virtualization state is not reused with stale widths.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 0f043c8e-b2cc-441d-b47f-df0a60fc6b24
📒 Files selected for processing (8)
.changeset/smooth-horizontal-scroll.mdArchitecture/wide-grid-performance.mdFEATURES.mdui/studio/grid/DataGrid.interactions.test.tsxui/studio/grid/DataGrid.tsxui/studio/grid/DataGrid.virtualization.test.tsxui/studio/grid/column-virtualization.test.tsui/studio/grid/column-virtualization.ts
- Serialize the center-column virtualization inputs key structurally with JSON.stringify instead of delimiter joining, so column ids containing the delimiter characters cannot produce colliding keys. - Keep a focused-cell auto-scroll pending while its prerequisites are missing (no scroll container, unknown column, or an unmeasured/hidden grid) instead of marking it handled up-front. Column changes and a new viewport-readiness tick re-trigger the effect when layout becomes ready; neither signal changes on plain scrolling, so the at-most-once-per-focus guarantee against fighting user scrolling is preserved. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 participant
Fixes #1476, fixes #1514
Root cause
Three interacting defects in the DataGrid's center-column virtualization:
scroll event → rAF → setState, re-rendering the whole grid on every scrolled pixel while the mounted column window trailed the real scroll position — outrunning the overscan and producing blank columns and jumpy repaints on wide (large JSON) columns.scrollLeftback toward the focused column — this is the "can never reach the end" / "clicking a cell jumps the viewport back" symptom.scrollLeft - leftPinnedWidth, shifting it left by the pinned width and under-rendering the right viewport edge.Fix
scrollLeft/clientWidth; store only the resolved window, and only when it changes — scrolling inside the overscan no longer re-renders the grid.Verification
scrollLeft), focusing a far-right cell leavesscrollLeftuntouched, and scrolling away from a focused cell is never yanked back.expected 2000 to be 2600); window-update-on-scroll test with rAF stubbed out; end-of-grid window coverage.Note: shares
DataGrid.tsxwith the smaller pin-reset fix PR (#1371 branch); whichever lands second needs a trivial rebase.🤖 Generated with Claude Code