Closed Bug 1317205 Opened 6 years ago Closed 6 years ago

CSS improvement for summary button

Categories

(DevTools :: Netmonitor, defect, P1)

defect

Tracking

(firefox53 verified)

VERIFIED FIXED
Firefox 53
Iteration:
53.1 - Nov 28
Tracking Status
firefox53 --- verified

People

(Reporter: rickychien, Assigned: rickychien)

References

(Blocks 1 open bug)

Details

(Whiteboard: [netmonitor])

Attachments

(2 files)

- 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.
No longer depends on: netmonitor-html
Flags: qe-verify?
Flags: qe-verify? → qe-verify+
Priority: -- → P2
QA Contact: ciprian.georgiu
Whiteboard: [netmonitor][triage] → [netmonitor]
Assignee: nobody → rchien
Status: NEW → ASSIGNED
Iteration: --- → 53.1 - Nov 28
Priority: P2 → P1
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 :)
@ntim: what do you think about the CSS changes?

Honza
Flags: needinfo?(ntim.bugs)
(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.
Flags: needinfo?(ntim.bugs)
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
Attachment #8812119 - Flags: review?(odvarko) → review+
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.
Attachment #8812119 - Flags: review?(jsnajdr)
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/8473572b1fbe
CSS improvement for summary button r=Honza
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/8473572b1fbe
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
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.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.