DevTools: Refactor memory counter display

Source/devtools/front_end/timeline/CountersGraph.js

Index: Source/devtools/front_end/timeline/CountersGraph.js
diff --git a/Source/devtools/front_end/timeline/CountersGraph.js b/Source/devtools/front_end/timeline/CountersGraph.js
index 820f5df52479304bdd79ce1e19fc2d50ea80a351..06297e3f6da581a40980b91df2ab8030ffa6da16 100644
--- a/Source/devtools/front_end/timeline/CountersGraph.js
+++ b/Source/devtools/front_end/timeline/CountersGraph.js
@@ -38,7 +38,7 @@
  */
 WebInspector.CountersGraph = function(title, delegate, model)
 {
-    WebInspector.SplitWidget.call(this, true, false);
+    WebInspector.SplitWidget.call(this, true, false, "memoryCountersSidebar");
 
     this.element.id = "memory-graphs-container";
 
@@ -402,11 +402,14 @@ WebInspector.CountersGraph.CounterUI = function(memoryCountersPane, title, curre
     this._memoryCountersPane = memoryCountersPane;
     this.counter = counter;
     var container = memoryCountersPane._infoWidget.element.createChild("div", "memory-counter-sidebar-info");
-    var swatchColor = graphColor;
-    this._swatch = new WebInspector.SwatchCheckbox(WebInspector.UIString(title), swatchColor);
-    this._swatch.addEventListener(WebInspector.SwatchCheckbox.Events.Changed, this._toggleCounterGraph.bind(this));
-    container.appendChild(this._swatch.element);
-    this._range = this._swatch.element.createChild("span");
+
+    this._filter = new WebInspector.CheckboxFilterUI(title, WebInspector.UIString(title));

[alph, 2015/08/24 17:19:50]

No need to use UIString here. It should be used right on the string literal.

[paulirish, 2015/08/25 23:18:46]

Done

+    var color = WebInspector.Color.parse(graphColor).setAlpha(0.5).asString(WebInspector.Color.Format.RGBA);
+    if (color !== null)

[alph, 2015/08/24 17:19:50]

if (color)

[paulirish, 2015/08/25 23:18:46]

Done

+        this._filter.setColor(color, "rgba(0,0,0,0.3)");
+    this._filter.addEventListener(WebInspector.FilterUI.Events.FilterChanged, this._toggleCounterGraph.bind(this));
+    container.appendChild(this._filter.element());
+    this._range = this._filter.labelElement().createChild("span", "range");

[alph, 2015/08/24 17:19:50]

I see the span text goes one pixel higher than the label.
Btw you can just add the span after the _filter element, so you don't need to
expose the label

[paulirish, 2015/08/25 23:18:46]

Tried that but that makes the label clickable and the range not. It's a little
awkward, so I prefer having the entire title be clickable for the checkbox.

 
     this._value = memoryCountersPane._currentValuesBar.createChild("span", "memory-counter-value");
     this._value.style.color = graphColor;
@@ -430,15 +433,23 @@ WebInspector.CountersGraph.CounterUI.prototype = {
     /**
      * @param {number} minValue
      * @param {number} maxValue
+     * @param {!WebInspector.CountersGraph.Counter} counter
      */
-    setRange: function(minValue, maxValue)
-    {
-        this._range.textContent = WebInspector.UIString("[%.0f:%.0f]", minValue, maxValue);
+    setRange: function(minValue, maxValue, counter)
+    {
+        if (counter == this._memoryCountersPane.counters("jsHeapSizeUsed")) {

[alph, 2015/08/24 17:19:50]

There should be no knowledge of a particular counter in the base class.
You can pass a custom formatter function to the constructor instead.

[paulirish, 2015/08/25 23:18:46]

good idea. Does the new patch handle this how you'd expect?

+            var min = Number.bytesToString(minValue);
+            var max = Number.bytesToString(maxValue);
+        } else {
+            var min = Number.withThousandsSeparator(minValue);
+            var max = Number.withThousandsSeparator(maxValue);
+        }
+        this._range.textContent = WebInspector.UIString("[%s\u2009\u2013\u2009%s]", min, max);
     },
 
     _toggleCounterGraph: function(event)

[alph, 2015/08/24 17:19:50]

nit: could you please annotate.

[paulirish, 2015/08/25 23:18:46]

Done.

     {
-        this._value.classList.toggle("hidden", !this._swatch.checked);
+        this._value.classList.toggle("hidden", !this._filter.checked());
         this._memoryCountersPane.refresh();
     },
 
@@ -459,7 +470,8 @@ WebInspector.CountersGraph.CounterUI.prototype = {
         if (!this.visible() || !this.counter.values.length || !this.counter.x)
             return;
         var index = this._recordIndexAt(x);
-        this._value.textContent = WebInspector.UIString(this._currentValueLabel, this.counter.values[index]);
+        var value = Number.withThousandsSeparator(this.counter.values[index]);
+        this._value.textContent = WebInspector.UIString(this._currentValueLabel, value);
         var y = this.graphYValues[index] / window.devicePixelRatio;
         this._marker.style.left = x + "px";
         this._marker.style.top = y + "px";
@@ -494,7 +506,7 @@ WebInspector.CountersGraph.CounterUI.prototype = {
         var bounds = counter._calculateBounds();
         var minValue = bounds.min;
         var maxValue = bounds.max;
-        this.setRange(minValue, maxValue);
+        this.setRange(minValue, maxValue, counter);
 
         if (!this.visible())
             return;
@@ -541,50 +553,6 @@ WebInspector.CountersGraph.CounterUI.prototype = {
      */
     visible: function()
     {
-        return this._swatch.checked;
+        return this._filter.checked();
     }
 }
-
-
-/**
- * @constructor
- * @extends {WebInspector.Object}
- */
-WebInspector.SwatchCheckbox = function(title, color)
-{
-    this.element = createElement("div");
-    this._swatch = this.element.createChild("div", "swatch");
-    this.element.createChild("span", "title").textContent = title;
-    this._color = color;
-    this.checked = true;
-
-    this.element.addEventListener("click", this._toggleCheckbox.bind(this), true);
-}
-
-WebInspector.SwatchCheckbox.Events = {
-    Changed: "Changed"
-}
-
-WebInspector.SwatchCheckbox.prototype = {
-    get checked()
-    {
-        return this._checked;
-    },
-
-    set checked(v)
-    {
-        this._checked = v;
-        if (this._checked)
-            this._swatch.style.backgroundColor = this._color;
-        else
-            this._swatch.style.backgroundColor = "";
-    },
-
-    _toggleCheckbox: function(event)
-    {
-        this.checked = !this.checked;
-        this.dispatchEventToListeners(WebInspector.SwatchCheckbox.Events.Changed);
-    },
-
-    __proto__: WebInspector.Object.prototype
-}