Bug 201539

Summary: Web Inspector: Better position for Sources tab when enabling the experimental setting
Product: WebKit Reporter: Joseph Pecoraro <joepeck>
Component: Web InspectorAssignee: Joseph Pecoraro <joepeck>
Status: RESOLVED FIXED    
Severity: Normal CC: hi, inspector-bugzilla-changes, joepeck, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
[PATCH] Proposed Fix hi: review+

Joseph Pecoraro
Reported 2019-09-06 01:50:17 PDT
Better position for Sources tab when enabling the experimental setting It is horrible that the new Sources tab ends up all the way at the end. Let's try to place it where the Debugger tab was.
Attachments
[PATCH] Proposed Fix (2.37 KB, patch)
2019-09-06 01:51 PDT, Joseph Pecoraro
hi: review+
Joseph Pecoraro
Comment 1 2019-09-06 01:51:11 PDT
Created attachment 378171 [details] [PATCH] Proposed Fix
Devin Rousso
Comment 2 2019-09-06 11:10:09 PDT
Comment on attachment 378171 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=378171&action=review r=me > Source/WebInspectorUI/UserInterface/Views/SettingsTabContentView.js:348 > let newTabs = WI._openTabsSetting.value.slice(); NIT: now that <https://webkit.org/b/17240> added `WI.Setting.prototype.save`, we can avoid this `slice` and instead just forcibly call `WI._openTabsSetting.save()` at the end of this function, right before `InspectorFrontendHost.reopen()`. > Source/WebInspectorUI/UserInterface/Views/SettingsTabContentView.js:352 > + let index = newTabs.indexOf(WI.DebuggerTabContentView.Type); Rather than look for `WI.DebuggerTabContentVIew` specifically, perhaps we could expose `productionTabClasses` (in `WI.contentLoaded` in Main.js) and use that for ordering (which would also benefit the Layers Tab), since that already has a defined order of where the default tabs should be. This way, if the user doesn't have the Debugger Tab open, but one of the other "nearby" tabs is open, we'd still add the Sources Tab in the "generally" right place :) > Source/WebInspectorUI/UserInterface/Views/SettingsTabContentView.js:354 > + newTabs.splice(index, 0, WI.SourcesTabContentView.Type); NIT: `Array.prototype.insertAtIndex`.
Joseph Pecoraro
Comment 3 2019-09-06 14:09:18 PDT
Comment on attachment 378171 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=378171&action=review >> Source/WebInspectorUI/UserInterface/Views/SettingsTabContentView.js:352 >> + let index = newTabs.indexOf(WI.DebuggerTabContentView.Type); > > Rather than look for `WI.DebuggerTabContentVIew` specifically, perhaps we could expose `productionTabClasses` (in `WI.contentLoaded` in Main.js) and use that for ordering (which would also benefit the Layers Tab), since that already has a defined order of where the default tabs should be. > > This way, if the user doesn't have the Debugger Tab open, but one of the other "nearby" tabs is open, we'd still add the Sources Tab in the "generally" right place :) I'm not sure how we would decide to reconcile all cases. So I've simplified to this: • If the Debugger tab exists, no matter where the user ordered it, use that spot • If the Debugger tab does not exist, then the user has already shows ability to customized tabs. In any case, this is a short term solution until we enable it by default! >> Source/WebInspectorUI/UserInterface/Views/SettingsTabContentView.js:354 >> + newTabs.splice(index, 0, WI.SourcesTabContentView.Type); > > NIT: `Array.prototype.insertAtIndex`. Nice!
Joseph Pecoraro
Comment 4 2019-09-06 14:20:13 PDT
Radar WebKit Bug Importer
Comment 5 2019-09-06 14:21:17 PDT
Note You need to log in before you can comment on or make changes to this bug.