WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
101612
Web Inspector: split SplitView into SplitView and SidebarView
https://bugs.webkit.org/show_bug.cgi?id=101612
Summary
Web Inspector: split SplitView into SplitView and SidebarView
Pavel Feldman
Reported
2012-11-08 08:49:13 PST
Currently, SplitView is rather a sidebar view. I'm extracting the reusable split component from it.
Attachments
Patch
(54.63 KB, patch)
2012-11-08 08:58 PST
,
Pavel Feldman
no flags
Details
Formatted Diff
Diff
[Patch] with canvas profiler migrated to the split
(57.82 KB, patch)
2012-11-08 11:01 PST
,
Pavel Feldman
no flags
Details
Formatted Diff
Diff
Patch
(58.02 KB, patch)
2012-11-09 01:08 PST
,
Pavel Feldman
vsevik
: review+
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Pavel Feldman
Comment 1
2012-11-08 08:58:32 PST
Created
attachment 173055
[details]
Patch
Pavel Feldman
Comment 2
2012-11-08 11:01:19 PST
Created
attachment 173074
[details]
[Patch] with canvas profiler migrated to the split
kov's GTK+ EWS bot
Comment 3
2012-11-08 13:24:32 PST
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 4
2012-11-09 00:07:47 PST
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
Comment 5
2012-11-09 01:08:16 PST
Created
attachment 173232
[details]
Patch
Pavel Feldman
Comment 6
2012-11-09 01:23:06 PST
Committed
r134031
: <
http://trac.webkit.org/changeset/134031
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug