1108325 - Switch box-model colors to prettier theme-colors

Status: NEW

(DevTools :: Framework, defect)

Description

[Tim Nguyen :ntim]

Attachment: box-model-theme-colors.png

The new Firebug-inspired colors don't look good. It'd be nice to change them to our theme colors.

I've attached a mockup of how it could look like.
Colors :
- Margins
It uses the current yellow, because we don't have a yellow theme color.
--theme-highlight-lightorange doesn't have enough contrast.

- Border : --theme-highlight-purple
- Padding : --theme-highlight-red
- Content : --theme-highlight-blue

What do you think ?

Comment 1

[Tim Nguyen :ntim]

For the content area, we'll use the dark theme colors, since it has more contrast with web content.

Comment 2

[Tim Nguyen :ntim]

(In reply to Tim Nguyen [:ntim] from comment #1)
> For the content area, we'll use the dark theme colors, since it has more
> contrast with web content.

Actually, it'd be a mix of dark and light theme colors for the content area.

Comment 3

[Tim Nguyen :ntim]

Attachment: Content area + Light theme screenshot

Comment 4

[Tim Nguyen :ntim]

Attachment: Patch

Comment 6

[Tim Nguyen :ntim]

Attachment: Patch v1.1

Changed the yellow to a slightly stronger one.

Comment 7

[Brian Grinstead [:bgrins]]

There are multiple considerations here:

1. we definitely want the colors from the 'box model' panel to match the ones that show up over page content (regardless of toolbox theme choice)
2. picking colors that are familiar from other tools has some amount of value
3. we want colors that have a good amount of contrast from each other

Given point 1, I just don't think choosing colors from our palette will work, since it will necessarily not match the page overlay depending on the toolbox theme.  Since we need to pick some set of colors that will work across themes and will not match our theme colors anyway, my assertion was that we should use the set of colors from Firebug, since it seemed to satisfy points 2 and 3.

Also see this [post](https://groups.google.com/forum/#!topic/mozilla.dev.developer-tools/fwjKu2QPPT0) and bug 989053 for more context on point 3.

Comment 8

[Brian Grinstead [:bgrins]]

We do have other options - we could go back to the colors we had before the change but tweak the opacity on the in-page highlighter to try and make them more visible, we could use the colors that Chrome uses instead, or we could just randomly choose some colors to use in both themes mixing and matching from the existing palettes.

But I'd like to know which part specifically looks bad so we can make a good decision.  I'm assuming it's the box model panel in the toolbox and not the in-content overlay?

Comment 9

[Brian Grinstead [:bgrins]]

Comment on attachment 8532883 [details]
box-model-theme-colors.png

See Comment 7 point 1.  This will cause a mismatch between the colors in the toolbox and the colors on the in-content highlight.

Comment 10

[Tim Nguyen :ntim]

(In reply to Brian Grinstead [:bgrins] from comment #7)
> There are multiple considerations here:
> 
> 1. we definitely want the colors from the 'box model' panel to match the
> ones that show up over page content (regardless of toolbox theme choice)
> 2. picking colors that are familiar from other tools has some amount of value
> 3. we want colors that have a good amount of contrast from each other
> 
> Given point 1, I just don't think choosing colors from our palette will
> work, since it will necessarily not match the page overlay depending on the
> toolbox theme.  Since we need to pick some set of colors that will work
> across themes and will not match our theme colors anyway, my assertion was
> that we should use the set of colors from Firebug, since it seemed to
> satisfy points 2 and 3.
> 
> Also see this
> [post](https://groups.google.com/forum/#!topic/mozilla.dev.developer-tools/
> fwjKu2QPPT0) and bug 989053 for more context on point 3.

I can fix point 1 easily. For point 2, we can't really do anything about it but go back to our old colors or switch to Chrome colors. For point 3, I did my best to create much contrast between each box model color.

Please go on IRC, so we can discuss this.

Comment 11

[Tim Nguyen :ntim]

(In reply to Brian Grinstead [:bgrins] from comment #8)
> We do have other options - we could go back to the colors we had before the
> change but tweak the opacity on the in-page highlighter to try and make them
> more visible, we could use the colors that Chrome uses instead, or we could
> just randomly choose some colors to use in both themes mixing and matching
> from the existing palettes.
> 
> But I'd like to know which part specifically looks bad so we can make a good
> decision.  I'm assuming it's the box model panel in the toolbox and not the
> in-content overlay?

Both look pretty bad.

Comment 12

[Brian Grinstead [:bgrins]]

(In reply to Tim Nguyen [:ntim] from comment #10)
> I can fix point 1 easily. For point 2, we can't really do anything about it
> but go back to our old colors or switch to Chrome colors. For point 3, I did
> my best to create much contrast between each box model color.
> 
> Please go on IRC, so we can discuss this.

I don't have time right this moment to discuss this - please post a proposal if you have one for handing point 1 / 2.  Regarding point 2, I don't see a strong reason to go with a brand new set of colors as opposed to using a set that is already familiar, especially considering that we won't be able to use each theme's highlight colors.

Comment 13

[Tim Nguyen :ntim]

Here are some other proposals :
Proposal 1 : Theme equivalents to old colors
- Content : #46afe3
- Padding : #2cbb0f
- Border : (old yellow) or #d99b28
- Margin : #ed2655 or #eb5368

Proposal 2 : Mix of Firebug and old colors (the theme equivalents)
- Content : #46afe3 (old color)
- Padding : #b82ee5 (Firebug)
- Border : (old yellow) or #d99b28 (old color)
- Margin : #ed2655 or #eb5368 (old color)

Proposal 2 is better for contrast, and proposal 1 is better for point 2.

Comment 14

[Tim Nguyen :ntim]

(In reply to Tim Nguyen [:ntim] from comment #13)
> Here are some other proposals :
> Proposal 1 : Theme equivalents to old colors
> - Content : #46afe3
> - Padding : #2cbb0f
> - Border : (old yellow) or #d99b28
> - Margin : #ed2655 or #eb5368
Small correction, margin : #d99b28

Comment 15

[Tim Nguyen :ntim]

Attachment: Proposal 1

I don't have time to test the colors for the content area, I'll probably get to it next week.

Comment 16

[Tim Nguyen :ntim]

Attachment: Proposal 1b

Alternative to proposal 1 with higher contrast.
Also uses theme colors (except for yellow)

Comment 17

[Tim Nguyen :ntim]

Attachment: Proposal 1c

Proposal 1c but with more pale colors.

This time with all colors coming from the devtools palette.

Comment 18

[Tim Nguyen :ntim]

Attachment: Proposal 2

Comment 19

[Patrick Brosset <:pbro>]

Thanks for these proposals Tim.

It's not an easy bug to fix. One of the reasons Brian an I decided to go ahead with the new colors was that doing it would certainly trigger some kind of discussion and we would have time to change the colors again during this cycle. And this is exactly what we're doing now.

So, thanks for filing. We need to get more eyes on this bug though, as it's not an easy set of colors to choose.

One of the thing your proposals do not show is how well the colors work on the page, when using the highlighter. It's important to check that, because the highlighter has a different opacity setting, and can be shown on any element, so on any background color.

Comment 20

[Brian Grinstead [:bgrins]]

(In reply to Patrick Brosset [:pbrosset] [:patrick] from comment #19)
> One of the thing your proposals do not show is how well the colors work on
> the page, when using the highlighter. It's important to check that, because
> the highlighter has a different opacity setting, and can be shown on any
> element, so on any background color.

Here is a test page I used for checking the highlighter against different backgrounds: http://fiddle.jshell.net/bgrins/64LvR/show/

Comment 21

[Brian Grinstead [:bgrins]]

Another option would be to just slightly tweak a couple of the colors (black/purple) a bit to be more visually pleasing.  This would allow us to keep the firebug feel / contrast.

Alternatively, we could set opacity: 1 to get rid of that washed-out look that was mentioned in bug 1037785.
* Dark: https://www.dropbox.com/s/vdjjinbg2zy9pc6/Screenshot%202014-12-09%2009.44.22.png?dl=0
* Light: https://www.dropbox.com/s/ywoa5l63j08xpti/Screenshot%202014-12-09%2009.44.53.png?dl=0

Comment 22

[Tim Nguyen :ntim]

(In reply to Patrick Brosset [:pbrosset] [:patrick] from comment #19)
> Thanks for these proposals Tim.
> 
> It's not an easy bug to fix. One of the reasons Brian an I decided to go
> ahead with the new colors was that doing it would certainly trigger some
> kind of discussion and we would have time to change the colors again during
> this cycle. And this is exactly what we're doing now.
> 
> So, thanks for filing. We need to get more eyes on this bug though, as it's
> not an easy set of colors to choose.
> 
> One of the thing your proposals do not show is how well the colors work on
> the page, when using the highlighter. It's important to check that, because
> the highlighter has a different opacity setting, and can be shown on any
> element, so on any background color.
I don't have the time to test each proposal, so if you could choose the ones you are leaning towards to, it would save me some time.

(In reply to Brian Grinstead [:bgrins] from comment #21)
> Another option would be to just slightly tweak a couple of the colors
> (black/purple) a bit to be more visually pleasing.  This would allow us to
> keep the firebug feel / contrast.
I guess I could try that, but between content and padding, the color difference isn't that big though (they are both made of blue).

> Alternatively, we could set opacity: 1 to get rid of that washed-out look
> that was mentioned in bug 1037785.
> * Dark:
> https://www.dropbox.com/s/vdjjinbg2zy9pc6/Screenshot%202014-12-09%2009.44.22.
> png?dl=0
> * Light:
> https://www.dropbox.com/s/ywoa5l63j08xpti/Screenshot%202014-12-09%2009.44.53.
> png?dl=0
It still doesn't look good since the colors aren't visually pleasing. Also, that bug was reported before we switched to firebug colors.

Comment 23

[Brian Grinstead [:bgrins]]

(In reply to Tim Nguyen [:ntim] from comment #22)
> (In reply to Patrick Brosset [:pbrosset] [:patrick] from comment #19)
> > Thanks for these proposals Tim.
> > 
> > It's not an easy bug to fix. One of the reasons Brian an I decided to go
> > ahead with the new colors was that doing it would certainly trigger some
> > kind of discussion and we would have time to change the colors again during
> > this cycle. And this is exactly what we're doing now.
> > 
> > So, thanks for filing. We need to get more eyes on this bug though, as it's
> > not an easy set of colors to choose.
> > 
> > One of the thing your proposals do not show is how well the colors work on
> > the page, when using the highlighter. It's important to check that, because
> > the highlighter has a different opacity setting, and can be shown on any
> > element, so on any background color.
> I don't have the time to test each proposal, so if you could choose the ones
> you are leaning towards to, it would save me some time.
> 
> (In reply to Brian Grinstead [:bgrins] from comment #21)
> > Another option would be to just slightly tweak a couple of the colors
> > (black/purple) a bit to be more visually pleasing.  This would allow us to
> > keep the firebug feel / contrast.
> I guess I could try that, but between content and padding, the color
> difference isn't that big though (they are both made of blue).
> 
> > Alternatively, we could set opacity: 1 to get rid of that washed-out look
> > that was mentioned in bug 1037785.
> > * Dark:
> > https://www.dropbox.com/s/vdjjinbg2zy9pc6/Screenshot%202014-12-09%2009.44.22.
> > png?dl=0
> > * Light:
> > https://www.dropbox.com/s/ywoa5l63j08xpti/Screenshot%202014-12-09%2009.44.53.
> > png?dl=0
> It still doesn't look good since the colors aren't visually pleasing. Also,
> that bug was reported before we switched to firebug colors.

This is all pretty subjective, I think.  This is the only report we've gotten about the colors not looking good so far, so I'd like to wait for more feedback to collect before making any changes.  My guess is that people aren't noticing the color changes, especially on the in-page highlighter.

Comment 24

[Helen V. Holmes (:helenvholmes) (:✨)]

Attachment: box-model-in-page-mockup.png

I'm uncertain if this bug was meant for Firebug or for regular devtools. Anyway, here's a spec I've been working on of the box model and the box model in context.

The colors don't match any of our current variables, unfortunately. We might consider making a group for the box model alone?

Comment 25

[Helen V. Holmes (:helenvholmes) (:✨)]

Attachment: box-model-in-page-mockup.png

Comment 26

[Helen V. Holmes (:helenvholmes) (:✨)]

Attachment: box-model-in-page-mockup.png

Comment 27

[Helen V. Holmes (:helenvholmes) (:✨)]

Comment on attachment 8750336 [details]
box-model-in-page-mockup.png

Tim, would you mind glancing over this?

Comment 29

[Tim Nguyen :ntim]

Comment on attachment 8750336 [details]
box-model-in-page-mockup.png

I love the new colours!
I do have one issue with the legend though. You seem to be assigning the light-blue colour to borders, but you're not using that colour anywhere inside the page highlighter. Also, how does it look for elements with large borders (let's say 10px for example)?

Comment 30

[Patrick Brosset <:pbro>]

I have a few questions about attachment 8750336 [details]:
- In a comment, it says that the boxes don't show the colors underneath them. What do you mean by that? Should the boxes be opaque and not show what's below? Transparency is really useful.
- The content box seems to have no fill color, which would make the highlighter invisible for elements that don't have borders or margins or paddings.
- It looks like the highlighter regions have 1/2px strokes around them. Like, the padding region is a light green, and it has a darker green stroke around it. I don't know how this will work when you have a very small padding (2px for instance). To me it seems like this would get in the way in some cases.

I think we should try this out on a variety of test cases.

Changing the fill/stroke/opacity of the boxes should be rather easy in https://dxr.mozilla.org/mozilla-central/source/devtools/server/actors/highlighters.css

Comment 31

[Brian Grinstead [:bgrins]]

(In reply to Patrick Brosset <:pbro> from comment #30)
> I think we should try this out on a variety of test cases.

Agreed - I have this page from Bug 989053 which has one simple test against dark and light backgrounds: http://jsfiddle.net/bgrins/64LvR/.   This page might also be helpful: https://bgrins.github.io/devtools-demos/inspector/positions.html.  But it'd be nice to have a test page with a bunch of variations of box model values to test this and make sure it looks good across them.

Comment 32

[Helen V. Holmes (:helenvholmes) (:✨)]

Attachment: box-model-in-page-mockup.png

(In reply to Tim Nguyen :ntim from comment #29)
> I do have one issue with the legend though. You seem to be assigning the
> light-blue colour to borders, but you're not using that colour anywhere
> inside the page highlighter. Also, how does it look for elements with large
> borders (let's say 10px for example)?

I created a small version of it in the spec—I think it's too small to read the content in it (it's a Codepen screenshot), but it just says, "This is a div with a 20px tomato border."

(In reply to Patrick Brosset <:pbro> from comment #30)
> I have a few questions about attachment 8750336 [details]:
> - In a comment, it says that the boxes don't show the colors underneath
> them. What do you mean by that? Should the boxes be opaque and not show
> what's below? Transparency is really useful.
The box model will still be transparent—I didn't explain this very well in my mockup so I tried again, this time illustrating it. If the picture still doesn't make sense, I'll try and explain it in the bug.

> - The content box seems to have no fill color, which would make the
> highlighter invisible for elements that don't have borders or margins or
> paddings.
I've bumped the contrast on this—it did have a faint blue but you're right, too faint.

> - It looks like the highlighter regions have 1/2px strokes around them.
> Like, the padding region is a light green, and it has a darker green stroke
> around it. I don't know how this will work when you have a very small
> padding (2px for instance). To me it seems like this would get in the way in
> some cases.
> 
> I think we should try this out on a variety of test cases.
I instead was lazy and just removed the borders instead—I'd like to keep the borders on the box model in the Computed area, but I'm not particularly attached to it in-page.

Comment 33

[Patrick Brosset <:pbro>]

Copying a comment from jonathan from bug 1128209 here since it really belongs in this bug.

(In reply to jonathan from bug 1128209 comment #13)
> I'm investigating the coloring for Chrome DevTools currently as well based
> on user feedback. In my research so far [1], I've found the *current*
> Firefox coloring to be (in my opinion) the superior choice. The reason being
> it provides the most consistent coloring for users with different types of
> color blindness. I think this should be one of the factors upon deciding on
> a new color scheme and I'd also like to see if we can't get all the major
> tooling vendors to align with these. That way users get the same experience
> across each tool for such a critical component to designing.
> 
> The current proposed color scheme is mostly-stable with different common
> color blindness forms, however it still varies more heavily compared to the
> existing scheme.
> 
> I'm still very new to understanding colors for accessibility, but hopefully
> some of the existing work can help guide future designs. I'm still looking
> to get actual a11y experts into the discussion to help out as well since
> they've probably dealt with this kind of situation before.
> 
> [1] https://bugs.chromium.org/p/chromium/issues/detail?id=614722

Comment 34

[Patrick Brosset <:pbro>]

Attachment: box-model-in-page-mockup.png

Here's the latest mockup from bug 1128209 (I've made bug 1128209 only about removing the guides, because that's what it originally was about, let's keep this one for changing the colors).

Comment 35

[Patrick Brosset <:pbro>]

Comment on attachment 8760640 [details]
box-model-in-page-mockup.png

Aesthetically speaking, I really like the colors. They go way better with our themes.
As discussed in comment 32, keeping the lines around each of the areas in the box-model tab is fine, but let's not have them in the in-page highlighter since that makes it hard to see small margins/paddings/borders.

In comment 33, jonathan says the FF colors are superior because they are more visible by everyone. How do these new colors compare?

About the content area: in the spec, I see it having a #EDF3FE border (using the eye-dropper), so judging by the other areas, I guess it will behave the same and have the same color for the background with a lower opacity.
I ask because I don't see it written anywhere in the spec.
Also, I still see find very hard to see. I'll attach 2 screenshots later of a page that has a single element in the center with no border, padding or margin, so that the highlighter only has a content area. I think this needs to be more visible.

In the spec it says that "boxes are cut out". We could either use an arrangement of DIVs to achieve this or, more simply, SVG.

Comment 36

[Patrick Brosset <:pbro>]

Attachment: content-color.png

Here's what the highlighter would look like on an element that has only a content area, over a white background.

Comment 37

[Patrick Brosset <:pbro>]

Attachment: content-color-on-eeeeee.png

Here's what the highlighter would look like on an element that has only a content area, over a #eeeeee background.

Comment 38

[Yura Zenevich [:yzen] (please use "needinfo")]

Comment on attachment 8760640 [details]
box-model-in-page-mockup.png

The mockup looks really nice IMO. The borders between boxes help with contrast between border/margin. It would be nice to have a little bit more contrast in this case but I'm not too firm on that. The only other concern I have is that the font/background contrast for margin is getting into an area where a lighter font would feel more accessible. What do you think?

Comment 39

[Helen V. Holmes (:helenvholmes) (:✨)]

(In reply to Patrick Brosset <:pbro> from comment #35)
> Comment on attachment 8760640 [details]
> box-model-in-page-mockup.png
> In comment 33, jonathan says the FF colors are superior because they are
> more visible by everyone. How do these new colors compare?
Jonathan, can you give us more information about how that data was obtained? It sounded like user testing of some sort? Could we potentially replicate the experiment with the new colors to test?

> About the content area: in the spec, I see it having a #EDF3FE border (using
> the eye-dropper), so judging by the other areas, I guess it will behave the
> same and have the same color for the background with a lower opacity.
> I ask because I don't see it written anywhere in the spec.
I can annotate this.

> Also, I still see find very hard to see. I'll attach 2 screenshots later of
> a page that has a single element in the center with no border, padding or
> margin, so that the highlighter only has a content area. I think this needs
> to be more visible.
Hm, good point, although arguably any color could potentially end up matching a background-color perfectly and become invisible, which is why it has the opacity.

Comment 40

[Helen V. Holmes (:helenvholmes) (:✨)]

(In reply to Yura Zenevich [:yzen] from comment #38)
> Comment on attachment 8760640 [details]
> box-model-in-page-mockup.png
> 
> The mockup looks really nice IMO. The borders between boxes help with
> contrast between border/margin. It would be nice to have a little bit more
> contrast in this case but I'm not too firm on that. The only other concern I
> have is that the font/background contrast for margin is getting into an area
> where a lighter font would feel more accessible. What do you think?
I could swap it to white—it does pass AA, though.

Comment 41

[Helen V. Holmes (:helenvholmes) (:✨)]

(In reply to Patrick Brosset <:pbro> from comment #37)
> Created attachment 8760655 [details]
> content-color-on-eeeeee.png
> 
> Here's what the highlighter would look like on an element that has only a
> content area, over a #eeeeee background.

I think we need to solve this before this can be shipped. Does the current color scheme handle the problem that at some color the content color can completely fade into the background, or do we rely on the guides to solve this issue?

Some proposals from last week included using blend modes and having a hex value changing point where we change from light color to a dark color and vice-versa depending upon the background.

Comment 42

[Norman Paschke]

The current colors (as shown in box-model-in-page-mockup.png) are not very different from another.

We used to have: blue, purple, dark grey and yellow. Now we have: light blue, green, blue and dark blue.. how am I supposed to know what I am currently looking at if it's all blue? Especially against certain backgrounds it is nearly impossible. Also, what about people with color blindness/deficiency?

I have nothing against changing colors but make sure they are distinctive.

Comment 43

[Patrick Brosset <:pbro>]

I think Helen can provide an answer to comment 42, but here are a couple of comments:

> Especially against certain backgrounds it is nearly impossible.
I agree, this is something I said in comment 37, and that Helen said we should fix in comment 41. I'm wondering if this got lost somewhere. I'll follow this up.

> Also, what about people with color blindness/deficiency?
I'm fairly certain the new colors were given a thorough contrast check with an accessibility tool. But Helen would know more.

Also, it looks like the colors were changed in bug 1291638, so this bug should be closed I guess.

Comment 44

[Helen V. Holmes (:helenvholmes) (:✨)]

(In reply to Patrick Brosset <:pbro> from comment #43)
> > Especially against certain backgrounds it is nearly impossible.
> I agree, this is something I said in comment 37, and that Helen said we
> should fix in comment 41. I'm wondering if this got lost somewhere. I'll
> follow this up.
Fell through the cracks as a result of bug 1291638, comment #17, before this particular issue could be fixed. For bug 1291638 I think we might look into blend modes on certain backgrounds. How did the old highlighter handle the issue of, there-might-be-a-background-that-perfectly-matches-it?

> > Also, what about people with color blindness/deficiency?
> I'm fairly certain the new colors were given a thorough contrast check with
> an accessibility tool. But Helen would know more.
Passed through an accessibility tool and the accessibility team. Passes our WCAG AA guidelines.

Comment 45

[Patrick Brosset <:pbro>]

(In reply to Helen V. Holmes (:helenvholmes) (:✨) from comment #44)
> How did the old highlighter handle the
> issue of, there-might-be-a-background-that-perfectly-matches-it?
It didn't in fact. I filed bug 1301676 to look into this issue.
The only difference I see though is that the old highlighter had really contrasting/ugly colors that no one used :) so there was less chances of them blending in with the background of the site.

I like the blend modes idea. I'll add that to bug 1301676.