-
Notifications
You must be signed in to change notification settings - Fork 93
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 padding of sidebar without secondary actions #1362
Conversation
Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
@@ -522,6 +527,10 @@ $top-buttons-spacing: 6px; | |||
// increase the padding to not overlap the menu | |||
.app-sidebar-header__desc { | |||
padding-right: #{$clickable-area * 2 + $top-buttons-spacing}; | |||
|
|||
&.app-sidebar-header__desc--without-actions { | |||
padding-right: #{$clickable-area + $top-buttons-spacing}; |
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.
Why does without actions have more padding than the default ?
The css doesn't make sense there 🤔
Without have 50px, default only have 6px ?
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.
Well, it's complicated. By default the menu action is part of the flow, so 6px is enough since the action menu itself will push the titles right border to the left. But as soon as you take the menu out of the flow (in case of no figure, or compact mode), the title needs a right-padding. And if there is no menu, the padding must be lower.
Feel free to change the CSS, the new base images in this PR will tell you if the CSS works. 😉
PIng? :) |
To be honest, I don't really know how to further simplify / clarify the CSS. The visual appearance resulting is ok, as you see from the visual regression tests. As said above, feel free to adjust the CSS structure. As long as you don't touch the base images and the regression tests still pass, I am all in 😉 |
No, I'm just super cautious now. 😅 |
I understand that. So should we merge it as is? |
Fixes the right-padding of the sidebar title for sidebars (in compact mode || without figure) && without secondary actions 😉.
Before:
After: