[webkit-reviews] review denied: [Bug 113198] Web Inspector: provide minimized mode. : [Attachment 195326] Patch

[bugzilla-daemon at webkit.org]

Alexander Pavlov (apavlov) <apavlov at chromium.org> has denied Dmitry Gozman
<dgozman at chromium.org>'s request for review:
Bug 113198: Web Inspector: provide minimized mode.
https://bugs.webkit.org/show_bug.cgi?id=113198

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

------- Additional Comments from Alexander Pavlov (apavlov)
<apavlov at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=195326&action=review


> Source/WebCore/inspector/front-end/DockController.js:84
> +	   if (this._dockSide && this._dockSide !=
WebInspector.DockController.State.Minimized)

!==

> Source/WebCore/inspector/front-end/DockController.js:107
> +	   if (this.isMinimized()) {

No braces around single-line blocks

> Source/WebCore/inspector/front-end/DockController.js:118
> +    isMinimized: function() {

This is still not fixed.

> Source/WebCore/inspector/front-end/DockController.js:122
> +    restore: function() {

Ditto

> Source/WebCore/inspector/front-end/DockController.js:127
> +    minimize: function() {

Ditto

> Source/WebCore/inspector/front-end/DockController.js:163
> +	       if (this._dockSide ==
WebInspector.DockController.State.Minimized)

===

> Source/WebCore/inspector/front-end/DockController.js:187
> +	   if (sides.indexOf(lastState) == -1)

===

> Source/WebCore/inspector/front-end/StatusBarButton.js:212
> +	   this.element.addStyleClass("visible-in-minimized");

visible-when-minimized ?

> Source/WebCore/inspector/front-end/Toolbar.js:54
> +	  
document.getElementById("toolbar-minimize-button-left").style.display = "none";


.addStyleClass("hidden")

> Source/WebCore/inspector/front-end/Toolbar.js:55
> +	  
document.getElementById("toolbar-minimize-button-right").style.display =
"none";

Ditto

> Source/WebCore/inspector/front-end/Toolbar.js:228
> +	   return !!WebInspector.dockController &&
WebInspector.dockController.dockSide() ==
WebInspector.DockController.State.DockedToRight;

===

> Source/WebCore/inspector/front-end/Toolbar.js:236
> +	   return !!WebInspector.dockController &&
WebInspector.dockController.dockSide() ==
WebInspector.DockController.State.Minimized;

===

> Source/WebCore/inspector/front-end/Toolbar.js:244
>	   return !!WebInspector.dockController &&
WebInspector.dockController.dockSide() ==
WebInspector.DockController.State.Undocked;

===

> Source/WebCore/inspector/front-end/Toolbar.js:392
> +    minimize: function() {

Opening brace on a new line

> Source/WebCore/inspector/front-end/Toolbar.js:395
> +	   if (controlsRight.parentNode != this.element)

!==

> Source/WebCore/inspector/front-end/Toolbar.js:399
> +	   while (this.element.hasChildNodes()) {

No braces around single-line blocks

> Source/WebCore/inspector/front-end/Toolbar.js:408
> +    restore: function() {

Opening brace on a new line

> Source/WebCore/inspector/front-end/Toolbar.js:411
> +	   if (controlsRight.parentNode == this.element)

===

> Source/WebCore/inspector/front-end/Toolbar.js:414
> +	   while (this._minimizedToolbar.hasChildNodes()) {

No braces...

> Source/WebCore/inspector/front-end/Toolbar.js:418
> +	   var firstButton = this.element.querySelectorAll("button")[0];

var firstButton = this.element.querySelector("button");

> Source/WebCore/inspector/front-end/Toolbar.js:419
> +	   if (firstButton)

This branching is not necessary. insertBefore(element, null); does what it is
expected to.

> Source/WebCore/inspector/front-end/inspector.css:597
> +body.minimized #panel-status-bar > div > :not(.visible-in-minimized),

.visible-when-minimized ?