RESOLVED FIXED Bug 139151
Web Inspector: selecting Paint timeline records from sidebar does not trigger preview of paint quad
https://bugs.webkit.org/show_bug.cgi?id=139151
Summary Web Inspector: selecting Paint timeline records from sidebar does not trigger...
Brian Burg
Reported 2014-12-01 14:13:22 PST
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.
Attachments
[PATCH] Proposed Fix - Making Hovering DataGrid show Paint Highlight Rects (2.73 KB, patch)
2014-12-04 14:35 PST, Joseph Pecoraro
no flags
[PATCH] Proposed Fix (14.84 KB, patch)
2014-12-04 17:00 PST, Joseph Pecoraro
burg: review+
Radar WebKit Bug Importer
Comment 1 2014-12-01 14:13:31 PST
Joseph Pecoraro
Comment 2 2014-12-04 13:52:22 PST
We used to do this before the new timeline redesign.
Joseph Pecoraro
Comment 3 2014-12-04 14:17:28 PST
Oh, I see. We do for the DataGrid but not for the sidebar.
Joseph Pecoraro
Comment 4 2014-12-04 14:35:25 PST
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
Comment 5 2014-12-04 14:35:44 PST
Created attachment 242588 [details] [PATCH] Proposed Fix - Making Hovering DataGrid show Paint Highlight Rects
Joseph Pecoraro
Comment 6 2014-12-04 17:00:31 PST
Created attachment 242600 [details] [PATCH] Proposed Fix Give this a try. Is it too spammy?
Joseph Pecoraro
Comment 7 2014-12-04 17:01:50 PST
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 8 2015-01-14 21:55:33 PST
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
Comment 9 2015-01-15 17:01:07 PST
Note You need to log in before you can comment on or make changes to this bug.