The netmonitor uses some much simpler CSS for its buttons, let's use that.

Comment on attachment 8735175 [details] [diff] [review] Patch Review of attachment 8735175 [details] [diff] [review]: ----------------------------------------------------------------- Overall I'm happy with moving the rules out of the individual tool's style sheets and into the shared one. I think this needs a UX review. Clearing review flag for now but left some notes inline ::: devtools/client/themes/toolbars.css @@ +62,5 @@ > background: transparent; > min-height: 18px; > text-shadow: none; > border: none; > + border-radius: 2px; Requesting UX review from Helen for this change - do we want all of our buttons to have slightly rounded corners? @@ +114,5 @@ > .devtools-button[standalone], > .devtools-button[data-standalone], > .devtools-toolbarbutton[standalone], > .devtools-toolbarbutton[data-standalone] { > + border: 1px solid rgba(120,120,120,0.2); Should we introduce a new variable for this border instead of using this hardcoded neutral color? Then we could customize it for dark and light theme @@ -120,2 @@ > min-height: 32px; > - background-color: var(--theme-toolbar-background); This is also changing the background color for toolbar buttons. Helen can you check with this patch applied and see what you think? @@ +196,4 @@ > opacity: 0.8; > } > > +.devtools-button:not(:disabled):not([disabled]):-moz-any(:hover,[checked],[open]):empty::before, This selector is pretty complex.. It could use at least a comment above it explaining what this is trying to do if it's not possible to simplify @@ +206,5 @@ > .devtools-toolbarbutton[disabled] { > opacity: 0.5 !important; > } > > +.devtools-button:-moz-any([checked],[open]):empty::before, I preferred the previous way of doing it instead of using -moz-any since it's more familiar to people and each selector is pretty simple @@ +242,5 @@ > +.devtools-toolbarbutton:not([disabled]):-moz-any([label], [data-text-only])[checked=true], > +.devtools-toolbarbutton:not([type=menu-button]):not([disabled]):-moz-any([label], [data-text-only])[open], > +.devtools-button:not(:empty)[checked=true], > +#toolbox-buttons .devtools-toolbarbutton[text-as-image][checked=true] { > + background: var(--theme-selection-background); This is a really big visual change (see selected background for webconsole filter icons which are now same color as the toolbox tabs). Is this the intent?

Mozscreenshots try push with patch applied: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2730b1e2b8ef

Here's a link to the screenshot artifacts on that try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2730b1e2b8ef&exclusion_profile=false&filter-tier=1&filter-tier=2&filter-tier=3

In particular, here's the webconsole buttons I'm talking about: http://mozilla-releng-blobs.s3.amazonaws.com/blobs/try/sha512/2f79c5dcbc76d376c57d4d311032c2979b03b2451265d76da449298d5bcd5a4b6b02b4ad21ba645ddcba4e5cbf5ff4669ca05c9a150a74f33607f156610dd24d

Comment on attachment 8735175 [details] [diff] [review] Patch (In reply to Brian Grinstead [:bgrins] from comment #2) > ::: devtools/client/themes/toolbars.css > @@ +62,5 @@ > > background: transparent; > > min-height: 18px; > > text-shadow: none; > > border: none; > > + border-radius: 2px; > > Requesting UX review from Helen for this change - do we want all of our > buttons to have slightly rounded corners? Yep, I'm fine with this. I'm not a huge fan of making the buttons that blue from the semi-transparent, though. When we hide the buttons in a dropdown menu (in the future designs here: http://cl.ly/2c0X3v143r0B) I'll be fine with it but I think that bgrins is right that they look strange right underneath the toolbar tabs. Can we roll back that change?

Summary of changes: - Consolidated all HTML button classes (.devtools-button, .tool-button, .menu-filter-button) into one single .devtools-button class - Removed support for the button checked attribute (use class names instead) - Added :hover state to .devtools-button icon (approved by Helen in bug 1296271) - Done some small tweaks that Helen approved in comment 6 - The old console button style has been preserved and has been moved into webconsole.css

Comment on attachment 8835755 [details] Bug 1255116 - Simplify toolbar button styles. https://reviewboard.mozilla.org/r/111376/#review113416 I noticed that with the dark theme: * Command buttons use a different background color on hover than tabs for panels do * Settings cog has the hover background color that's used for panels (which I guess makes sense because it will show a panel...) which is different from its neighboring command buttons With both themes: * The settings cog icon has less contrast compared to neighboring command buttons These seem like small issues. If they are not expected, please file follow ups for them if you don't want to address them here.

Pushed by ntim.bugs@gmail.com: https://hg.mozilla.org/integration/autoland/rev/6eb03c463039 Simplify toolbar button styles. r=jryans

Automation screenshots of the changes for review: https://screenshots.mattn.ca/compare/?oldProject=mozilla-central&oldRev=0a7831d838f793a263456df62f95a615472a7f95&newProject=mozilla-central&newRev=5e691d87bed93f740094604be686567b34a6f29d