Web Inspector: ButtonNavigationItem should support image and text button style
authormattbaker@apple.com <mattbaker@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 17 Nov 2017 20:38:26 +0000 (20:38 +0000)
committermattbaker@apple.com <mattbaker@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 17 Nov 2017 20:38:26 +0000 (20:38 +0000)
https://bugs.webkit.org/show_bug.cgi?id=179625
<rdar://problem/35512238>

Reviewed by Joseph Pecoraro.

* UserInterface/Main.html:

* UserInterface/Views/ButtonNavigationItem.css:
(.navigation-bar .item.button):
(.navigation-bar .item.button.image-only):
(.navigation-bar .item.button.image-and-text):
(.navigation-bar .item.button.image-and-text > span):
(.navigation-bar .item.button.text-only):
(.navigation-bar .item.button.text-only.checkbox): Deleted.
(.navigation-bar .item.button.text-only.checkbox label): Deleted.
Checkbox styles have been moved to a separate CSS file.

* UserInterface/Views/ButtonNavigationItem.js:
Add backing data members for the image and label, allowing the DOM
representation of the button to change as needed.

(WI.ButtonNavigationItem):
Remove duplicate role logic. The base class sets the role.

(WI.ButtonNavigationItem.prototype.set label):
(WI.ButtonNavigationItem.prototype.set image):
(WI.ButtonNavigationItem.prototype.get buttonStyle):
(WI.ButtonNavigationItem.prototype.set buttonStyle):
(WI.ButtonNavigationItem.prototype.get additionalClassNames):
(WI.ButtonNavigationItem.prototype._mouseClicked):
(WI.ButtonNavigationItem.prototype._update):
(WI.ButtonNavigationItem.prototype.updateButtonText): Deleted.

* UserInterface/Views/CheckboxNavigationItem.css: Added.
(.navigation-bar .item.checkbox):
(.navigation-bar .item.checkbox label):

* UserInterface/Views/CheckboxNavigationItem.js:
Change base class to NavigationItem, since this item lacks an image
and makes use of an actual label element instead of a span for its text.

(WI.CheckboxNavigationItem):
(WI.CheckboxNavigationItem.prototype._checkboxChanged):
(WI.CheckboxNavigationItem.prototype.updateButtonText): Deleted.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@224978 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/UserInterface/Main.html
Source/WebInspectorUI/UserInterface/Views/ButtonNavigationItem.css
Source/WebInspectorUI/UserInterface/Views/ButtonNavigationItem.js
Source/WebInspectorUI/UserInterface/Views/CheckboxNavigationItem.css [new file with mode: 0644]
Source/WebInspectorUI/UserInterface/Views/CheckboxNavigationItem.js

index a42bd0f..accfc54 100644 (file)
@@ -1,3 +1,51 @@
+2017-11-17  Matt Baker  <mattbaker@apple.com>
+
+        Web Inspector: ButtonNavigationItem should support image and text button style
+        https://bugs.webkit.org/show_bug.cgi?id=179625
+        <rdar://problem/35512238>
+
+        Reviewed by Joseph Pecoraro.
+
+        * UserInterface/Main.html:
+
+        * UserInterface/Views/ButtonNavigationItem.css:
+        (.navigation-bar .item.button):
+        (.navigation-bar .item.button.image-only):
+        (.navigation-bar .item.button.image-and-text):
+        (.navigation-bar .item.button.image-and-text > span):
+        (.navigation-bar .item.button.text-only):
+        (.navigation-bar .item.button.text-only.checkbox): Deleted.
+        (.navigation-bar .item.button.text-only.checkbox label): Deleted.
+        Checkbox styles have been moved to a separate CSS file.
+
+        * UserInterface/Views/ButtonNavigationItem.js:
+        Add backing data members for the image and label, allowing the DOM
+        representation of the button to change as needed.
+
+        (WI.ButtonNavigationItem):
+        Remove duplicate role logic. The base class sets the role.
+
+        (WI.ButtonNavigationItem.prototype.set label):
+        (WI.ButtonNavigationItem.prototype.set image):
+        (WI.ButtonNavigationItem.prototype.get buttonStyle):
+        (WI.ButtonNavigationItem.prototype.set buttonStyle):
+        (WI.ButtonNavigationItem.prototype.get additionalClassNames):
+        (WI.ButtonNavigationItem.prototype._mouseClicked):
+        (WI.ButtonNavigationItem.prototype._update):
+        (WI.ButtonNavigationItem.prototype.updateButtonText): Deleted.
+
+        * UserInterface/Views/CheckboxNavigationItem.css: Added.
+        (.navigation-bar .item.checkbox):
+        (.navigation-bar .item.checkbox label):
+
+        * UserInterface/Views/CheckboxNavigationItem.js:
+        Change base class to NavigationItem, since this item lacks an image
+        and makes use of an actual label element instead of a span for its text.
+
+        (WI.CheckboxNavigationItem):
+        (WI.CheckboxNavigationItem.prototype._checkboxChanged):
+        (WI.CheckboxNavigationItem.prototype.updateButtonText): Deleted.
+
 2017-11-17  Joseph Pecoraro  <pecoraro@apple.com>
 
         Web Inspector: Remove FIXMEs for GTK+ missing icons
