[Webkit-unassigned] [Bug 107129] Web Inspector: Single column layout for the elements panel when docked right
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Jan 17 08:56:55 PST 2013
https://bugs.webkit.org/show_bug.cgi?id=107129
Pavel Feldman <pfeldman at chromium.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #183192|review? |review-
Flag| |
--- Comment #2 from Pavel Feldman <pfeldman at chromium.org> 2013-01-17 08:58:42 PST ---
(From update of attachment 183192)
View in context: https://bugs.webkit.org/attachment.cgi?id=183192&action=review
> Source/WebCore/inspector/front-end/DockController.js:187
> + __proto__: WebInspector.Object.prototype
You should also annotate constructor with @extends {WebInspector.Object}. Check out Source/WebCore/inspector/compile-front-end.py.
> Source/WebCore/inspector/front-end/ElementsPanel.js:60
> + this.splitView.setMinimumMainWidthPercent(minimumContentHeightPercent);
setMinimalMainHeightPercent
> Source/WebCore/inspector/front-end/ElementsPanel.js:62
> + // ElementsPanel never uses Top or Left sidebar position so it is safe to assume
We don't use comments unless absolutely necessary.
> Source/WebCore/inspector/front-end/ElementsPanel.js:179
> + _onDockStateChanged: function(event) {
Please place { on the next line.
> Source/WebCore/inspector/front-end/ElementsPanel.js:186
> + _sidebarPosition: function() {
ditto
> Source/WebCore/inspector/front-end/SidebarView.js:43
> + WebInspector.SplitView.call(this, vertical, sidebarWidthSettingName, defaultSidebarWidth || 200, defaultSidebarHeight);
defaultSidebarHeight || 200
> Source/WebCore/inspector/front-end/SidebarView.js:72
> + get mainElement()
We try to not use getters to improve compilability.
> Source/WebCore/inspector/front-end/SidebarView.js:90
> + this.sidebarElement.removeStyleClass("split-view-sidebar-left");
You could use classList for batch operations.
> Source/WebCore/inspector/front-end/SidebarView.js:103
> + if (this.sidebarPosition_ == sidebarPosition)
We use === when comparing primitive types.
> Source/WebCore/inspector/front-end/SidebarView.js:200
> + var minMainWidthPercent = this.isVertical() ? this._minimumMainWidthPercent : this._minimumMainHeightPercent;
var minMainSizePercent ?
> Source/WebCore/inspector/front-end/SplitView.js:37
> +WebInspector.SplitView = function(isVertical, sidebarSizeSettingName, defaultSidebarSize, defaultSidebarSize2)
You should move this change into a separate bug. In fact, introducing a single "swap" method that would convert this split into another instance and simply re-style it might remove the necessity of the changes below.
> Source/WebCore/inspector/front-end/SplitView.js:60
> + if (defaultSidebarSize2) {
Then we can drop this comment.
> Source/WebCore/inspector/front-end/SplitView.js:63
> + this._sidebarWidthSettingName = sidebarSizeSettingName + 'Width';
We only use double quotes. WebKit JS style is easiest to remember as the opposite to Google Closure's.
> Source/WebCore/inspector/front-end/SplitView.js:73
> + if (this._sidebarHeightSettingName != this._sidebarWidthSettingName) {
I would create settings for the given defaults.
> Source/WebCore/inspector/front-end/SplitView.js:147
> + _updateLayout: function() {
{ on the next line.
> Source/WebCore/inspector/front-end/SplitView.js:409
> + resizerElement.addEventListener("mousedown", this._onDragStart.bind(this), false);
Why did this change?
> Source/WebCore/inspector/front-end/SplitView.js:416
> + _onDragStart: function(event) {
ditto
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list