Skip to content

Conversation

@severo
Copy link
Contributor

@severo severo commented Jan 2, 2026

It replaces #347 (the remaining parts). The current commit will be recreated later, but I wanted to save my work.

remaining:

Later?

  • add tests (with playwright / vitest browser?)
  • handle edge cases (eg. row / header height > viewport height)
Screencast.From.2026-01-03.01-35-13.mp4

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This WIP pull request implements virtual scrolling for the HighTable component to handle large datasets efficiently. The implementation introduces coordinate transformation between physical canvas height and virtual canvas height, along with custom scroll handling logic. The PR is currently broken due to an update loop issue as noted in the title.

Key changes:

  • Implements virtual scroll provider with coordinate transformation between canvas and virtual canvas spaces
  • Adds IntersectionObserver-based focus management for cells in virtual scroll mode
  • Updates scroll behavior to support options parameter for smooth/instant scrolling

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 13 comments.

Show a summary per file
File Description
src/providers/ScrollModeVirtualProvider.tsx Main implementation of virtual scrolling logic with state management, coordinate transformations, and scroll-to-row functionality
src/providers/ScrollModeProvider.tsx Passes required props (numRows, headerHeight) to ScrollModeVirtualProvider
src/hooks/useCellFocus.ts Adds IntersectionObserver-based focus logic for virtual scroll mode with horizontal scroll handling
src/contexts/ScrollModeContext.ts Extends context interface with shouldScrollHorizontally flag and behavior options for scrollToTop
src/components/HighTable/Scroller.tsx Updates scrollTo implementation to accept behavior options (instant/smooth)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 9 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@severo severo requested a review from Copilot January 3, 2026 00:32
@severo severo marked this pull request as ready for review January 3, 2026 00:33
@severo severo changed the title [wip] replaces #347 - currently broken due to update loop Implement virtual scroll for large dataframes Jan 3, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 10 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@severo severo requested a review from platypii January 3, 2026 00:47
@severo
Copy link
Contributor Author

severo commented Jan 3, 2026

Finally, I have a valid PR for virtual scrolling of huge dataframes. Would you like to have a look @platypii?

@severo
Copy link
Contributor Author

severo commented Jan 7, 2026

The bottom border is not visible when scrolling past the end of the table (it only happens in "virtual scroll mode")

Screencast.From.2026-01-07.14-44-18.mp4

@platypii
Copy link
Contributor

platypii commented Jan 7, 2026

Still getting weird behavior: if I scroll down even one "notch" on my scroll wheel, I jump to row 4000+.

Worse: when I try to scroll back up, there's no way to get back to row 1

simplescreenrecorder-2026-01-07_12.06.49.mp4

@severo
Copy link
Contributor Author

severo commented Jan 8, 2026

Hmmm, I don't know how to change this while letting the browser natively handle the scroll.

The current logic is:

  • the scrollbar is only for global navigation. Ie: 1% scroll = 1% of the number of rows, which might mean a gap of 4000 rows, indeed. It is controlled by any scroll event received from the browser (mouse wheel, drag/drop of the scroll handle, click on the scroll bar), plus some keyboard events when the whole component is focused
  • local navigation is only available through the keyboard (arrows, pgup/pgdn).

I understand the UX is maybe not what we expected, but I don't think we would be able to do more with the current concepts/logic.

Alternatives:

  1. apply some kind of non-linear reaction to changes to the native 'scrollTop' value: if the change is small, act "locally", if it's big, act "globally". I can try this, but 1. I'm not sure the UX will be predictable for the user (it might feel weird), 2. I'm afraid of having corner cases where the behavior is hard to define,
  2. replace the scrollbar with a custom component: to be honest, I'm not sure it would fix anything, as I think one scrollbar cannot provide global navigation and local precision at the same time.
  3. provide additional controls/buttons, to make the navigation explicit.

For 3., an option might be to use the scrollbar for local scrolling, eg from row 0 to row 8.000.000, then:

  • at the bottom of the scroll area: show a button 'Load more', that would change the local range to 4.000.000 -> 12.000.000 and put the local scroll to 50%, keeping (or trying to keep) the current visible rows. It might be autoloading instead of an explicit button.
  • same at the top of the scroll area, when scrolling backwards
  • additionally, show a minimap on the right, with the range highlighted. It might be another native scrollbar, or a custom component (more clarity if the scrollbar and the minimap are visually different).

@severo severo force-pushed the implement-virtual-scroll-for-big-dataframes branch from f7e95fe to f3baeb6 Compare January 8, 2026 18:11
@severo severo changed the base branch from master to refactor January 8, 2026 20:15
@severo severo force-pushed the implement-virtual-scroll-for-big-dataframes branch 5 times, most recently from aad7611 to bce4625 Compare January 8, 2026 20:47
Base automatically changed from refactor to master January 8, 2026 21:07
@severo severo force-pushed the implement-virtual-scroll-for-big-dataframes branch 2 times, most recently from e067934 to 8abfa1c Compare January 8, 2026 21:30
@severo severo force-pushed the implement-virtual-scroll-for-big-dataframes branch from 8abfa1c to 4d66f9b Compare January 8, 2026 21:35
@severo severo force-pushed the implement-virtual-scroll-for-big-dataframes branch from 4d66f9b to e40e212 Compare January 8, 2026 21:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The scrollbar does not work as expected above 542K rows (firefox) Render only the visible rows + padding

3 participants