Web Inspector: selecting Paint timeline records from sidebar does not trigger preview of paint quad

• Component: Web Inspector
• Status: RESOLVED FIXED

[Brian Burg]

1. Create a timeline recording with paint records.
2. Select the Layout timeline.
3. Select a paint record by clicking its row in the table (i.e., in <N>px cell).

The relevant rect should be highlighted on the page overlay.

However, if instead you click on the TreeOutline row (to the left of the record) nothing is highlighted.

[Radar WebKit Bug Importer]

<rdar://problem/19106598>

[Joseph Pecoraro]

We used to do this before the new timeline redesign.

[Joseph Pecoraro]

Oh, I see. We do for the DataGrid but not for the sidebar.

[Joseph Pecoraro]

Actually I think showing the highlight only once you've selected a record is cumbersome. How about just hovering over an element in the datagrid, like hovering elements in the DOM Tree? That way you can very quickly get highlights and see where the paint is associated with.

We could retitle this: "Cumbersome to view paint highlights, should be easier". We could make it so hovering in the sidebar triggers the highlight too, but I think if hovering in the data grid is enabled it easy enough.

Patch attached, thoughts?

[Joseph Pecoraro]

Created attachment 242588 [details]
[PATCH] Proposed Fix - Making Hovering DataGrid show Paint Highlight Rects

[Joseph Pecoraro]

Created attachment 242600 [details]
[PATCH] Proposed Fix

Give this a try. Is it too spammy?

[Joseph Pecoraro]

Comment on attachment 242600 [details]
[PATCH] Proposed Fix

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

> Source/WebInspectorUI/UserInterface/Views/TreeOutline.js:373
> +}

Nit: missing semicolon.

[Brian Burg]

Comment on attachment 242600 [details]
[PATCH] Proposed Fix

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

r=me

It doesn't look too spammy to me. In fact, it's kind of hard to follow the hovered row. Is there a way to add a table row hover highlight within the current design language?

> Source/WebInspectorUI/UserInterface/Views/LayoutTimelineView.js:279
> +        const contentColor = {r: 111, g: 168, b: 220, a: 0.66};

(Aside: I'm surprised WebInspector.Color doesn't have a .toProtocol() method)

> Source/WebInspectorUI/UserInterface/Views/LayoutTimelineView.js:290
> +        var rect = record.rect;

Nit: Could we put this case first and avoid detecting DOMAgent.highlightQuad?

> Source/WebInspectorUI/UserInterface/Views/LayoutTimelineView.js:300
> +            this._showingHighlight = false;

Nit: I would switch these lines so the member lines up.

[Joseph Pecoraro]

http://trac.webkit.org/changeset/178544