Bug 181448 – Web Inspector: TabBar redesign: add context menu to TabBar for toggling available tabs

• Component: Web Inspector
• Status: RESOLVED FIXED

[Matt Baker]

Created attachment 330844 [details]
[Image] TabBar context menu

Summary:
TabBar redesign: add context menu to TabBar for toggling available tabs.

This patch replaces context menus for GeneralTabItem (Close Tab, Close Other Tabs) and the New Tab button (Recently Closed Tabs) with a single menu for the entire bar.

Note:
The context menu's list of available tabs comes from WI.knownTabClasses(). Previously, production tab classes are added to this list alphabetically, but this patch establishes the following order:

WI.ElementsTabContentView
WI.ConsoleTabContentView
WI.DebuggerTabContentView
WI.NetworkTabContentView
WI.ResourcesTabContentView
WI.TimelineTabContentView
WI.CanvasTabContentView
WI.LayersTabContentView
WI.StorageTabContentView

// Ephemeral/specialized tabs. Skipped when building context menu so just put at the end.
WI.SearchTabContentView
WI.NewTabContentView
WI.SettingsTabContentView

[Radar WebKit Bug Importer]

<rdar://problem/36383298>

[Matt Baker]

Created attachment 330845 [details]
Patch

[Nikita Vasilyev]

I've noticed enabling a tab always show it last. I suppose this is fine.
Alternatively, it could try to remember its previous position.

This patch keeps:
1. the "+" button and New Tab tab.
2. the "x" buttons to close tabs.

Do you plan to address these in follow up patches?

[Nikita Vasilyev]

(In reply to Matt Baker from comment #0)
> Note:
> The context menu's list of available tabs comes from WI.knownTabClasses().
> Previously, production tab classes are added to this list alphabetically,
> but this patch establishes the following order:
> 
> WI.ElementsTabContentView
> WI.ConsoleTabContentView
> WI.DebuggerTabContentView
> WI.NetworkTabContentView
> WI.ResourcesTabContentView
> WI.TimelineTabContentView
> WI.CanvasTabContentView
> WI.LayersTabContentView
> WI.StorageTabContentView

I think the order in the context menu should match the default tab order
(when Web Inspector opened for the first time).

[Matt Baker]

(In reply to Nikita Vasilyev from comment #3)
> I've noticed enabling a tab always show it last. I suppose this is fine.
> Alternatively, it could try to remember its previous position.

I think it would be worth exploring. The ability to reorder tabs might complicate things, but we can probably do better than the "append at the end" method.

> This patch keeps:
> 1. the "+" button and New Tab tab.
> 2. the "x" buttons to close tabs.
> 
> Do you plan to address these in follow up patches?

Yep, I just filed <https://webkit.org/b/181468> Web Inspector: TabBar redesign: improvements to tab layout and resize behavior.

[Matt Baker]

(In reply to Nikita Vasilyev from comment #4)
> (In reply to Matt Baker from comment #0)
> > Note:
> > The context menu's list of available tabs comes from WI.knownTabClasses().
> > Previously, production tab classes are added to this list alphabetically,
> > but this patch establishes the following order:
> > 
> > WI.ElementsTabContentView
> > WI.ConsoleTabContentView
> > WI.DebuggerTabContentView
> > WI.NetworkTabContentView
> > WI.ResourcesTabContentView
> > WI.TimelineTabContentView
> > WI.CanvasTabContentView
> > WI.LayersTabContentView
> > WI.StorageTabContentView
> 
> I think the order in the context menu should match the default tab order
> (when Web Inspector opened for the first time).

I updated the default value for the open tabs setting to match.

Was:
["elements", "network", "resources", "timeline", "debugger", "storage", "canvas", "console"]

Now:
["elements", "console", "debugger", "network", "resources", "timeline", "canvas", "storage"]

I'm happy to revisit the order.

[Joseph Pecoraro]

> Was:
> ["elements", "network", "resources", "timeline", "debugger", "storage",
> "canvas", "console"]

This is the order I expected. Its actually the order my tabs are in right now!

> Now:
> ["elements", "console", "debugger", "network", "resources", "timeline",
> "canvas", "storage"]

Personally I'd never have the Console tab so far to the left, or have Resources so far to the right.

Of course this is just personal taste.

[Matt Baker]

(In reply to Joseph Pecoraro from comment #7)
> > Was:
> > ["elements", "network", "resources", "timeline", "debugger", "storage",
> > "canvas", "console"]
> 
> This is the order I expected. Its actually the order my tabs are in right
> now!
> 
> > Now:
> > ["elements", "console", "debugger", "network", "resources", "timeline",
> > "canvas", "storage"]
> 
> Personally I'd never have the Console tab so far to the left, or have
> Resources so far to the right.

The other browser dev tools all place it after Elements, but this always seemed a little odd to me.
> 
> Of course this is just personal taste.

I like our original order, except for Debugger being so far to the right. In the feedback we've received it is definitely more heavily used than Timelines. How about:

["elements", "network", "debugger", "resources", "timeline", "storage", "canvas", "console"]

[Matt Baker]

Created attachment 330951 [details]
Patch

[EWS Watchlist]

Comment on attachment 330951 [details]
Patch

Attachment 330951 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/6029108

New failing tests:
webgl/1.0.2/conformance/uniforms/uniform-default-values.html

[EWS Watchlist]

Created attachment 331018 [details]
Archive of layout-test-results from ews106 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[Matt Baker]

Created attachment 331227 [details]
Patch

[Devin Rousso]

Comment on attachment 331227 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=331227&action=review

r=me

> Source/WebInspectorUI/UserInterface/Views/NewTabContentView.js:37
>          this._updateShownTabs();

Since this was the only place that this is used, I think you can just move the logic in `_updateShownTabs` here and clean it up a bit.

> Source/WebInspectorUI/UserInterface/Views/TabBar.js:763
> +            for (let tabBarItem of this._tabBarItems) {

It would be awesome if there was another way of determining whether there is an instance of a TabClass, but I suppose that since this is run on a contextmenu event, it's not the end of the world.

[Matt Baker]

Created attachment 331367 [details]
Patch for landing

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 331367 [details]:

media/video-seek-past-end-playing.html bug 181668 (authors: [e-mail], [e-mail], [e-mail], and [e-mail])
imported/w3c/web-platform-tests/media-source/mediasource-config-change-mp4-a-bitrate.html bug 181669 (authors: [e-mail] and [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 331367 [details]
Patch for landing

Clearing flags on attachment: 331367

Committed r226963: <https://trac.webkit.org/changeset/226963>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.