[webkit-reviews] review denied: [Bug 109298] Web Inspector: Show Elements sidebar panes in two tabbed panes side by side : [Attachment 188505] Patch

[bugzilla-daemon at webkit.org]

Vsevolod Vlasov <vsevik at chromium.org> has denied Vladislav Kaznacheev
<kaznacheev at chromium.org>'s request for review:
Bug 109298: Web Inspector: Show Elements sidebar panes in two tabbed panes side
by side
https://bugs.webkit.org/show_bug.cgi?id=109298

Attachment 188505: Patch
https://bugs.webkit.org/attachment.cgi?id=188505&action=review

------- Additional Comments from Vsevolod Vlasov <vsevik at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=188505&action=review


> Source/WebCore/inspector/front-end/ScriptsPanel.js:1189
> +	   if (!this._splitDirectionSetting) {

This could be moved to constructor simply as
this._splitDirectionSetting = WebInspector.settings.createSetting(this.name +
"PanelSplitHorizontally, false);
this._splitDirectionSetting.addChangeListener(this._arrangeSidebarPanes.bind(th
is));

> Source/WebCore/inspector/front-end/ScriptsPanel.js:1197
> +	   if (!this._splitSidebarSetting) {

Ditto.

> Source/WebCore/inspector/front-end/SidebarPane.js:80
>      _setExpandCallback: function(callback)

I don't like that this setter returns a boolean value. Why not call
expandCallback explicitly here?