It's exceptionally hard to select/expand/contract the tree view disclosures. If the user wants to use the keyboard arrows, or mis-clicks on the disclosure triangle, then they are whisked away to a different content view. I've gotten really frustrated by this when trying to expand deep trees. I think that behavior could be moved to command/ctrl + click. Some of the rows also have go-to arrows. We could add more of those if it's hard to view resources otherwise.
<rdar://problem/16728603>
FWIW, clicking a row in the content view works, and keyboard navigation (expand and contract too) from there works. I'm not opposed to doing something different here. Maybe go-to arrows is right.
(In reply to comment #2) > FWIW, clicking a row in the content view works, and keyboard navigation (expand and contract too) from there works. Yeah, I figured it out eventually. But if you are staring at the text in the sidebar with the intent of exploring subtees (by looking at where the disclosures are), it's a rude surprise. > I'm not opposed to doing something different here. Maybe go-to arrows is right. Another thing I raised with Joe on IRC is that this scheme would let us highlight data points for a specific row on the timeline, when the relevant row is selected.
Created attachment 251206 [details] [Patch] Proposed Fix
Comment on attachment 251206 [details] [Patch] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=251206&action=review Nice work! > Source/WebInspectorUI/UserInterface/Views/LayoutTimelineView.js:35 > + var columns = {eventType: {}, location: {}, width: {}, height: {}, startTime: {}, totalTime: {}}; Was this actually functionally broken, or just not matching column names of other TimelineView subclasses? > Source/WebInspectorUI/UserInterface/Views/NetworkTimelineView.js:158 > + console.error("Unknown tree element selected."); Please log treeElement as a second parameter. It eliminates a debugging step that you would always have to do to investigate this error message. > Source/WebInspectorUI/UserInterface/Views/TimelineView.js:191 > + console.error("Unknown tree element selected."); As above. > Source/WebInspectorUI/UserInterface/Views/TimelineView.js:249 > + if (this.navigationSidebarTreeOutline.selectedTreeElement) The existing logic is fine for this patch. I related an existing bug that proposes improving the logic.
Comment on attachment 251206 [details] [Patch] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=251206&action=review >> Source/WebInspectorUI/UserInterface/Views/LayoutTimelineView.js:35 >> + var columns = {eventType: {}, location: {}, width: {}, height: {}, startTime: {}, totalTime: {}}; > > Was this actually functionally broken, or just not matching column names of other TimelineView subclasses? I broke this in https://trac.webkit.org/changeset/182660.
Created attachment 251357 [details] [Patch] Proposed Fix
Comment on attachment 251357 [details] [Patch] Proposed Fix Clearing flags on attachment: 251357 Committed r183134: <http://trac.webkit.org/changeset/183134>
All reviewed patches have been landed. Closing bug.