Bug 218126 - Web Inspector: Promote experimental "Show independent Styles sidebar" setting to "Elements" settings tab and enable by default
Summary: Web Inspector: Promote experimental "Show independent Styles sidebar" setting...
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: Patrick Angle
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-10-23 08:59 PDT by Patrick Angle
Modified: 2021-01-08 13:10 PST (History)
4 users (show)

See Also:


Attachments
Patch v1.0 (7.60 KB, patch)
2021-01-07 14:39 PST, Patrick Angle
no flags Details | Formatted Diff | Diff
Patch v1.1 - Review Notes (7.81 KB, patch)
2021-01-08 11:00 PST, Patrick Angle
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick Angle 2020-10-23 08:59:43 PDT
Add/modify sidebar layout control to support states beyond just Shown/Hidden to provide control over the appearance of multiple sidebars (when enabled).

A couple considerations here include:
 - What happens when multiple sidebars can't be shown, which can change as the window is resized.
 - It should be easy to just click to toggle between the two states the user wants (probably the current state and previous state).
 - There should be minimal/no difference in the presentation of the control when multiple sidebars are or aren't available.
Comment 1 Radar WebKit Bug Importer 2020-10-30 09:00:43 PDT
<rdar://problem/70879134>
Comment 2 Patrick Angle 2021-01-07 14:39:26 PST
Created attachment 417214 [details]
Patch v1.0
Comment 3 Devin Rousso 2021-01-07 15:04:47 PST
Comment on attachment 417214 [details]
Patch v1.0

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

r=me

> Source/WebInspectorUI/UserInterface/Base/Setting.js:197
> +    enableIndependentStylesPanel: new WI.Setting("independent-styles-panel", true),

FYI if you wanted to be very "accurate" then you'd probably want to replace the `true` with `WI.Setting.migrateValue("experimental-independent-styles-panel") ?? true`, as that would preserve the old value and remove it from `localStorage`.

Not necessary to do since this was experimental, but something to keep in mind as you modify any saved data :)

NIT: while you're at it, can you add "Elements" and "Details" in there somewhere so it's a bit more clear what it's referring to?  e.g. "enableElementsTabIndependentStylesDetailsSidebarPanel" :P

> Source/WebInspectorUI/UserInterface/Views/SettingsTabContentView.js:288
> +        elementsSettingsView.addSetting(WI.UIString("Styles Sidebar:", "Styles Sidebar: @ Settings Elements Tab", "Category label for Styles sidebar settings."), WI.settings.enableIndependentStylesPanel, WI.UIString("Show independent Styles sidebar", "Show independent Styles sidebar @ Settings Elements Tab", "Settings tab checkbox label for whether the independent styles sidebar should be shown"));

It seems a bit repetitive to have "Styles Sidebar" in both the label and the checkbox text.  How about having the label be something more generic like "Details Sidebars" so that we could maybe use it for other things in the future too (e.g. if we instead wanted to allow the Computed panel to be independent then we could make these into radio buttons)?

Also, we refer to these as "panes" in our documentation <https://webkit.org/web-inspector/web-inspector-settings/#elements> so please use that instead of "tab" :)
Comment 4 Patrick Angle 2021-01-08 11:00:01 PST
Created attachment 417278 [details]
Patch v1.1 - Review Notes
Comment 5 EWS 2021-01-08 13:10:50 PST
Committed r271319: <https://trac.webkit.org/changeset/271319>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 417278 [details].