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

Horizontal overflow on VariableSizeGrid #65

Closed
martynaskadisa opened this issue Oct 2, 2018 · 6 comments
Closed

Horizontal overflow on VariableSizeGrid #65

martynaskadisa opened this issue Oct 2, 2018 · 6 comments

Comments

@martynaskadisa
Copy link
Contributor

Similar to #22. Except this time it's for VariableSizeGrid. I've created a few examples to demonstrate.

Horizontal scrollbar appears on OSes with always visible scrollbars (e.g. Windows) once scrolled a bit:
https://codesandbox.io/s/r55m5n027n

The same issue could be reproduced on other OSes (e.g. macOS) when using custom scrollbars via :-webkit-scrollbar:
https://codesandbox.io/s/618026v83r

The only workaround I've found is to offset each column by scrollbar width:
https://codesandbox.io/s/l2rqwyv989

Ofc then we have to know the scrollbar width beforehand either using a constant width when we have custom scrollbars or using some kind of width detection mechanism (calculating from elm.clientWidth - elm.offsetWidth). The latter sounds more ugly and could lead to jank when called to frequently. But then again, we would only need to calculate scrollbar width once in user's session.

@bvaughn
Copy link
Owner

bvaughn commented Oct 3, 2018

I don't think this is the same as #22. I think grid is kind of behaving correctly in this case by adding the horizontal scroll bar. (Although I agree it's weird that it only appears once you start to scroll. At least for me though, the Code Sandbox page you linked to itself also does the same thing with a horizontal scrollbar. I think this is kind of just a janky browser quirk.)

The same issue could be reproduced on other OSes (e.g. macOS) when using custom scrollbars via :-webkit-scrollbar:

Fair enough, although I'm not really interested in supporting custom scrollbars with this lib.

The only workaround I've found is to offset each column by scrollbar width:

If you definitely don't want to have horizontal scrollbars, you could add style={{ overflowX: 'hidden' }} to the grid in question. But as I said, I think in this case it's behavior correctly.

If you really want a non-horizontally scrolling grid, then it sounds like you actually want a list.

I'm going to close this issue because I think it reflects a misunderstanding but we can continue to talk on it if anything I've said in response seems incorrect! 😄

@bvaughn bvaughn closed this as completed Oct 3, 2018
@martynaskadisa
Copy link
Contributor Author

I think we're a bit in a misuderstanding 😅. I'm not expecting react-window to support custom scrollbars so I only provided a link with custom scrollbars to emulate this issue on platforms without always visible scrollbars. I'm not sure what platform you're on, so you might not be experiencing this issue 😁

I'll provide some screenshots from Windows for clarity.

@martynaskadisa
Copy link
Contributor Author

This is how it looks from my first link (https://codesandbox.io/s/r55m5n027n) on Windows, no custom scrollbars:
1

The inner container width is exactly larger by scrollbar width.

If you definitely don't want to have horizontal scrollbars, you could add style={{ overflowX: 'hidden' }} to the grid in question. But as I said, I think in this case it's behavior correctly.

This also doesn't help since it only hides the scrollbar, the inner container is still wider because of the scrollbar and the content will just be cut off:
2

I hope I've described the issue better now 😅

@bvaughn
Copy link
Owner

bvaughn commented Oct 3, 2018

No worries. I believe I understood the issue initially. (I'm on OS X but you can turn scrollbars on in System Preferences, which I do from time to time for testing purposes.)

The sandbox you shared is showing a horizontal scrollbar because you set the width of your grid columns to half the width of the grid. This doesn't really seem like a valid use case to me. If that's the behavior you want, then you should be using a list component instead of a grid. If you do want to use grid because you have a variable number of columns (with predetermined/fixed widths)- then I think that the behavior the sandbox you shared is actually correct, because the columns are slightly too wide to fit into the grid, given that a vertical scrollbar is taking up part of the width.

@martynaskadisa
Copy link
Contributor Author

Yeah, I want to use grid because I have variable column count with fixed widths. Do you think VariableSizeList is a better choice here even though it will still essentially be a grid layout?

@bvaughn
Copy link
Owner

bvaughn commented Oct 3, 2018

I have variable column count with fixed width

In this case, it sounds like VariableSizeGrid is the right choice for you then 👍but if your grid ends up having two columns, and each is 300 pixels wide, and your grid ends up being 600 pixels wide– then I believe the correct thing for VariableSizeGrid to do is to show both a vertical and horizontal scroll bar (as the Code Sandbox you linked to shows) 😄

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

No branches or pull requests

2 participants