[webkit-reviews] review granted: [Bug 201539] Web Inspector: Better position for Sources tab when enabling the experimental setting : [Attachment 378171] [PATCH] Proposed Fix

[bugzilla-daemon at webkit.org]

Devin Rousso <drousso at apple.com> has granted Joseph Pecoraro
<joepeck at webkit.org>'s request for review:
Bug 201539: Web Inspector: Better position for Sources tab when enabling the
experimental setting
https://bugs.webkit.org/show_bug.cgi?id=201539

Attachment 378171: [PATCH] Proposed Fix

https://bugs.webkit.org/attachment.cgi?id=378171&action=review




--- Comment #2 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 378171
  --> https://bugs.webkit.org/attachment.cgi?id=378171
[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`.