[Webkit-unassigned] [Bug 107552] Web Inspector: Show element style panes in a tabbed pane when they are below the DOM tree pane.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=107552


Pavel Feldman <pfeldman at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #184450|review?                     |review-
               Flag|                            |




--- Comment #9 from Pavel Feldman <pfeldman at chromium.org>  2013-01-24 04:43:53 PST ---
(From update of attachment 184450)
View in context: https://bugs.webkit.org/attachment.cgi?id=184450&action=review

> Source/WebCore/inspector/front-end/ElementsPanel.js:99
> +    this.sidebarPaneGroup = new WebInspector.SidebarPaneGroup(this.sidebarPanes, !this.splitView.isVertical());

I'd pass array into it (I.e. Object.values(this.sidebarPanes))

> Source/WebCore/inspector/front-end/ElementsPanel.js:168
> +        setTimeout(this._updateSidebar.bind(this), 0);

A comment here would not hurt.

> Source/WebCore/inspector/front-end/SidebarPane.js:170
> + * @extends {WebInspector.Object}

extends WebInspector.view

> Source/WebCore/inspector/front-end/SidebarPane.js:206
> +    _innerSetTabbed: function(tabbed)

Please annotate.

> Source/WebCore/inspector/front-end/SidebarPane.js:218
> +            this._tabbedPane.detach();

no {} around single line block

> Source/WebCore/inspector/front-end/SidebarPane.js:221
> +        for (var id in this._panes) {

So we are going to do it every time say Elements panel is selected - not good.

> Source/WebCore/inspector/front-end/SidebarPane.js:227
> +                var view = new WebInspector.View();

You already have a view created in _createTabbedPane.

> Source/WebCore/inspector/front-end/SidebarPane.js:229
> +                this._tabbedPane.changeTabView(id, view);

Why changing the tab view? Just populate the existing one...

> Source/WebCore/inspector/front-end/SidebarPane.js:249
> +        for (var id in this._panes) {

No need for {} for single line blocks.

> Source/WebCore/inspector/front-end/SidebarPane.js:250
> +            this._panes[id].lockExpanded(false);

Why is this necessary? For the jumping one?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.