Summary: | Web Inspector: Styles Redesign: turn on CSS spreadsheet editor by default | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Nikita Vasilyev <nvasilyev> | ||||||||||
Component: | Web Inspector | Assignee: | Nikita Vasilyev <nvasilyev> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | bburg, commit-queue, hello, inspector-bugzilla-changes, webkit-bug-importer | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
Attachments: |
|
Description
Nikita Vasilyev
2017-10-31 19:08:38 PDT
Created attachment 325660 [details]
Patch
Created attachment 325661 [details]
[Animated GIF] With patch applied
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! (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! 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?) 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. (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. Created attachment 325744 [details]
Patch
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. Created attachment 325760 [details]
Patch
Comment on attachment 325760 [details] Patch Clearing flags on attachment: 325760 Committed r224354: <https://trac.webkit.org/changeset/224354> All reviewed patches have been landed. Closing bug. |