Created attachment 252256 [details] Can't navigate to any of these files or mentioned lines. Right now you have to go to the specific timeline view, then use the go-to arrow there. It would be good to show that on the overview timeline view rows as well.
<rdar://problem/20792727>
Yes, these should get goto arrows. We should be consistent and put goto arrows in the sidebar column for anything with a location.
Created attachment 252381 [details] Patch
Created attachment 252382 [details] Screenshot with Patch
Comment on attachment 252381 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=252381&action=review r=me The overall approach is good. If you can find a cleaner way to determine whether to show arrows, then even better! > Source/WebInspectorUI/ChangeLog:59 > + (.item > .status > .status-button): Fix an alignment issues with close and go-to arrows being side-by-side. issues -> issue > Source/WebInspectorUI/UserInterface/Views/TimelineSidebarPanel.css:196 > +.sidebar > .panel.navigation.timeline .item:not(:hover, .selected) .status .status-button, Have you experimented with showing a very dim go-to arrow when not hovered? I am a little worried about discoverability. > Source/WebInspectorUI/UserInterface/Views/TimelineSidebarPanel.js:364 > + if (!(treeElement instanceof WebInspector.ResourceTreeElement || treeElement instanceof WebInspector.ScriptTreeElement || This hardcoded list makes me nervous.. is there a programmatic way to determine whether the go-to arrow should be created? For example, is the existence of an overridden showContentViewForTreeElement sufficient to make a go-to arrow? What about canShowDifferentContentView? > Source/WebInspectorUI/UserInterface/Views/TimelineSidebarPanel.js:375 > + // Only create a go-to button for tree elements that can show a different content view when selected. So, for every tree element that could show a content view, we create a hidden close element for every tree element and add listeners eagerly. It seems like this hardcoded list is duplicated then, no?
Comment on attachment 252381 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=252381&action=review >> Source/WebInspectorUI/UserInterface/Views/TimelineSidebarPanel.css:196 >> +.sidebar > .panel.navigation.timeline .item:not(:hover, .selected) .status .status-button, > > Have you experimented with showing a very dim go-to arrow when not hovered? I am a little worried about discoverability. Or, maybe sticking with the visual language used (inconsistently) elsewhere and additionally underline the source code location text, to make it look like a link when not hovering the row.
(In reply to comment #6) > Comment on attachment 252381 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=252381&action=review > > >> Source/WebInspectorUI/UserInterface/Views/TimelineSidebarPanel.css:196 > >> +.sidebar > .panel.navigation.timeline .item:not(:hover, .selected) .status .status-button, > > > > Have you experimented with showing a very dim go-to arrow when not hovered? I am a little worried about discoverability. > > Or, maybe sticking with the visual language used (inconsistently) elsewhere > and additionally underline the source code location text, to make it look > like a link when not hovering the row. In using it I think it is fine. We use hover for other go-to arrows. And we force it to show when the row is selected.
Comment on attachment 252381 [details] Patch https://trac.webkit.org/r183821