Bug 101612 – Web Inspector: split SplitView into SplitView and SidebarView

• Component: Web Inspector (Deprecated)
• Status: RESOLVED FIXED

[Pavel Feldman]

Currently, SplitView is rather a sidebar view. I'm extracting the reusable split component from it.

[Pavel Feldman]

Created attachment 173055 [details]
Patch

[Pavel Feldman]

Created attachment 173074 [details]
[Patch] with canvas profiler migrated to the split

[kov's GTK+ EWS bot]

Comment on attachment 173074 [details]
[Patch] with canvas profiler migrated to the split

Attachment 173074 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/14777132

[Vsevolod Vlasov]

Comment on attachment 173074 [details]
[Patch] with canvas profiler migrated to the split

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

> Source/WebCore/inspector/front-end/ScriptsPanel.js:75
> +    this.splitView.setMinimalMainWidthPercent(maximalDebugSidebarWidthPercent);

"100 - " is missing

> Source/WebCore/inspector/front-end/SidebarView.js:38
> +    WebInspector.SplitView.call(this, true, sidebarWidthSettingName, defaultSidebarWidth || 200);

I don't think this is correct way to set defaultSidebarWidth in case of right sidebar.
Also your code will save splitOffset in setting which is not correct, we should save "constant width" element width.

> Source/WebCore/inspector/front-end/SidebarView.js:50
> +    this.setChangeFirstOnResize(sidebarPosition !== WebInspector.SidebarView.SidebarPosition.Left);

I would move setChangeFirstOnResize call to _innerSetSidebarPosition

> Source/WebCore/inspector/front-end/SidebarView.js:127
> +        this.setSplitOffset(width);

This is not correct for right sidebar as far as I can see.

> Source/WebCore/inspector/front-end/SidebarView.js:135
> +        return this.splitOffset();

Ditto

> Source/WebCore/inspector/front-end/SplitView.js:46
> +    this._firstElement.className = "split-view-contents split-view-contents-" + (isVertical ? "vertical" : "horizontal");

var orientationString = isVertical ? "vertical" : "horizontal"?

[Pavel Feldman]

Created attachment 173232 [details]
Patch

[Pavel Feldman]

Committed r134031: <http://trac.webkit.org/changeset/134031>