Bug 222429

Summary: Web Inspector: Jump from Layout panel to grid container element
Product: WebKit Reporter: Razvan Caliman <rcaliman>
Component: Web InspectorAssignee: Razvan Caliman <rcaliman>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, hi, inspector-bugzilla-changes, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
[Image] With patch applied
none
[Video] Behavior with patch applied
none
Patch none

Description Razvan Caliman 2021-02-25 11:05:11 PST
Remove current behavior from label and let it just toggle the checkbox.

Add an arrow icon next to the color swatch on hover over the row (see CSS Variables). 
Clicking on the arrow should jump to element in context in the DOM tree of the Elements Tab
Comment 1 Radar WebKit Bug Importer 2021-02-25 11:05:29 PST
<rdar://problem/74751801>
Comment 2 Razvan Caliman 2021-03-08 08:12:06 PST
Created attachment 422567 [details]
Patch
Comment 3 Razvan Caliman 2021-03-08 08:20:21 PST
Created attachment 422568 [details]
[Image] With patch applied
Comment 4 Razvan Caliman 2021-03-08 08:21:09 PST
Created attachment 422570 [details]
[Video] Behavior with patch applied
Comment 5 Devin Rousso 2021-03-08 09:51:27 PST
Comment on attachment 422567 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Views/CSSGridSection.css:55
> +    opacity: 0;

Why not `display: none;`?

> Source/WebInspectorUI/UserInterface/Views/CSSGridSection.js:136
> +            buttonElement.title = WI.UIString("Click to inspect element");

This should be named something like "Reveal in DOM Tree" (you can use `WI.repeatedUIString.revealInDOMTree()`) to match the naming convention of the `title` of other `WI.createGoToArrowButton`.
Comment 6 Razvan Caliman 2021-03-08 10:03:44 PST
Comment on attachment 422567 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Views/CSSGridSection.css:55
>> +    opacity: 0;
> 
> Why not `display: none;`?

Long node display names are truncated in such a way that the color swatch is still visible and the far end.

If we force a relayout on hover with `display: none` -> `display: inline-block`, this will shift the truncation point and the position of the color swatch element. See the homepage of https://stripe.com for really long node display names.

I originally used `visibility: hidden`, but I got some strange side-effects on scroll resulting in misplaced elements. I didn't investigate, but it's probably a compound problem of scroll + visibility +  flex container and whatever else contributes to layout in the ancestor tree.

I stuck with `opacity: 0` because it didn't have side-effects and still used the arrow space so that the truncation point and position of the swatch were not affected on hover.

>> Source/WebInspectorUI/UserInterface/Views/CSSGridSection.js:136
>> +            buttonElement.title = WI.UIString("Click to inspect element");
> 
> This should be named something like "Reveal in DOM Tree" (you can use `WI.repeatedUIString.revealInDOMTree()`) to match the naming convention of the `title` of other `WI.createGoToArrowButton`.

Will do.
Comment 7 Razvan Caliman 2021-03-08 10:08:15 PST
Created attachment 422579 [details]
Patch
Comment 8 Devin Rousso 2021-03-09 10:22:34 PST
Comment on attachment 422579 [details]
Patch

r=me
Comment 9 EWS 2021-03-09 10:32:12 PST
Committed r274157: <https://commits.webkit.org/r274157>

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