Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

two_billion_rows example scrolls no less than ~2000 rows at a time #15

Open
telamonian opened this issue May 25, 2020 · 6 comments
Open
Labels
bug Concrete, reproducible bugs

Comments

@telamonian
Copy link
Contributor

telamonian commented May 25, 2020

Not sure if this is a bug, or intended behavior (or if I'm just missing something obvious).

Steps to Reproduce:

Clone this repo, then:

yarn install
yarn start

then navigate to the two_billion_rows example.

Expected Result:

For the example to only scroll 1 row at a time (I guess? That might not be the best behavior either) for every press of the up/down keys or tic of the scroll wheel.

Actual Result:

I was playing around with the two_billion_rows example, and I noticed that the least you can scroll (ie on a single press of up/down or on a single click of the mousewheel) is 2364 rows at a time. This appears to be the behavior of any sufficiently long regular-table. This creates a UX problem, as there doesn't seem to be a way to scroll, eg, the 1000th row into view, much less a way to position the 1000th row in a specific location on my screen.

The implementation of this behavior is in the _calculate_row_range function. I think there's three questions here:

  • What's the intended UX for scrolling to a specific row?
  • Does it make sense for regular-table to be able to scroll several thousands rows on a single keypress/scroll wheel tic? Or should it max out at 2-3?
  • Given the above UX considerations, what needs to change in the implementation (if anything)?
@timkpaine
Copy link
Member

If your dataset has 2 billion rows, then on a single down press you're scrolling through 0.0001% of the data. Too much more granular and you'll never be able to scroll to the end.

@timkpaine
Copy link
Member

I suppose scroll could remain unchanged, but down/up and space/shift space could change to +/- 1 and +/- 100 respectively

@telamonian
Copy link
Contributor Author

telamonian commented May 25, 2020

Too much more granular and you'll never be able to scroll to the end.

I agree that there's an inherent contradiction here someplace. At the same time, whether I'm working with 1e3 rows or 1e9 rows, my interaction with individual rows will be more or less the same. If regular-table jumps around thousands of rows for every minimal input, that's going to make some basic interactions difficult.

I suppose scroll could remain unchanged, but down/up and space/shift space could change to +/- 1 and +/- 100 respectively

I like your idea about the keys and the shift modifier. I'm not crazy about the current scroll wheel behavior. My specific issue is that if the screen scrolls by more than the number of rows currently shown (scroll_rows in _calculate_row_range, I think?) on minimal scroll wheel input, I get a bit disoriented and tend to lose track of where I am in the data.

My own preference would be to default to 1-3 rows for both scroll wheel and arrow keys. If someone wants to navigate faster, they can always use a modifier key, or just scrub the scroll bar thumb.

@timkpaine timkpaine added bug Concrete, reproducible bugs question Questions about use, potential features, or improvements and removed type:Bug labels May 25, 2020
@texodus texodus changed the title [BUG?] two_billion_rows example scrolls no less than ~2000 rows at a time two_billion_rows example scrolls no less than ~2000 rows at a time May 25, 2020
@texodus
Copy link
Member

texodus commented May 25, 2020

This is due to a bug in Chrome that we compensate for by limiting the vertical scroll virtual size to 10,000,000px. Beyond this limit, rows in the virtual space are 10,000,000 / N shorter, with a default row height of 20px to a theoretical 1-pixel-per-row of 200,000,000 rows.

regular-table uses a regular overflow: auto CSS rules to implement scrolling. This makes it simple to control the scroll position with scrollTop, style the scrollbars with ::-webkit-scroll-thumb, etc., and it is important to maintain this simplicity. However, I would like to see an extension via Javascript API that allows more fine-grained scrolling, scrollRowsBy() or some such that can scroll in virtual-row coordinates. Developers could then easily override and fine-tune the browser default mouse scroll to customize this functionality using normal DOM API:

regularTable.addEventListener("scroll", event => {
    regularTable.scrollBy(translateMouseWheelToVirtualRows(event.wheelX));
}

@texodus texodus removed the question Questions about use, potential features, or improvements label May 25, 2020
@shubhamcodez
Copy link

This is due to a bug in Chrome that we compensate for by limiting the vertical scroll virtual size to 10,000,000px. Beyond this limit, rows in the virtual space are 10,000,000 / N shorter, with a default row height of 20px to a theoretical 1-pixel-per-row of 200,000,000 rows.

regular-table uses a regular overflow: auto CSS rules to implement scrolling. This makes it simple to control the scroll position with scrollTop, style the scrollbars with ::-webkit-scroll-thumb, etc., and it is important to maintain this simplicity. However, I would like to see an extension via Javascript API that allows more fine-grained scrolling, scrollRowsBy() or some such that can scroll in virtual-row coordinates. Developers could then easily override and fine-tune the browser default mouse scroll to customize this functionality using normal DOM API:

regularTable.addEventListener("scroll", event => {
    regularTable.scrollBy(translateMouseWheelToVirtualRows(event.wheelX));
}

the issue is closed?

@texodus
Copy link
Member

texodus commented Mar 30, 2021

@shubhamcodez The issue is not-fixable, as noted. If you have a real, usable example of a grid with enough rows to trigger this issue however, I'd be very interested to see it - you need over 500k rows in a dataset before even a single pixel will drift on Chrome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Concrete, reproducible bugs
Projects
None yet
Development

No branches or pull requests

4 participants