-
Notifications
You must be signed in to change notification settings - Fork 183
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
frontend: Improve scroll bars #2715
base: main
Are you sure you want to change the base?
Conversation
e106dbf
to
cda138e
Compare
Signed-off-by: Evangelos Skopelitis <[email protected]>
Signed-off-by: Evangelos Skopelitis <[email protected]>
87d25ca
to
3e3b634
Compare
flexGrow: 1, | ||
marginLeft: 'initial', | ||
overflow: 'auto', | ||
[theme.breakpoints.down('sm')]: { marginTop: { xs: '-12px', sm: '-8px' } }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's a lot of new magic values (64px, 72px, -12px, -8px). maybe we can do this change without introducing them? maybe making topbar not positioned absolute or sticky.
or if that's not possible at least introduce some constants and plan for a possibility that topbar size might be changed in the future
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if the topbar isn't set to fixed then that defeats the purpose of it right? I think the new values are a good workaround (temporary if we can fix the topbar somehow) so I can abstract them into constants for now
Signed-off-by: Evangelos Skopelitis <[email protected]>
Signed-off-by: Evangelos Skopelitis <[email protected]>
3e3b634
to
8e9c095
Compare
These changes display the scroll bars for the main content and sidebar under the top bar. The width of the sidebar and the height of the top bar are also adjusted to prevent inconsistencies.
Fixes: #2679
Testing