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
<rdar://problem/36383298>
Created attachment 330845 [details] Patch
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?
(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).
(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.
(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.
> 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.
(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"]
Created attachment 330951 [details] Patch
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
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
Created attachment 331227 [details] Patch
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.
Created attachment 331367 [details] Patch for landing
The commit-queue encountered the following flaky tests while processing attachment 331367 [details]: media/video-seek-past-end-playing.html bug 181668 (authors: annacc@chromium.org, fischman@chromium.org, jamesr@chromium.org, and vrk@chromium.org) imported/w3c/web-platform-tests/media-source/mediasource-config-change-mp4-a-bitrate.html bug 181669 (authors: cdumez@apple.com and jer.noble@apple.com) The commit-queue is continuing to process your patch.
Comment on attachment 331367 [details] Patch for landing Clearing flags on attachment: 331367 Committed r226963: <https://trac.webkit.org/changeset/226963>
All reviewed patches have been landed. Closing bug.