1317205 - CSS improvement for summary button

Status: VERIFIED FIXED

(DevTools :: Netmonitor, defect, P1)

Description

[Ricky Chien [:rickychien] (inactive)]

- the "performance" icon in the summary info should change opacity on mouseover. There are rules for ".devtools-button::before" that do this, but they don't apply in this case.
- the colors of the performance icon and the summary text should be the same. They aren't in the dark theme - white vs lightblue

This CSS issue is spotted by Jarda https://bugzilla.mozilla.org/show_bug.cgi?id=1309194#c50. There are existed problems in summary button and it needs CSS improvement.

Comment 1

[Ricky Chien [:rickychien] (inactive)]

Attachment: performance-tool-icon-and-text-buttons.png

Hover opacity effect and color is unsafe to touch since it defines at devtools common.css https://dxr.mozilla.org/mozilla-central/source/devtools/client/themes/common.css#369-383

For some reasons we make these styling difference between Icon-only buttons and Icon-and-text buttons and it makes sense to me for making all devtools elements to be more consistent. 

There is a place which is using Icon-and-text buttons which is in performance tool, but maybe there are more. (see attachment)

For icon-and-text buttons of performance tool, there is nothing changed in opacify when hovering on it but instead the background color is changed. These unchecked buttons remain white icon and light-blue text. As a result, I prefer to overwrite the icon-and-text buttons in common.css to make look and feel more consistent within netmonitor. The new style would apply opacity: 0.8 to both icon and text and update opacity to 1 while hovering.

Please apply my patch to see the behaviors :)

Comment 2

[Ricky Chien [:rickychien] (inactive)]

Attachment: Bug 1317205 - CSS improvement for summary button

Review commit: https://reviewboard.mozilla.org/r/94002/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/94002/

Comment 3

[Jan Honza Odvarko [:Honza] (always need-info? me)]

@ntim: what do you think about the CSS changes?

Honza

Comment 4

[Tim Nguyen :ntim]

(In reply to Jan Honza Odvarko [:Honza] from comment #3)
> @ntim: what do you think about the CSS changes?
> 
> Honza

Looks fine to me, although I'd rather investigate to make .devtools-button suitable for icon and text buttons. I'm fine to do that as a follow up though.

Comment 5

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8812119 [details]
Bug 1317205 - CSS improvement for summary button

https://reviewboard.mozilla.org/r/94002/#review94534

R+, but please see also ntim's comment #4

Honza

Comment 6

[Jarda Snajdr [:jsnajdr]]

Comment on attachment 8812119 [details]
Bug 1317205 - CSS improvement for summary button

https://reviewboard.mozilla.org/r/94002/#review94568

Canceling my review request, as I have nothing to add. ntim is more competent for this bug.

Comment 7

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8812119 [details]
Bug 1317205 - CSS improvement for summary button

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/94002/diff/1-2/

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8473572b1fbe
CSS improvement for summary button r=Honza

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8473572b1fbe

Comment 10

[Ciprian Georgiu, Desktop QA]

I've managed to reproduce this issue on a Nightly build from 2016-11-21.

This is verified fixed on latest Nightly 53.0a1 (2016-11-23) across platforms:
- Windows 10 x64
- Ubuntu 16.04 x64 LTS
- Mac OS X 10.11.6 
Now the performance icon in the summary info is changing the opacity on mouseover.