1141571 - Add a legend to the Box Model tool

Status: VERIFIED FIXED

(DevTools :: Inspector, defect, P2)

Description

[Jan Keromnes [:janx]]

The Box Model inspector is a great tool, but it's suboptimal that you have to know what each color is for by heart.

Therefore, it would be nice to show a legend underneath like:

    [#] margin (# is apple green)
    [#] border (# is grey)
    [#] padding (# is purple)
    [#] bounding client rect (# is sky blue)

Comment 1

[Jan Keromnes [:janx]]

Note: Hovering on a particular color does indicate what it represents, but as almost invisible white text on apple green background in the bottom right corner.

A proper legend would be more discoverable and readable, even in screenshots.

Comment 2

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

Setting devedition-40 bugs to P2, filter on C17C996C-A0BE-4153-8057-FAB642D0746D

Comment 3

[Jan Keromnes [:janx]]

I'll try to do this tomorrow.

Comment 4

[Jan Keromnes [:janx]]

Attachment: Add a legend to the Box Model tool.

Brian, please have a look.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0320e09f678c

Comment 5

[Jan Keromnes [:janx]]

Attachment: boxmodel-legend.png

Attached screenshot.

Note: For the color preview boxes, I chose a dotted outline with an offset of -1px, because it resembles the original more. However, it seems that on some zoom levels the offset looks more like -2px (probably a rounding error).

Maybe there is a better way to do the dotted outline?

Comment 6

[Jan Keromnes [:janx]]

Comment on attachment 8591640 [details] [diff] [review]
Add a legend to the Box Model tool.

(git bz forgot about the r? flag again)

Brian, please have a look.

This patch…
1. replaces the useless white-on-light-green, bottom-right tooltip with actual hover tooltips ("title" attribute).
2. defines the box model colors as CSS variables.
3. reuses these variables in a legend underneath the box model view.

Comment 7

[Patrick Brosset <:pbro>]

For info, we do have a legend already. It appears when you hover over regions, for now it's just the name of the region and it's placed in the margin box (bottom right corner).
You might not have seen it because we have a regression at the moment whereby it's not easily visible anymore (white text on yellow background): see bug 1150507.
I do like the idea of a more prominent legend as you propose here (although in the screenshot you attached, I think it might be a little too big), but if we go for this, we'll need to get rid of the other legend.

Comment 8

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

I think it might be better to have the legend text appearing inside the box model similar to chrome and safari. It would provide a better UX and avoid this color matching the user would have to go through to determine whether or not it is a margin, padding, or border.

Comment 9

[Jan Keromnes [:janx]]

Thanks for the feedback!

Actually, I had noticed the bottom-right text-on-hover (see comment 1 and comment 6), but I believe that ― even when readable ― it's not discoverable, and so it doesn't help with users who get confused about the colors and don't hover (this actually happened to me several times). And it doesn't help with screenshots either.

I replaced this weird tooltip mechanism with an actual `title` attribute, so that when you hover over any of the values, you still see the "margin-top" or "border-bottom" text in an actual tooltip. The legend is just there for convenience, but it doesn't take up valuable space (it can overflow at the bottom) and can be ignored as needed.

Comment 10

[Jan Keromnes [:janx]]

TL;DR: With my patch, users still see useful information on hover, but can also benefit from a proper legend if they need it.

Comment 11

[Brian Grinstead [:bgrins]]

Comment on attachment 8591640 [details] [diff] [review]
Add a legend to the Box Model tool.

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

Would you mind splitting this into two patches?  One that just replaces the old 'tooltip' mechanism with the title attribute, and a second that adds the legend.

Regarding the legend, I wonder if it would be easier for the user to have the text 'margin', 'border', 'padding' in the top left of the corresponding region.  See how Firebug does it, for example (I'll attach a screenshot momentarily)

Comment 12

[Brian Grinstead [:bgrins]]

Attachment: firebug-regions.png

See how there is text in the corner of each region.  I think this is nice since it's overlaid directly on what you are looking at

Comment 13

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

(In reply to Brian Grinstead [:bgrins] from comment #12)
> Created attachment 8591836 [details]
> firebug-regions.png
> 
> See how there is text in the corner of each region.  I think this is nice
> since it's overlaid directly on what you are looking at

I would definitely prefer this approach! Having the legend overlays directly in the box model reduces that extra step of looking at the legend and matching it with the box.

Comment 14

[Patrick Brosset <:pbro>]

(In reply to Brian Grinstead [:bgrins] from comment #11)
> Comment on attachment 8591640 [details] [diff] [review]
> Add a legend to the Box Model tool.
> 
> Review of attachment 8591640 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Would you mind splitting this into two patches?  One that just replaces the
> old 'tooltip' mechanism with the title attribute:
I have a patch in bug 1151956 ready to go that replaces all tooltip attributes with title attributes. But I wasn't as thorough as Jan, I didn't remove the old tooltip mechanism ... I havn't even seen we had this in place :|

Comment 15

[Jan Keromnes [:janx]]

(In reply to Gabriel Luong (:gl) from comment #13)
> (In reply to Brian Grinstead [:bgrins] from comment #12)
> > Created attachment 8591836 [details]
> > firebug-regions.png
> > 
> > See how there is text in the corner of each region.  I think this is nice
> > since it's overlaid directly on what you are looking at
> 
> I would definitely prefer this approach! Having the legend overlays directly
> in the box model reduces that extra step of looking at the legend and
> matching it with the box.

OK fair enough, I will use this approach instead.

Comment 16

[Jan Keromnes [:janx]]

Attachment: Replace box model's old tooltip mechanism with title attributes.

(In reply to Patrick Brosset [:pbrosset] [:patrick] from comment #14)
> I have a patch in bug 1151956 ready to go that replaces all tooltip
> attributes with title attributes. But I wasn't as thorough as Jan, I didn't
> remove the old tooltip mechanism ... I havn't even seen we had this in place
> :|

Whoops, sorry for the duplicate work!

You can either rebase your patch on top of mine, or land yours first. I'm fine either way :)

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5915ee441cd1

Comment 17

[Jan Keromnes [:janx]]

Attachment: Add a legend to the Box Model tool.

Sheriffs, please land the other patch; this one still needs r+.

Changed legend as suggested, I'll upload a screenshot.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=88cfdec022c8

Comment 18

[Jan Keromnes [:janx]]

Attachment: boxmodel-legend2.png

Screenshot of new box model legend.

Comment 19

[Brian Grinstead [:bgrins]]

Comment on attachment 8592281 [details] [diff] [review]
Add a legend to the Box Model tool.

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

Looks good

Comment 20

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/52a143ca5a1a
https://hg.mozilla.org/integration/fx-team/rev/0ceb71fdbe2f

Comment 21

[Ekanan Ketunuti]

https://hg.mozilla.org/mozilla-central/rev/52a143ca5a1a
https://hg.mozilla.org/mozilla-central/rev/0ceb71fdbe2f

Comment 23

[Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact [e-mail]]

I reproduced this issue using 47.0a1, build ID: 20160202030232, on Windows 10 x64.
I can confirm this issue is fixed, I verified using Fx 51.0b11, build ID: 20170103031119 and Fx 52.0a2, build ID: 20170105004018 and Fx53.0a1, build ID: 20170104030214, on Windows 10 x64, Mac OS X 10.11 and Ubuntu 14.04 LTS.

Cheers!