968315 - [markup view] Current / hovered node highlighting colors need to be higher contrast

Status: VERIFIED FIXED

(DevTools :: Inspector, defect, P1)

Description

[Jeff Griffiths (:canuckistani) (:⚡︎)]

Part of the new color schemes is a much higher contrast color for the currently selected and currently hovered nodes in the markup view. 

After speaking with Joe about this this morning, we felt we should log a bug specifically for implementing this part of the new theme and raise the priority for it. We've gotten pretty consistent feedback from developers that our current tastefully shaded colors are not visible enough and also different enough from what chrome and firebug do to be off-putting when people try our tools.

Setting P1 as a strawman :)

Some screenshots for comparison:

http://note.io/MtmtTu
http://note.io/Mtnrz1

Comment 1

[Brian Grinstead [:bgrins]]

Mockups:

Dark: https://people.mozilla.org/~shorlander/mockups/devTools/ux-refresh-2013/[e-mail]
Light: https://people.mozilla.org/~shorlander/mockups/devTools/ux-refresh-2013/[e-mail]

I believe the mockups are using 'select highlight blue' for each theme from: https://developer.mozilla.org/en-US/docs/Tools/DevToolsColors.

And actually, it looks like the light mockup is a bit lower contrast than what we have now, but it could probably be tweaked.

Comment 2

[Brian Grinstead [:bgrins]]

Bug 947242 is also tangentially related, in that it is meant for switching over to the new text colors for both themes (and the new sidebar background color for the dark theme).

> Part of the new color schemes is a much higher contrast color for the
> currently selected and currently hovered nodes in the markup view. 

This particular contrast issue can be handled here separately, since it is much more narrow in focus and easier to fix without affecting other UI issues.

Comment 3

[Jeff Griffiths (:canuckistani) (:⚡︎)]

(In reply to Brian Grinstead [:bgrins] from comment #1)
> Mockups:
> 
> Dark:
> https://people.mozilla.org/~shorlander/mockups/devTools/ux-refresh-2013/
> [e-mail]
> Light:
> https://people.mozilla.org/~shorlander/mockups/devTools/ux-refresh-2013/
> [e-mail]
> 
> I believe the mockups are using 'select highlight blue' for each theme from:
> https://developer.mozilla.org/en-US/docs/Tools/DevToolsColors.
> 
> And actually, it looks like the light mockup is a bit lower contrast than
> what we have now, but it could probably be tweaked.

I discussed these mockups with Shorlander last year and I seem to recall him saying he would re-do them to address this particualar issue. I'm cc'ing him if he has some content. The color for 'select highlight blue' in the light theme from MDN looks darker than what is in the mockup though?

I want to be really clear, I do not think the colors in those mockups are something we should ship. I noticed again today that the highlighted node in chrome is *exactly* the same as Firebug in appearance. That's deliberate. My preference would be to just do that for the light theme and figure out something similarly high contrast for the dark theme. 

When I look at 'select highlight blue' for the dark theme from MDN, it seems *really* dark and I assume will be against a black background. If the dark theme is going to be the default, the selected and hovered nodes need to be more visible than they currently are. As the other browsers do not *have* a dark theme, it's up to us to figure out how to do this for a dark theme.

Comment 4

[Brian Grinstead [:bgrins]]

Also, the selected color should have a higher priority than the hover color.  Right now, when you click to select an element from the page it has the hover background until you mouse over and out.  If a node is selected, it should always have the selected color.

Comment 5

[Brian Grinstead [:bgrins]]

Attachment: select-colors-markupview.png

I'm inclined to just stick with our current dark/light select colors for this (top row).  It matches the selected tab color, and the other highlight color just looks weird on the dark theme.

Comment 6

[Brian Grinstead [:bgrins]]

(In reply to Brian Grinstead [:bgrins] from comment #5)
> Created attachment 8376285 [details]
> select-colors-markupview.png
> 
> I'm inclined to just stick with our current dark/light select colors for
> this (top row).  It matches the selected tab color, and the other highlight
> color just looks weird on the dark theme.

Here are the colors being used in the above screenshot: 

Top left = #4c9ed9
Bottom left = rgb(0, 124, 213)

Top right = #1d4f73
Bottom right = rgb(0, 124, 213)

Comment 7

[Brian Grinstead [:bgrins]]

Attachment: markup-selected-dark-theme.png

Darrin, can you please remake the controls.png image [1] with a couple of changes.  Take a look at this attachment for reference

1) A personal pet peeve of mine is that the expanded twisty doesn't look right.  It is rotated at what looks to be 45 degrees at a weird origin (so that it ends up bottom aligned with the text).  IMO it should be either rotated 90 degrees instead so that the arrow is pointing straight down, or it could stay at the same alignment but just middle aligned

2) If you think it would be best, we could change the color of the light arrows to #f5f7fa to match the text in the markup view.  Be aware though that it will also change the color of the twisty on the right of the screen (in the computed view tab), and it may look *too* bright there.  So it maybe would be best to just lighten it up a bit instead of going all the way to the text color.  Whatever you think is best.

[1] https://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/devtools/controls.png

Comment 8

[Darrin Henein [:darrin]]

Attachment: controls.png

Attached are @1x and @2x controls.png

Comment 9

[Darrin Henein [:darrin]]

Attachment: controls.zip

Comment 10

[Darrin Henein [:darrin]]

Attachment: controls.zip

3rd times a charm :)

Comment 11

[Brian Grinstead [:bgrins]]

Attachment: selected-markupview.patch

Joe, can you take a look at these styling changes?  Basically, I'm using a new icon (+ a 2x) for the controls (checkbox on rule view and twisties on markupview and computed styles).

Also, setting the background to selected color and text to #f5f7fa when the node is selected in the markup view.

I've moved the controls CSS down lower in the theme files because it didn't make sense near the top (where color definitions are) and that became more obvious as that code got a bit bigger.

There is a try build for downloads here: https://tbpl.mozilla.org/?tree=Try&rev=ef44d7fcd4bb.

Comment 12

[Brian Grinstead [:bgrins]]

Attachment: markupview-selected.png

Screenshot of light and dark theme markupview with patch applied

Comment 13

[Joe Walker [:jwalker]]

Comment on attachment 8385397 [details] [diff] [review]
selected-markupview.patch

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

Looks good.

Comment 14

[Brian Grinstead [:bgrins]]

https://hg.mozilla.org/integration/fx-team/rev/36bebb7bf489
https://tbpl.mozilla.org/?tree=Fx-Team&rev=36bebb7bf489

Comment 15

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/36bebb7bf489