Summary: Debugger tab should support only the Scope Chain sidebar panel. The Resource panel doesn't fit in with the Debugger tab workflow, and only distracts from the (very important) Scope Chain panel.
<rdar://problem/31410771>
(In reply to Matt Baker from comment #0) > Summary: > Debugger tab should support only the Scope Chain sidebar panel. Reworded the bug. I'd forgotten about Probes.
(In reply to Matt Baker from comment #0) > Summary: > Debugger tab should support only the Scope Chain sidebar panel. > > The Resource panel doesn't fit in with the Debugger tab workflow, and only > distracts from the (very important) Scope Chain panel. I think that there are some cases where showing the Resource panel is useful. The Scope Chain panel is really only useful when the debugger has paused execution (with the exception of Watch Expressions). I've had situations where I've looked at a file via the Debugger tab and then gone on to find where its initiated (via the Resource panel) and put a breakpoint there. Personally, I think a setting would be more apt than just removing the Resource panel altogether. <https://webkit.org/b/149487>
Created attachment 306107 [details] Patch
I agree with Devin, the sidebar is useful for some things. It is the only place we show the full URL for example. That could be needed when debugging and it would be a pain to switch tabs and find the resource again.
Okay, I think a good solution will be to keep the Resource panel but reorder the panels making Scope Chain first. Since navigation items are added/removed as the currently inspected object changes, ContentBrowserTabContentView will need to maintain the ordering based on the order defined by TabContentView.detailsSidebarPanels.
Created attachment 306140 [details] Patch
Transient sidebars like Scope Chain should be last. The goal being that hit targets for labels would not move. That being said, the labels are center aligned so they move anyway. Does Devin's recent change make this not matter as much? https://trac.webkit.org/changeset/214847
Comment on attachment 306140 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=306140&action=review > Source/WebInspectorUI/UserInterface/Views/ContentBrowserTabContentView.js:154 > + WebInspector.detailsSidebar.addSidebarPanel(sidebarPanel, (a, b) => { Wouldn't we know the index it needs inserted at? You would be better off adding a inserSidebarPanel function that takes an index. No need to sort the array for one item insert when the order is known already. > Source/WebInspectorUI/UserInterface/Views/Sidebar.js:63 > + addSidebarPanel(sidebarPanel, comparator) The index implementation you add below can be used for a new insertSidebarPanel. And you can make addSidebarPanel use it to reduce duplication.
(In reply to Timothy Hatcher from comment #8) > Transient sidebars like Scope Chain should be last. The goal being that hit I think it's actually the Resource sidebar that is transient, Scope Chain is always shown.
Created attachment 306300 [details] Patch
Comment on attachment 306300 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=306300&action=review > Source/WebInspectorUI/UserInterface/Views/ContentBrowserTabContentView.js:157 > + let index = this.detailsSidebarPanels.indexOf(sidebarPanel) - hiddenSidebarPanels; > + WebInspector.detailsSidebar.insertSidebarPanel(sidebarPanel, index); This could just be: let index = i - hiddenSidebarPanels
Comment on attachment 306300 [details] Patch Landed: https://trac.webkit.org/changeset/215047/webkit.