Closed Bug 965985 Opened 6 years ago Closed 6 years ago

DevTools light theme button followup

Categories

(DevTools :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 29

People

(Reporter: bgrins, Assigned: bgrins)

References

Details

Attachments

(1 file, 1 obsolete file)

I've noticed that the active buttons have the normal hover effect when hovered, which makes it confusing when you click a toggle-able button like on the console.  Just need to do a little cleanup before we move on to Bug 952100.
Duplicate of this bug: 966200
Another issue (from Bug 966200):

1. Enable devtools.debugger.tracer
2. Open debugger

The stepping buttons have a different border color vs. the tracer button.
I think all the buttons should have the same border.

The [O|Import] button in the profiler has a darker border than the rest of the buttons.
Status: NEW → ASSIGNED
Attached patch theme-button-follow-up.patch (obsolete) — Splinter Review
The issue with groups having different borders has been handled in debugger.inc.css and profiler.inc.css.  We should really have a 'devtools-button-group' class for this type of pattern so it can be reused, but this resolves the issue in its current state.

Also fixes the [checked]:hover issue on light theme buttons by applying certain rules specifically to the dark theme in toolbars.inc.css and moves some of the shared rules above the specific theme overrides.
Attachment #8368587 - Flags: review?(vporof)
Comment on attachment 8368587 [details] [diff] [review]
theme-button-follow-up.patch

Review of attachment 8368587 [details] [diff] [review]:
-----------------------------------------------------------------

Is there a followup for creating a shared devtools-button-group class? If so, mind adding the bug in comments in the css files? If not, would it be super hard to fix it now? (I don't feel strongly either way)

::: browser/themes/shared/devtools/toolbars.inc.css
@@ +106,4 @@
>    background-color: transparent !important;
>  }
>  
> +.theme-dark .devtools-toolbarbutton[checked=true]:hover:active {

Do we need .theme-light equivalents for these colors? If so (and they're in the file, care to move them here?
Attachment #8368587 - Flags: review?(vporof) → review+
(In reply to Victor Porof [:vp] from comment #4)
> Comment on attachment 8368587 [details] [diff] [review]
> theme-button-follow-up.patch
> 
> Review of attachment 8368587 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Is there a followup for creating a shared devtools-button-group class? If
> so, mind adding the bug in comments in the css files? If not, would it be
> super hard to fix it now? (I don't feel strongly either way)
> 

I've updated the patch to go ahead and make the helper class and remove specific code for each group.  I've asked for review again just to cover these changes.

> ::: browser/themes/shared/devtools/toolbars.inc.css
> @@ +106,4 @@
> >    background-color: transparent !important;
> >  }
> >  
> > +.theme-dark .devtools-toolbarbutton[checked=true]:hover:active {
> 
> Do we need .theme-light equivalents for these colors? If so (and they're in
> the file, care to move them here?

In this case, the color is defined alongside a bunch of others: 
.theme-light .devtools-menulist[open=true],
.theme-light .devtools-toolbarbutton[open=true],
.theme-light .devtools-toolbarbutton[open=true]:hover,
.theme-light .devtools-toolbarbutton[open=true]:hover:active,
.theme-light .devtools-toolbarbutton[checked=true],
.theme-light .devtools-toolbarbutton[checked=true]:hover,
.theme-light .devtools-toolbarbutton[checked=true]:hover:active

The button UI (for both themes) is going to be changed in Bug 952100, so some of this will probably change soon.
Assignee: nobody → bgrinstead
Attachment #8368587 - Attachment is obsolete: true
Attachment #8368773 - Flags: review?(vporof)
Attachment #8368773 - Flags: review?(vporof) → review+
https://hg.mozilla.org/mozilla-central/rev/ce2e31b34b1e
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 29
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.