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

Group and scope the secondary toolbar button container/icon rules using CSS nesting #18595

Merged

Conversation

timvandermeij
Copy link
Contributor

@timvandermeij timvandermeij commented Aug 11, 2024

The secondary toolbar CSS rules predate the general availability of CSS nesting, which makes them more difficult to understand and change safely. The primary issues are that the rules are spread over the viewer.css file, they share blocks with other elements and the scope of the rules is sometimes bigger than necessary.

This refactoring groups all CSS rules for the secondary toolbar button container/icons together, scoped to the top-level #secondaryToolbar element, for improved overview and isolation. Note that this patch only intends to move the existing rules around and not change any behavior. Moreover, this patch does not move the rules for the secondary toolbar itself and the secondary toolbar buttons; those will be part of a follow-up patch and will be easier once this is in place first.

Notes for the reviewer:

  • I have deliberately introduced the #secondaryToolbar rules in a different part of the file to make the diff (and future diffs for the secondary toolbar) easier. Now the deletions and additions can more easily be compared side-by-side while otherwise they would be intertwined.
  • I have put the icon rules in the exact same order as in the HTML for consistency and easier comparison.

Extracts a part of #18385.

…ng CSS nesting

The secondary toolbar CSS rules predate the general availability of CSS
nesting, which makes them more difficult to understand and change
safely. The primary issues are that the rules are spread over the
`viewer.css` file, they share blocks with other elements and the scope
of the rules is sometimes bigger than necessary.

This refactoring groups all CSS rules for the secondary toolbar button
container/icons together, scoped to the top-level `#secondaryToolbar`
element, for improved overview and isolation. Note that this patch only
intends to move the existing rules around and not change any behavior.
Moreover, this patch does not move the rules for the secondary toolbar
itself and the secondary toolbar buttons; those will be part of a
follow-up patch and will be easier once this is in place first.

Co-authored-by: Calixte Denizet <calixte.denizet@gmail.com>
@timvandermeij
Copy link
Contributor Author

/botio-linux preview

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Received

Command cmd_preview from @timvandermeij received. Current queue size: 0

Live output at: http://54.241.84.105:8877/a0cd436ed27e4b2/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/a0cd436ed27e4b2/output.txt

Total script time: 1.01 mins

Published

Copy link
Collaborator

@Snuffleupagus Snuffleupagus left a comment

Choose a reason for hiding this comment

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

r=me, thank you.

@timvandermeij timvandermeij merged commit efe00cf into mozilla:master Aug 11, 2024
6 checks passed
@timvandermeij timvandermeij deleted the css-secondary-toolbar-part-1 branch August 11, 2024 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants