Skip to content

gh-15785: Fix right-sidebar gradient shifts in compact mode #15961

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.

Sign up for GitHub

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

Merged
mr-cheffy merged 4 commits into from
Oct 15, 2026
Merged

gh-15785: Fix right-sidebar gradient shifts in compact mode #15961

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d775869
gh-15785: Keep right sidebar gradients aligned during reveal
yosofbadr Oct 15, 2026
0a7731c
gh-15785: Remove redundant test introduction
yosofbadr Oct 15, 2026
bebb53b
gh-15785: Simplify gradient fix after review
yosofbadr Oct 15, 2026
750bdc5
gh-15785: Remove unnecessary background-position priority
yosofbadr Oct 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/zen/compact-mode/zen-compact-mode.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,13 @@
background-size: 100vw 100vh !important;

@mr-cheffy mr-cheffy Oct 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just move it up here with the rest of the background related properties?

@yosofbadr yosofbadr Oct 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@mr-cheffy mr-cheffy Oct 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

.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?

@yosofbadr yosofbadr Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. 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;
    }
  }
}
  1. 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.

@mr-cheffy mr-cheffy Oct 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see, in that case, it should be alright now

}

:root[zen-right-side='true'] & {
&::before,
&::after {
background-position: right top;
}
}

&,
&::before,
&::after {
Expand Down
Loading