index 5d00e50..aa4af6e 100644 (file)
@@ -49,6 +49,7 @@
     <link rel="stylesheet" href="Views/CanvasOverviewContentView.css">
     <link rel="stylesheet" href="Views/CanvasTabContentView.css">
     <link rel="stylesheet" href="Views/ChartDetailsSectionRow.css">
+    <link rel="stylesheet" href="Views/CheckboxNavigationItem.css">
     <link rel="stylesheet" href="Views/CircleChart.css">
     <link rel="stylesheet" href="Views/ClusterContentView.css">
     <link rel="stylesheet" href="Views/CodeMirrorOverrides.css">
index cabb42c..801ebdd 100644 (file)
  */
 
 .navigation-bar .item.button {
-    width: 26px;
     justify-content: center;
+    color: hsl(0, 0%, 18%);
+}
+
+.navigation-bar .item.button.image-only {
+    width: 26px;
+}
+
+.navigation-bar .item.button.image-and-text {
+    padding: 0 8px;
+}
+
+.navigation-bar .item.button.image-and-text > span {
+    padding-bottom: 1px;
+    -webkit-padding-start: 4px;
 }
 
 .navigation-bar .item.button.text-only {
-    width: auto;
     padding: 1px 8px 3px;
     margin: 0 2px;
 
     height: initial;
     line-height: 11px;
-    color: hsl(0, 0%, 18%);
     border: 1px solid transparent;
     border-radius: 3px;
     text-align: center;
 }
 
-.navigation-bar .item.button.text-only.checkbox {
-    padding: 1px 4px 3px;
-}
-
-.navigation-bar .item.button.text-only.checkbox label {
-    -webkit-padding-start: 2px;
-}
-
 .navigation-bar .item.button > .glyph {
     color: var(--glyph-color);
 }
index e734550..0ceabfd 100644 (file)
@@ -27,7 +27,7 @@ WI.ButtonNavigationItem = class ButtonNavigationItem extends WI.NavigationItem
 {
     constructor(identifier, toolTipOrLabel, image, imageWidth, imageHeight, role, label)
     {
-        super(identifier);
+        super(identifier, role || "button");
 
         console.assert(identifier);
         console.assert(toolTipOrLabel);
@@ -38,19 +38,17 @@ WI.ButtonNavigationItem = class ButtonNavigationItem extends WI.NavigationItem
 
         this.element.addEventListener("click", this._mouseClicked.bind(this));
 
-        this.element.setAttribute("role", role || "button");
-
         if (label)
             this.element.setAttribute("aria-label", label);
 
+        this._buttonStyle = null;
+        this._disabled = false;
+        this._image = image || null;
         this._imageWidth = imageWidth || 16;
         this._imageHeight = imageHeight || 16;
-        this._label = "";
+        this._label = toolTipOrLabel;
 
-        if (image)
-            this.image = image;
-        else if (toolTipOrLabel)
-            this.label = toolTipOrLabel;
+        this.buttonStyle = this._image ? WI.ButtonNavigationItem.Style.Image : WI.ButtonNavigationItem.Style.Text;
     }
 
     // Public
@@ -80,10 +78,9 @@ WI.ButtonNavigationItem = class ButtonNavigationItem extends WI.NavigationItem
         if (this._label === newLabel)
             return;
 
-        this.element.classList.add(WI.ButtonNavigationItem.TextOnlyClassName);
         this._label = newLabel;
+        this._update();
 
-        this.updateButtonText();
         if (this.parentNavigationBar)
             this.parentNavigationBar.needsLayout();
     }
@@ -95,20 +92,12 @@ WI.ButtonNavigationItem = class ButtonNavigationItem extends WI.NavigationItem
 
     set image(newImage)
     {
-        if (!newImage) {
-            this.element.removeChildren();
+        newImage = newImage || null;
+        if (this._image === newImage)
             return;
-        }
-
-        this.element.removeChildren();
-        this.element.classList.remove(WI.ButtonNavigationItem.TextOnlyClassName);
 
         this._image = newImage;
-
-        this._glyphElement = WI.ImageUtilities.useSVGSymbol(this._image, "glyph");
-        this._glyphElement.style.width = this._imageWidth + "px";
-        this._glyphElement.style.height = this._imageHeight + "px";
-        this.element.appendChild(this._glyphElement);
+        this._update();
     }
 
     get enabled()
