Summary: | Web Inspector: clicking Timelines tree view nodes should not change the current content view | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Brian Burg <burg> | ||||||
Component: | Web Inspector | Assignee: | Matt Baker <mattbaker> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, graouts, joepeck, mattbaker, simon.fraser, timothy, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 139727 | ||||||||
Attachments: |
|
Description
Brian Burg
2014-04-25 12:43:54 PDT
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. |