Bug 66819 – Web Inspector: Directional arrow on element info box looks terrible

• Component: Web Inspector (Deprecated)
• Status: RESOLVED FIXED

[Matt Lilek]

The new little directional arrow on the yellow box that contains the name, ID, class and dimensions of a DOM node is very poorly drawn.

1) The bottom left portion should be shifted over by 1px so the first pixel doesn't draw on top of the box's border.
2) The tip is obnoxious.  Far too large.
3) The bottom right side draws on top of the box's border, as well as an extra pixel with it too.
4) The bottom right side needs to be shifted to avoid the same problem as (1).

Why was this even changed?  Just because callout arrows are in vogue doesn't mean every last UI element on every platform needs it.  It looks kind of ridiculous.

[Matt Lilek]

Created attachment 104926 [details]
Screenshot

[Pavel Feldman]

There were three requests / problems this change was addressing:

a. Numerous users requested that different colors are used for padding/margin/etc.
b. Title was rendered with plain monospace (different from elements panel), no color coding was used
c. Title box was aligned with the margin box and for small elements it resulted in the confusion: "does it overlap margin"?

(In reply to comment #0)
> The new little directional arrow on the yellow box that contains the name, ID, class and dimensions of a DOM node is very poorly drawn.
> 1) The bottom left portion should be shifted over by 1px so the first pixel doesn't draw on top of the box's border.

+1, definitely requires fixing.

> 2) The tip is obnoxious.  Far too large.

+1, Mac OS port problem, we are using smaller fonts on Mac, should be reflected in the title box.

> 3) The bottom right side draws on top of the box's border, as well as an extra pixel with it too.

+1, will fix.


> 4) The bottom right side needs to be shifted to avoid the same problem as (1).

+1, will fix.
> 
> Why was this even changed?  Just because callout arrows are in vogue doesn't mean every last UI element on every platform needs it.  It looks kind of ridiculous.

The reason behind the change is (c) from above. If you have an idea on how to make it more transparent / make arrow less rediculous, please let us know.

[Alexander Pavlov (apavlov)]

(In reply to comment #2)
> c. Title box was aligned with the margin box and for small elements it resulted in the confusion: "does it overlap margin"?

> > Why was this even changed?  Just because callout arrows are in vogue doesn't mean every last UI element on every platform needs it.  It looks kind of ridiculous.
> 
> The reason behind the change is (c) from above. If you have an idea on how to make it more transparent / make arrow less rediculous, please let us know.

Another data point: in a few places across the Web Inspector UI, we use a popover which is exactly a callout with an arrow, having a similar appearance, so this concept is familiar to our users.

In addition to the point (c) from Pavel, some people (especially when the info box was just introduced) claimed that they were unable to see the [border of an] adjoining element above/below the selected one.

[Alexander Pavlov (apavlov)]

Created attachment 105003 [details]
[PATCH] Suggested fix

[Alexander Pavlov (apavlov)]

Created attachment 105010 [details]
[PATCH] Remove unnecessary adjustment term

[Alexander Pavlov (apavlov)]

Created attachment 105011 [details]
[PATCH] Remove unnecessary path clipping

[Pavel Feldman]

Comment on attachment 105011 [details]
[PATCH] Remove unnecessary path clipping

View in context: https://bugs.webkit.org/attachment.cgi?id=105011&action=review

> Source/WebCore/inspector/DOMNodeHighlighter.cpp:336
> +        for (int i = 0; i < 8; ++i)

Translating canvas 0.5, 0.5 would effectively do the same thing, but in a more elegant manner.

[Alexander Pavlov (apavlov)]

Committed r93768: <http://trac.webkit.org/changeset/93768>

[Alexander Pavlov (apavlov)]

Created attachment 105153 [details]
[PATCH] Patch for landing (EWS check)

[Alexander Pavlov (apavlov)]

Created attachment 105156 [details]
[PATCH] For EWS

[Alexander Pavlov (apavlov)]

Rolled out by r93770

[Alexander Pavlov (apavlov)]

Created attachment 105158 [details]
[PATCH] For EWS (bug reopened)

[Alexander Pavlov (apavlov)]

Committed r93772: <http://trac.webkit.org/changeset/93772>

[Alexander Pavlov (apavlov)]

(In reply to comment #7)
> (From update of attachment 105011 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=105011&action=review
> 
> > Source/WebCore/inspector/DOMNodeHighlighter.cpp:336
> > +        for (int i = 0; i < 8; ++i)
> 
> Translating canvas 0.5, 0.5 would effectively do the same thing, but in a more elegant manner.

Landed a variation of patch 105011 with this suggested change applied.