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

[BUG] Double scrollbar on table with useWindowScroll and horizontally scrollable container #998

Open
mihkeleidast opened this issue Oct 23, 2023 · 3 comments
Labels
bug Something isn't working

Comments

@mihkeleidast
Copy link
Contributor

mihkeleidast commented Oct 23, 2023

Describe the bug
Scenario:

  • I want the Table to use window (or parent) scroll for vertical scrolling
  • I want the Table to have a horizontally scrollable container, so it wouldn't make the entire page scroll horizontally
  • I want to have a single scrollbar in both directions on both 100% and other scaled displays (very common on laptops and tablets, etc)

Reproduction
https://codesandbox.io/s/dreamy-hill-vvjgd2?file=/styles.css

To Reproduce
Steps to reproduce the behavior:

  1. Open the sandbox (may need a scaled display or zooming, as it comes down to rounding errors)
  2. See double vertical scrollbar

Expected behavior
There should be only a single scrollbar in either direction.

Screenshots

double vertical scrollbar

image

Desktop (please complete the following information):

  • OS: Windows
  • Browser: Edge 118, Firefox 118

Additional context
I initially discovered this on an a scaled display. However, after playing around with some CSS, also reproduced it on a 100% scaled display.

This may happen because of row height rounding. E.g. when inspecting, some rows have data-known-size of 50, while they are actually rendered at 50.19px.

The scroller height is set at 50020px, while its scrollHeight is 50025px. The difference comes up just at about 26 (rows rendered) x 0.19px = 4.94px.

I know I can work around this by setting overfow-y: hidden; on the horizontal scroll container, but thought it's still best to report this.

@mihkeleidast mihkeleidast added the bug Something isn't working label Oct 23, 2023
@petyosi
Copy link
Owner

petyosi commented Oct 23, 2023

Hey @mihkeleidast ,

what you describe makes sense, thank you for sharing the workaround. I've gone back and forth on rounding/not-rounding floating item sizes, since both approaches have their tradeoffs. Leaving a trace of the last commit that touched that: 9127413.

I might give this some more tries at some point, but the ultimate culprit here is that you can't call scrollTo with a floating value.

@mihkeleidast
Copy link
Contributor Author

I wonder if ceiling instead of simple rounding would work better for some calculations? Or, keeping track of the diff of rounded/non-rounded heights and adding that deviation to the scroller height as well?

@petyosi
Copy link
Owner

petyosi commented Oct 23, 2023

I don't know. It's quite the case to solve, maybe the rounding should come at the scrollTo/scrollBy calls. You have some experience with the internals of the library, and the commit actually includes a test that denotes the problems discovered when rounding was not applied. Happy to accept a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants