[webkit-reviews] review granted: [Bug 172304] Web Inspector: Use initialLayout for Settings tab : [Attachment 310537] Patch

[bugzilla-daemon at webkit.org]

Matt Baker <mattbaker at apple.com> has granted Devin Rousso <drousso at apple.com>'s
request for review:
Bug 172304: Web Inspector: Use initialLayout for Settings tab
https://bugs.webkit.org/show_bug.cgi?id=172304

Attachment 310537: Patch

https://bugs.webkit.org/attachment.cgi?id=310537&action=review




--- Comment #7 from Matt Baker <mattbaker at apple.com> ---
Comment on attachment 310537
  --> https://bugs.webkit.org/attachment.cgi?id=310537
Patch

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

r=me. Please add at least a short summary to the change log mentioning the
motivation for the change.

> Source/WebInspectorUI/UserInterface/Views/SettingsTabContentView.js:171
> +	  
WebInspector.settings.zoomFactor.addEventListener(WebInspector.Setting.Event.Ch
anged, boundNeedsLayout);

I realize this was just moved from the constructor, but it seems like a
layering violation having the tab know about specific settings. It should be up
to GeneralSettingsView to update itself. A general solution would be even
better, where SettingEditor updates itself whenever the value of its backing
setting changes.