Web Inspector: Reorder Debugger tab sidebar panels: Scope Chain, Resource, Probes

• Component: Web Inspector
• Status: RESOLVED FIXED

[Matt Baker]

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.

[Matt Baker]

<rdar://problem/31410771>

[Matt Baker]

(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.

[Devin Rousso]

(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>

[Matt Baker]

Created attachment 306107 [details]
Patch

[Timothy Hatcher]

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.

[Matt Baker]

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.

[Matt Baker]

Created attachment 306140 [details]
Patch

[Timothy Hatcher]

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

[Timothy Hatcher]

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.

[Matt Baker]

(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.

[Matt Baker]

Created attachment 306300 [details]
Patch

[Timothy Hatcher]

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

[Matt Baker]

Comment on attachment 306300 [details]
Patch

Landed: https://trac.webkit.org/changeset/215047/webkit.