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

Fix scroll #33

Closed
gsimone opened this issue Dec 30, 2020 · 5 comments · Fixed by #65
Closed

Fix scroll #33

gsimone opened this issue Dec 30, 2020 · 5 comments · Fixed by #65
Labels
bug Something isn't working

Comments

@gsimone
Copy link
Member

gsimone commented Dec 30, 2020

When the panels are higher than the window, bad stuff happens. We should handle scroll inside the pane itself

@gsimone gsimone added the bug Something isn't working label Dec 30, 2020
@dbismut
Copy link
Collaborator

dbismut commented Dec 31, 2020

That one is not that easy unfortunately. The obvious solution would be to set {max-height: 100%; overflow: auto} on the root container. But because we have overflowing controls (the color picker or the joystick) we would have artefacts such as this one:

Before:
image

After:
image

Of course we could try tricks such as setting position: fixed on the color picker, or put them in portal that would be a sibling of the root, have a scroll listener on the root so that we can reflow the color picker at the right position, but that's a lot of troubles tbh.

One potential quick fix would be to only add overflow: auto to the root only when its height is greater than the window height. We would still have the clipping issue for color-pickers at the very bottom of the pane but that's an acceptable trade-off imo.

@gsimone
Copy link
Member Author

gsimone commented Dec 31, 2020

Ideally we should portal the color picker and other floating elements to a container outside of the overflow-hidden one, that would be the safest solution. Do we have that kind of control for the colors? (I guess we do for the joystick input)

@dbismut
Copy link
Collaborator

dbismut commented Dec 31, 2020

Just that wouldn't work because of the scrolling + we would have to set the initial position manually with js. Not saying it's impossible, but handling the sync is ugly and adds some logic. Maybe v1.1?

@marcofugaro
Copy link
Member

What about manually checking if it overflows the window and setting overflow-y: auto accordingly?

Could be done after the drag is finished as well.

@dbismut
Copy link
Collaborator

dbismut commented Jan 30, 2021

Yes we could do that. Then I guess could also manage in which direction should the overlays pop (either on upwards or downwards, depending on their position on the screen).

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

Successfully merging a pull request may close this issue.

3 participants