DevTools: Small refactoring of InspectorView

Source/devtools/front_end/InspectorView.js

Index: Source/devtools/front_end/InspectorView.js
diff --git a/Source/devtools/front_end/InspectorView.js b/Source/devtools/front_end/InspectorView.js
index 0e93ceaae061626e891f335dde8961bd72de6438..18648677620b8d443664a9acee959e66c161f7cd 100644
--- a/Source/devtools/front_end/InspectorView.js
+++ b/Source/devtools/front_end/InspectorView.js
@@ -327,67 +327,54 @@ WebInspector.InspectorView.prototype = {
 
     _keyDownInternal: function(event)
     {
-        if (this._openBracketIdentifiers[event.keyIdentifier]) {
-            var isRotateLeft = !event.shiftKey && !event.altKey;
-            if (isRotateLeft) {
-                var panelOrder = this._tabbedPane.allTabs();
-                var index = panelOrder.indexOf(this.currentPanel().name);
-                index = (index === 0) ? panelOrder.length - 1 : index - 1;
-                this.showPanel(panelOrder[index]);
-                event.consume(true);
-                return;
-            }
+        var direction = 0;
 
-            var isGoBack = event.altKey;
-            if (isGoBack && this._canGoBackInHistory()) {
-                this._goBackInHistory();
-                event.consume(true);
-            }
-            return;
-        }
+        if (this._openBracketIdentifiers[event.keyIdentifier])
+            direction = -1;
 
-        if (this._closeBracketIdentifiers[event.keyIdentifier]) {
-            var isRotateRight = !event.shiftKey && !event.altKey;
-            if (isRotateRight) {
-                var panelOrder = this._tabbedPane.allTabs();
-                var index = panelOrder.indexOf(this.currentPanel().name);
-                index = (index + 1) % panelOrder.length;
-                this.showPanel(panelOrder[index]);
-                event.consume(true);
-                return;
-            }
+        if (this._closeBracketIdentifiers[event.keyIdentifier])
+            direction = 1;
 
-            var isGoForward = event.altKey;
-            if (isGoForward && this._canGoForwardInHistory()) {
-                this._goForwardInHistory();
-                event.consume(true);
-            }
-            return;
+        if (direction) {
+            this._handlePanelChangeShortcut(event, direction);
+            this._handleHistoryMoveShortcut(event, direction);
         }
     },
 
-    _canGoBackInHistory: function()
+    _handleHistoryMoveShortcut: function(event, move)

[vsevik, 2014/03/04 17:29:56]

I would inline event handling and extract history/panel specific logic only.

[sergeyv, 2014/03/04 17:55:42]

Done.

     {
-        return this._historyIterator > 0;
+        if (event.handled || !event.altKey || !this._isValidMoveInHistory(move))
+            return;
+
+        this._moveInHistory(move);
+        event.consume(true);
     },
 
-    _goBackInHistory: function()
+    _handlePanelChangeShortcut: function(event, direction)

[vsevik, 2014/03/04 17:29:56]

Ditto.

[sergeyv, 2014/03/04 17:55:42]

Done.

     {
-        this._inHistory = true;
-        this.setCurrentPanel(WebInspector.panels[this._history[--this._historyIterator]]);
-        delete this._inHistory;
+        if (event.handled || event.shiftKey || event.altKey)
+            return;
+
+        var panelOrder = this._tabbedPane.allTabs();
+        var index = panelOrder.indexOf(this.currentPanel().name);
+        index = (index + panelOrder.length + direction) % panelOrder.length;
+        this.showPanel(panelOrder[index]);
+        event.consume(true);
     },
 
-    _canGoForwardInHistory: function()
+    _isValidMoveInHistory: function(move)

[vsevik, 2014/03/04 17:29:56]

This could be inlined into moveInHistory

[sergeyv, 2014/03/04 17:55:42]

Done.

     {
-        return this._historyIterator < this._history.length - 1;
+        var newIndex = this._historyIterator + move;
+        return newIndex < this._history.length && newIndex >= 0;
     },
 
-    _goForwardInHistory: function()
+    _moveInHistory: function(move)
     {
         this._inHistory = true;
-        this.setCurrentPanel(WebInspector.panels[this._history[++this._historyIterator]]);
+        this._historyIterator = this._historyIterator + move;
+        this.setCurrentPanel(WebInspector.panels[this._history[this._historyIterator]]);
         delete this._inHistory;
+
     },
 
     _pushToHistory: function(panelName)
@@ -414,57 +401,32 @@ WebInspector.InspectorView.prototype = {
         return this._tabbedPane.headerElement();
     },
 
+    _createImagedCounterElementIfNeeded: function(count, id, styleName)
+    {
+        if (count) {

[vsevik, 2014/03/04 17:29:56]

if (!count)
    return

[sergeyv, 2014/03/04 17:55:42]

Done.

+            var imageElement = this._errorWarningCountElement.createChild("div", styleName);
+            var counterElement = this._errorWarningCountElement.createChild("span");
+            counterElement.id = id;
+            counterElement.textContent = count;
+        }
+    },
+
     /**
      * @param {number} errors
      * @param {number} warnings
      */
     setErrorAndWarningCounts: function(errors, warnings)
     {
-        if (!errors && !warnings) {
-            this._errorWarningCountElement.classList.add("hidden");
-            this._tabbedPane.headerResized();
-            return;
-        }
-
-        this._errorWarningCountElement.classList.remove("hidden");
+        this._errorWarningCountElement.enableStyleClass("hidden", !errors && !warnings);

[vsevik, 2014/03/04 17:29:56]

classList.toggle

[sergeyv, 2014/03/04 17:55:42]

Done.

         this._errorWarningCountElement.removeChildren();
 
-        if (errors) {
-            var errorImageElement = this._errorWarningCountElement.createChild("div", "error-icon-small");
-            var errorElement = this._errorWarningCountElement.createChild("span");
-            errorElement.id = "error-count";
-            errorElement.textContent = errors;
-        }
-
-        if (warnings) {
-            var warningsImageElement = this._errorWarningCountElement.createChild("div", "warning-icon-small");
-            var warningsElement = this._errorWarningCountElement.createChild("span");
-            warningsElement.id = "warning-count";
-            warningsElement.textContent = warnings;
-        }
-
-        if (errors) {
-            if (warnings) {
-                if (errors == 1) {
-                    if (warnings == 1)
-                        this._errorWarningCountElement.title = WebInspector.UIString("%d error, %d warning", errors, warnings);
-                    else
-                        this._errorWarningCountElement.title = WebInspector.UIString("%d error, %d warnings", errors, warnings);
-                } else if (warnings == 1)
-                    this._errorWarningCountElement.title = WebInspector.UIString("%d errors, %d warning", errors, warnings);
-                else
-                    this._errorWarningCountElement.title = WebInspector.UIString("%d errors, %d warnings", errors, warnings);
-            } else if (errors == 1)
-                this._errorWarningCountElement.title = WebInspector.UIString("%d error", errors);
-            else
-                this._errorWarningCountElement.title = WebInspector.UIString("%d errors", errors);
-        } else if (warnings == 1)
-            this._errorWarningCountElement.title = WebInspector.UIString("%d warning", warnings);
-        else if (warnings)
-            this._errorWarningCountElement.title = WebInspector.UIString("%d warnings", warnings);
-        else
-            this._errorWarningCountElement.title = null;
+        this._createImagedCounterElementIfNeeded(errors, "error-count", "error-icon-small");
+        this._createImagedCounterElementIfNeeded(warnings, "warning-count", "warning-icon-small");
 
+        var errorString = errors ?  WebInspector.UIString("%d error%s", errors, errors > 1 ? "s" : "") : "";
+        var warningString = warnings ?  WebInspector.UIString("%d warning%s", warnings, warnings > 1 ? "s" : "") : "";
+        var commaString = errors & warnings ? ", " : "";

[vsevik, 2014/03/04 17:29:56]

did you mean &&?

[sergeyv, 2014/03/04 17:55:42]

Done.

+        this._errorWarningCountElement.title = errorString + commaString + warningString;
         this._tabbedPane.headerResized();
     },