Bug 222747 – Web Inspector: border color of "grid" badge should match color of corresponding outline

• Component: Web Inspector
• Status: RESOLVED FIXED

[Nikita Vasilyev]

Say, if the particular grid outline is pink, its badge should have a pink border when activated.
This will allow to mentally grid elements in the DOM tree outline to their corresponding outlines (Maciej's feedback was that, currently, you have to interact with the Layout panel to understand the outline colors).

[Radar WebKit Bug Importer]

<rdar://problem/75042342>

[Nikita Vasilyev]

Created attachment 422275 [details]
Patch

[Nikita Vasilyev]

Created attachment 422276 [details]
[Video] With patch applied

[Blaze Burg]

Comment on attachment 422275 [details]
Patch

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

r=me with a question

> Source/WebInspectorUI/ChangeLog:18
> +        For text and background color, only use the hue of the selected color to keep "grid" text always readable.

Good idea.

> Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.css:38
> +    box-shadow: 0 0 1px hsla(0, 0%, 0%, 0.5);

Is this even noticeable? I couldn't see it in the attached video.

> Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js:2063
> +        let color = WI.overlayManager.getGridColorForNode(this.representedObject);

Is this fallible if the DOMNode is not known by WI.overlayManager?

[Devin Rousso]

Comment on attachment 422275 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js:2063
>> +        let color = WI.overlayManager.getGridColorForNode(this.representedObject);
> 
> Is this fallible if the DOMNode is not known by WI.overlayManager?

Somewhat along those lines, does this badge also appear in the Console?  Do we want that?

[Nikita Vasilyev]

Comment on attachment 422275 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.css:38
>> +    box-shadow: 0 0 1px hsla(0, 0%, 0%, 0.5);
> 
> Is this even noticeable? I couldn't see it in the attached video.

It's more noticeable when the color is very light. Extreme example — white.

>>> Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js:2063
>>> +        let color = WI.overlayManager.getGridColorForNode(this.representedObject);
>> 
>> Is this fallible if the DOMNode is not known by WI.overlayManager?
> 
> Somewhat along those lines, does this badge also appear in the Console?  Do we want that?

BJ: It wouldn't have `this._gridBadgeElement` and would return a few lines above (line 2057).
Looking at this code again, I think I should move this line inside of the `if (isGridVisible) {...}`.

Devin: yes, it does. I think we do want that.

[Nikita Vasilyev]

Created attachment 422385 [details]
Patch

[EWS]

Committed r273992: <https://commits.webkit.org/r273992>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 422385 [details].