@@ -126,18 +115,32 @@ WI.ButtonNavigationItem = class ButtonNavigationItem extends WI.NavigationItem
         this.element.classList.toggle("disabled", !this._enabled);
     }
 
-    // Protected
-
-    get additionalClassNames()
+    get buttonStyle()
     {
-        return ["button"];
+        return this._buttonStyle;
     }
 
-    updateButtonText()
+    set buttonStyle(newButtonStyle)
     {
-        // Overridden by subclasses.
+        newButtonStyle = newButtonStyle || WI.ButtonNavigationItem.Style.Image;
+        if (this._buttonStyle === newButtonStyle)
+            return;
+
+        this.element.classList.remove(...Object.values(WI.ButtonNavigationItem.Style));
+        this.element.classList.add(newButtonStyle);
+
+        this._buttonStyle = newButtonStyle;
+        this._update();
+
+        if (this.parentNavigationBar)
+            this.parentNavigationBar.needsLayout();
+    }
+
+    // Protected
 
-        this.element.textContent = this._label;
+    get additionalClassNames()
+    {
+        return ["button"];
     }
 
     // Private
@@ -148,10 +151,33 @@ WI.ButtonNavigationItem = class ButtonNavigationItem extends WI.NavigationItem
             return;
         this.dispatchEventToListeners(WI.ButtonNavigationItem.Event.Clicked, {nativeEvent: event});
     }
-};
 
-WI.ButtonNavigationItem.TextOnlyClassName = "text-only";
+    _update()
+    {
+        this.element.removeChildren();
+
+        if (this._buttonStyle === WI.ButtonNavigationItem.Style.Text)
+            this.element.textContent = this._label;
+        else {
+            let glyphElement = WI.ImageUtilities.useSVGSymbol(this._image, "glyph");
+            glyphElement.style.width = this._imageWidth + "px";
+            glyphElement.style.height = this._imageHeight + "px";
+            this.element.appendChild(glyphElement);
+
+            if (this._buttonStyle === WI.ButtonNavigationItem.Style.ImageAndText) {
+                let labelElement = this.element.appendChild(document.createElement("span"));
+                labelElement.textContent = this._label;
+            }
+        }
+    }
+};
 
 WI.ButtonNavigationItem.Event = {
     Clicked: "button-navigation-item-clicked"
 };
+
+WI.ButtonNavigationItem.Style = {
+    Image: "image-only",
+    Text: "text-only",
+    ImageAndText: "image-and-text",
+};
diff --git a/Source/WebInspectorUI/UserInterface/Views/CheckboxNavigationItem.css b/Source/WebInspectorUI/UserInterface/Views/CheckboxNavigationItem.css
new file mode 100644 (file)
index 0000000..0cb76ce
--- /dev/null
@@ -0,0 +1,33 @@
+/*
+ * Copyright (C) 2017 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+.navigation-bar .item.checkbox {
+    margin: 0;
+    padding: 1px 8px 3px;
+}
+
+.navigation-bar .item.checkbox label {
+    -webkit-padding-start: 3px;
+}
index 34df4e9..359c600 100644 (file)
  * THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-WI.CheckboxNavigationItem = class CheckboxNavigationItem extends WI.ButtonNavigationItem
+WI.CheckboxNavigationItem = class CheckboxNavigationItem extends WI.NavigationItem
 {
     constructor(identifier, label, checked)
     {
-        super(identifier, label);
+        super(identifier, "checkbox");
 
         this._checkboxElement = this.element.appendChild(document.createElement("input"));
         this._checkboxElement.checked = !checked;
@@ -38,9 +38,8 @@ WI.CheckboxNavigationItem = class CheckboxNavigationItem extends WI.ButtonNaviga
 
         this._checkboxLabel = this.element.appendChild(document.createElement("label"));
         this._checkboxLabel.className = "toggle";
+        this._checkboxLabel.textContent = label;
         this._checkboxLabel.setAttribute("for", this._checkboxElement.id);
-
-        this.updateButtonText();
     }
 
     // Public
@@ -62,16 +61,14 @@ WI.CheckboxNavigationItem = class CheckboxNavigationItem extends WI.ButtonNaviga
         return ["checkbox", "button"];
     }
 
-    updateButtonText()
-    {
-        if (this._checkboxLabel)
-            this._checkboxLabel.textContent = this.label;
-    }
-
     // Private
 
     _checkboxChanged(event)
     {
-        this.dispatchEventToListeners(WI.ButtonNavigationItem.Event.Clicked);
+        this.dispatchEventToListeners(WI.CheckboxNavigationItem.Event.CheckedDidChange);
     }
 };
+
+WI.CheckboxNavigationItem.Event = {
+    CheckedDidChange: "checkbox-navigation-item-checked-did-change",
+};