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

Desktop: Fixes #9822: Allow dialogs to scroll on small screens #9823

Merged

Conversation

personalizedrefrigerator
Copy link
Collaborator

Summary

Fixes #9822 by allowing dialogs to scroll when there is overflow.

Screen recording:

scroll-demo.webm

Notes

I don't know why overflow: hidden was originally added, but presumably it's because scrollbars add extra space on small screens. As such, to prevent visual regressions, this pull request hides the new overflow scrollbars.

@laurent22
Copy link
Owner

don't know why overflow: hidden was originally added, but presumably it's because scrollbars add extra space on small screens. As such, to prevent visual regressions, this pull request hides the new overflow scrollbars.

I think it's because some element somewhere was making the layer scroll, even when there was enough space for the dialog. Maybe it doesn't happen anymore but could you check a few dialogs to confirm?

@personalizedrefrigerator
Copy link
Collaborator Author

personalizedrefrigerator commented Feb 2, 2024

I think it's because some element somewhere was making the layer scroll, even when there was enough space for the dialog. Maybe it doesn't happen anymore but could you check a few dialogs to confirm?

I tested with the "edit folder", "master password", and "sync wizard" dialogs. Note that the tags and due-date dialogs don't use Dialog.tsx to show a modal. As such, overflow: hidden may have been an attempt to fix #9817.

It seems that even if we use overflow: auto, the scrollbars are still invisible. However, when there is overflow, additional space is allocated for them at the edge of the screen:
screenshot: Extra space at the edge of the window for scrollbars

This seems to be because we use rgba(100,100,100,.3) for the scrollbar thumb color and rgba(0,0,0,0.6) for the dialog background color. As such, while checking for unnecessary scrollbars above, I temporarily changed the scrollbar thumb color to red.

@laurent22
Copy link
Owner

That makes sense then, thanks for checking!

@laurent22 laurent22 merged commit 43d36f9 into laurent22:dev Feb 2, 2024
10 checks passed
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.

Desktop: Unable to close sync wizard on a small screen
2 participants