1165576 - Netmonitor theme refresh

Status: RESOLVED FIXED

(DevTools :: Netmonitor, defect)

Comment 1

[Tim Nguyen :ntim]

Attachment: netmonitor-pretty.patch

Changes :
- Refreshed table
- Row is now highlighted when hovered (I've seen feedback requesting this)
- Performance report and Sidebar now uses --theme-sidebar-background
- Refreshed footer buttons
- Fixed a bug where the security icon when clicking it.

Comment 2

[Tim Nguyen :ntim]

Attachment: Screenshot

Comment 3

[Brian Grinstead [:bgrins]]

Comment on attachment 8606627 [details] [diff] [review]
netmonitor-pretty.patch

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

Deleting code and switching from preprocessor to CSS variables?  Sounds good to me :).  I'll let Victor decide whether we need to send this through a ux review or not.

::: browser/themes/shared/devtools/netmonitor.inc.css
@@ -164,5 @@
>  .requests-security-state-icon:hover {
>    -moz-image-region: rect(0px, 32px, 16px, 16px);
>  }
>  
> -.requests-security-state-icon:active {

Why isn't this needed anymore?

@@ +207,1 @@
>    background-color: transparent;  

Nit: please remove this trailing whitespace while you are here

Comment 4

[Tim Nguyen :ntim]

(In reply to Brian Grinstead [:bgrins] from comment #3)
> Comment on attachment 8606627 [details] [diff] [review]
> netmonitor-pretty.patch
> 
> Review of attachment 8606627 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Deleting code and switching from preprocessor to CSS variables?  Sounds good
> to me :).  I'll let Victor decide whether we need to send this through a ux
> review or not.
> 
> ::: browser/themes/shared/devtools/netmonitor.inc.css
> @@ -164,5 @@
> >  .requests-security-state-icon:hover {
> >    -moz-image-region: rect(0px, 32px, 16px, 16px);
> >  }
> >  
> > -.requests-security-state-icon:active {
> 
> Why isn't this needed anymore?
This fixes an issue on OSX where pressing on the security icon would make it disappear.

Comment 5

[Victor Porof [:vporof][:vp]]

I'll get to this asap. Sorry about the wait.

Comment 6

[Victor Porof [:vporof][:vp]]

Comment on attachment 8606627 [details] [diff] [review]
netmonitor-pretty.patch

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

Very nice, much cleaner!

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/3d27abe9852b

Comment 8

[Gabriel Luong [:gl] (ΦωΦ)]

Changes look nice and I like the highlight on hover. Just a couple of issues I wanted to point out from my early testing:

I think the white text on black list items on the network panel is a bit hard on the eyes. Looking at https://people.mozilla.org/~shorlander/mockups/devTools/ux-refresh-2013/[e-mail] - the text font color should be grey like the tabs which is better for UX [1]. 

I also notice the reload button isn't really clear that it is a button because of the dark background in the button.

The background colors used seems darker than shorlander's mockup. Was this intentional?

I think changes like this would also really benefit from a ui-review from shorlander prior to landing.

[1] http://uxmovement.com/content/when-to-use-white-text-on-a-dark-background/

Comment 9

[Tim Nguyen :ntim]

(In reply to Gabriel Luong [:gl] from comment #8)
I tried to minimize the use of custom colors as much as I could, and I have privileged the colors provided by our variables. 
Anyway, can you file a followup and needinfo Stephen on it ?

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/3d27abe9852b