Bug 201539 - Web Inspector: Better position for Sources tab when enabling the experimental setting
Summary: Web Inspector: Better position for Sources tab when enabling the experimental...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Joseph Pecoraro
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-09-06 01:50 PDT by Joseph Pecoraro
Modified: 2019-09-06 14:21 PDT (History)
4 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (2.37 KB, patch)
2019-09-06 01:51 PDT, Joseph Pecoraro
hi: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 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.
Comment 1 Joseph Pecoraro 2019-09-06 01:51:11 PDT
Created attachment 378171 [details]
[PATCH] Proposed Fix
Comment 2 Devin Rousso 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`.
Comment 3 Joseph Pecoraro 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!
Comment 4 Joseph Pecoraro 2019-09-06 14:20:13 PDT
https://trac.webkit.org/changeset/249592/webkit
Comment 5 Radar WebKit Bug Importer 2019-09-06 14:21:17 PDT
<rdar://problem/55127412>