Bug 174741

<rdar://problem/33467954>

Created attachment 316159 [details] Patch

Created attachment 316162 [details] Patch

Comment on attachment 316162 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=316162&action=review > Source/WebInspectorUI/UserInterface/Views/CSSStyleDetailsSidebarPanel.js:36 > + if (WebInspector.settings.experimentalSpreadsheetStyleEditor.value) You need to listen for the setting change event and swap which instance is being used. Otherwise you'd need to reopen the inspector to test whether it works. The current setup within this class doesn't allow for that to happen easily. It's probably easiest to store both panels in instance variables and convert this._panels into a getter. The getter can dynamically decide which of the rule sidebars to show. You'll probably need to invalidate the view hierarchy or something to force sidebar visibility to get synced correctly. There are various other places that access the rules sidebar directly, so you could turn this._rulesStyleDetailsPanel into a getter as well so that clients are oblivious to which implementation is being used. As for the navigationInfo, if you use the same identifier for both panels then it should just work (as long as it goes through the getter).

I think it's acceptable to reload Web Inspector for experimental features to be enabled. Turning an experimental feature on and off shouldn't be a very common action. Chrome shows "WARNING: These experiments could be dangerous and may require restart.". I'm not sure about this warning, but it would be nice to show "Reload" button below the experimental checkboxes once an experimental setting has been changed.

(In reply to Nikita Vasilyev from comment #5) > I think it's acceptable to reload Web Inspector for experimental features to > be enabled. > Turning an experimental feature on and off shouldn't be a very common action. > > Chrome shows "WARNING: These experiments could be dangerous and may require > restart.". > I'm not sure about this warning, but it would be nice to show "Reload" > button below the experimental checkboxes once an experimental setting has > been changed. That approach fine, but the current UI is mysterious and not something I want to get further copied. Maybe you can add an "Apply Settings and Reload" button to the settings screen that shows up when this setting changes.

(In reply to Nikita Vasilyev from comment #5) > I think it's acceptable to reload Web Inspector for experimental features to > be enabled. > Turning an experimental feature on and off shouldn't be a very common action. Agree. > Chrome shows "WARNING: These experiments could be dangerous and may require > restart.". > I'm not sure about this warning, but it would be nice to show "Reload" > button below the experimental checkboxes once an experimental setting has > been changed. A message explaining that a restart may be required after enabling features seems like a good idea. I don't think it needs to be as dire sounding as Chrome's warning.

Created attachment 316390 [details] Patch

Created attachment 316391 [details] [Animated GIF] Reopen Web Inspector button in Settings

Comment on attachment 316390 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=316390&action=review r=me please adjust the label text > Source/WebInspectorUI/UserInterface/Views/SettingsTabContentView.js:275 > + reopenInspectorButton.textContent = WebInspector.unlocalizedString("Reopen Web Inspector"); Should be "Reload Web Inspector". It never closes.

Created attachment 316394 [details] Patch

Comment on attachment 316394 [details] Patch Clearing flags on attachment: 316394 Committed r219888: <http://trac.webkit.org/changeset/219888>

All reviewed patches have been landed. Closing bug.