Repository navigation
gh-15785: Fix right-sidebar gradient shifts in compact mode - #15961
Conversation
|
Before the fix, the gradient shifts as the right sidebar slides into view, then settles into place. With the fix, it stays aligned throughout the reveal. The side-by-side video in the PR description shows the old behavior on the left and the fixed behavior on the right: https://github.com/user-attachments/assets/52181d86-1460-4716-aa2e-d0893b00a1a3 |
mr-cheffy
left a comment
There was a problem hiding this comment.
Also, let me know when its ready for merge, right now its marked as "draft"
| @@ -22,6 +22,13 @@ | |||
| background-size: 100vw 100vh !important; | |||
There was a problem hiding this comment.
Why not just move it up here with the rest of the background related properties?
There was a problem hiding this comment.
I misunderstood your first comment as moving the rule inside .zen-toolbar-background, rather than inside the existing pseudo-element block.
If you meant this nesting, I tested it locally:
.zen-toolbar-background {
&::before,
&::after {
background-attachment: fixed !important;
background-size: 100vw 100vh !important;
:root[zen-right-side='true'] & {
background-position: right top !important;
}
}
}It brings back the right-sidebar gradient shift. The inner & cannot represent the parent pseudo-elements, so the positioning does not apply. This is a CSS nesting limitation.
We could keep the current structure, where the condition comes before the pseudo-elements:
.zen-toolbar-background {
:root[zen-right-side='true'] & {
&::before,
&::after {
background-position: right top !important;
}
}
}Or use a custom property to put background-position alongside the other background properties:
.zen-toolbar-background {
:root[zen-right-side='true'] & {
--zen-compact-background-position: right top;
}
&::before,
&::after {
outline: 1px solid rgba(255, 255, 255, .15);
outline-offset: -1px;
background-attachment: fixed !important;
background-size: 100vw 100vh !important;
background-position:
var(--zen-compact-background-position, left top) !important;
}
}Both options pass all 16 rendering assertions on the local Firefox 157 runtime, including both background layers with the sidebar on the left and on the right. The deeper nesting above fails the two right-sidebar color checks. The left sidebar passes in all three versions.
Which approach would you prefer?
There was a problem hiding this comment.
.zen-toolbar-background {
&::before,
&::after {
background-attachment: fixed !important;
background-size: 100vw 100vh !important;
:root[zen-right-side='true'] & {
background-position: right top !important;
}
}
}would be nice, but I dont think we need !important?
There was a problem hiding this comment.
Good callout on !important. I verified that the current rule works without it and removed it.
One clarification: the first snippet in my earlier reply was the failing example. Nesting the condition inside &::before, &::after does not work because the inner & cannot represent pseudo-elements. I reran that exact version with and without !important. Both leave the position at 0% 0% and bring back the gradient shift. I agree it looks cleaner, but that nesting is not a working option.
We have two working options:
- Keep the current structure. I'd recommend this because it needs no custom property or
!important:
.zen-toolbar-background {
:root[zen-right-side='true'] & {
&::before,
&::after {
background-position: right top;
}
}
}
- Use a custom property to group the background declarations:
.zen-toolbar-background {
:root[zen-right-side='true'] & {
--zen-compact-background-position: right top;
}
&::before,
&::after {
background-attachment: fixed !important;
background-size: 100vw 100vh !important;
background-position:
var(--zen-compact-background-position, left top) !important;
}
}
The second keeps the properties together, but adds a custom property and needs !important to override the existing, more specific background shorthand.
Both pass the rendered-color checks for left/right sidebars and both background layers locally. Happy to use the second if you prefer that structure.
There was a problem hiding this comment.
Oh I see, in that case, it should be alright now
Fixes #15785.
Summary
While testing the right-sidebar behavior for #15775, I noticed this gradient shift and found that others had already reported it in #15785. During the compact-mode reveal, the sidebar briefly shows the wrong part of the theme before settling into place.
This fix aligns both background layers to the right edge, so the sidebar keeps the same part of the gradient as it slides. The left sidebar keeps its existing alignment.
Before and after
Side-by-side comparison, before on the left and after on the right:
zen-gradient-comparison.mp4
Before:
zen-gradient-before.mp4
After:
zen-gradient-after.mp4
Testing
A temporary browser-chrome rendering check passes all 16 assertions. It samples rendered colors halfway through the reveal for left and right sidebars and both workspace-theme layers. Without the CSS fix, the two right-sidebar color checks fail.
Explicit CSS lint of the complete
.zen-toolbar-backgroundblock and the incremental UI build pass. The usual lint command ignores this file.The recordings and browser tests use the existing local Firefox 157 runtime with the current compact-mode CSS. A fresh full build of the Firefox 158 base has not been run. Windows and Linux have not been tested.