Bug 179100 – Web Inspector: Styles Redesign: turn on CSS spreadsheet editor by default

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Nikita Vasilyev]

- Make the redesigned styles sidebar (spreadsheet editor) on by default.
- Replace Experimental Spreadsheet Style Editor checkbox with something like "Styles Editing: [ ] Use legacy styles sidebar".
- The legacy styles sidebar has setting:

Styles Editing: [x] Show inline warnings
                [x] Automatically insert newline
                [x] Select text on first click

They are not applicable to the redesigned styles sidebar. I think we should remove them entirely.
Most Web Inspector users don't even know about the settings. I expect very few people to use the legacy styles sidebar AND
have custom settings for it.

<rdar://problem/33524171>

[Radar WebKit Bug Importer]

<rdar://problem/35285990>

[Nikita Vasilyev]

Created attachment 325660 [details]
Patch

[Nikita Vasilyev]

Created attachment 325661 [details]
[Animated GIF] With patch applied

[Haroen Viaene]

Some feedback here, if I enable the spreadsheet editor now, it doesn't create a `property: value` if I type that in the property part, it requires me to tab into the value part, would be nice to automatically switch to the value input when a : is typed. 

Thanks!

[Blaze Burg]

(In reply to Haroen Viaene from comment #4)
> Some feedback here, if I enable the spreadsheet editor now, it doesn't
> create a `property: value` if I type that in the property part, it requires
> me to tab into the value part, would be nice to automatically switch to the
> value input when a : is typed. 
> 
> Thanks!

Already filed: https://bugs.webkit.org/show_bug.cgi?id=178795

In general, you should just file a new bug if your feedback are not related to the patch in a bug. Thanks!

[Blaze Burg]

Comment on attachment 325660 [details]
Patch

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

> Source/WebInspectorUI/ChangeLog:9
> +        Add a "Legacy Style Editor" checkbox and remove the experimental "Spreadsheet Style Editor" checkbox.

I don't think this should be in the General section. Let's just move it to Experimental or Debug until we completely remove it. (Is there any reason to not remove it right now?)

[Blaze Burg]

Comment on attachment 325660 [details]
Patch

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

The changes to remove old settings look fine. I want this in Experimental though, so that we get more feedback and its clear we don't plan to maintain it any more.

> Source/WebInspectorUI/UserInterface/Views/SettingsTabContentView.js:209
> +        reloadInspectorButton.addEventListener("click", () => { window.location.reload(); });

This should show up at the bottom of the settings pane. It looks weird putting it inline. I think Experimental section has a reusable implementation already, so you should be able to just remove this.

[Nikita Vasilyev]

(In reply to Brian Burg from comment #6)
> Comment on attachment 325660 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=325660&action=review
> 
> > Source/WebInspectorUI/ChangeLog:9
> > +        Add a "Legacy Style Editor" checkbox and remove the experimental "Spreadsheet Style Editor" checkbox.
> 
> I don't think this should be in the General section. Let's just move it to
> Experimental or Debug until we completely remove it. (Is there any reason to
> not remove it right now?)

I can move the setting to the Experimental section. As long as the checkbox is available for everybody, it's fine with me.

I don't want to remove it just yet. I want to remove it once we receive a good amount of positive feedback.

The new styles sidebar still misses some features and lacks polish. If somebody find the new styles sidebar unusable for their workflow I want to allow them to switch back to the old one without running a different version of WebKit.

[Nikita Vasilyev]

Created attachment 325744 [details]
Patch

[Blaze Burg]

Comment on attachment 325744 [details]
Patch

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

r=me

> Source/WebInspectorUI/UserInterface/Base/Setting.js:126
> +    legacyStyleEditor: new WI.Setting("legacy-style-editor", false),

Nit: experimental settings need experimental- prefix.

[Nikita Vasilyev]

Created attachment 325760 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 325760 [details]
Patch

Clearing flags on attachment: 325760

Committed r224354: <https://trac.webkit.org/changeset/224354>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.