Bug 1239730 - memory table formatNumber/Percent : add space every 3 digits. r=jsantell

devtools/client/memory/components/census-tree-item.js

@@ -4,7 +4,7 @@
 
 const { isSavedFrame } = require("devtools/shared/DevToolsUtils");
 const { DOM: dom, createClass, createFactory } = require("devtools/client/shared/vendor/react");
-const { L10N } = require("../utils");
+const { L10N, formatNumber, formatPercent } = require("../utils");
 const Frame = createFactory(require("devtools/client/shared/components/frame"));
 const unknownSourceString = L10N.getStr("unknownSource");
 const { TREE_ROW_HEIGHT } = require("../constants");
@@ -12,25 +12,6 @@ const { TREE_ROW_HEIGHT } = require("../constants");
 const CensusTreeItem = module.exports = createClass({
   displayName: "CensusTreeItem",
 
-  formatPercent(showSign, percent) {
-    return L10N.getFormatStr("tree-item.percent",
-                             this.formatNumber(showSign, percent));
-  },
-
-  formatNumber(showSign, number) {
-    const rounded = Math.round(number);
-    if (rounded === 0 || rounded === -0) {
-      return "0";
-    }
-
-    let sign = "";
-    if (showSign) {
-      sign = rounded < 0 ? "-" : "+";
-    }
-
-    return sign + Math.abs(rounded);
-  },
-
   shouldComponentUpdate(nextProps, nextState) {
     return this.props.item != nextProps.item
       || this.props.depth != nextProps.depth
@@ -52,17 +33,17 @@ const CensusTreeItem = module.exports = createClass({
       onViewSourceInDebugger,
     } = this.props;
 
-    const bytes = this.formatNumber(showSign, item.bytes);
-    const percentBytes = this.formatPercent(showSign, getPercentBytes(item.bytes));
+    const bytes = formatNumber(item.bytes, showSign);
+    const percentBytes = formatPercent(getPercentBytes(item.bytes), showSign);
 
-    const count = this.formatNumber(showSign, item.count);
-    const percentCount = this.formatPercent(showSign, getPercentCount(item.count));
+    const count = formatNumber(item.count, showSign);
+    const percentCount = formatPercent(getPercentCount(item.count), showSign);
 
-    const totalBytes = this.formatNumber(showSign, item.totalBytes);
-    const percentTotalBytes = this.formatPercent(showSign, getPercentBytes(item.totalBytes));
+    const totalBytes = formatNumber(item.totalBytes, showSign);
+    const percentTotalBytes = formatPercent(getPercentBytes(item.totalBytes), showSign);
 
-    const totalCount = this.formatNumber(showSign, item.totalCount);
-    const percentTotalCount = this.formatPercent(showSign, getPercentCount(item.totalCount));
+    const totalCount = formatNumber(item.totalCount, showSign);
+    const percentTotalCount = formatPercent(getPercentCount(item.totalCount), showSign);
 
     return dom.div({ className: `heap-tree-item ${focused ? "focused" :""}` },
       dom.span({ className: "heap-tree-item-field heap-tree-item-bytes" },

devtools/client/memory/components/dominator-tree-item.js

@@ -4,7 +4,7 @@
 
 const { assert, isSavedFrame } = require("devtools/shared/DevToolsUtils");
 const { DOM: dom, createClass, createFactory, PropTypes } = require("devtools/client/shared/vendor/react");
-const { L10N } = require("../utils");
+const { L10N, formatNumber, formatPercent } = require("../utils");
 const Frame = createFactory(require("devtools/client/shared/components/frame"));
 const { TREE_ROW_HEIGHT } = require("../constants");
 
@@ -28,20 +28,6 @@ const DominatorTreeItem = module.exports = createClass({
     onViewSourceInDebugger: PropTypes.func.isRequired,
   },
 
-  formatPercent(percent) {
-    return L10N.getFormatStr("tree-item.percent",
-                             this.formatNumber(percent));
-  },
-
-  formatNumber(number) {
-    const rounded = Math.round(number);
-    if (rounded === 0 || rounded === -0) {
-      return "0";
-    }
-
-    return String(Math.abs(rounded));
-  },
-
   shouldComponentUpdate(nextProps, nextState) {
     return this.props.item != nextProps.item
       || this.props.depth != nextProps.depth
@@ -58,11 +44,11 @@ const DominatorTreeItem = module.exports = createClass({
       onViewSourceInDebugger,
     } = this.props;
 
-    const retainedSize = this.formatNumber(item.retainedSize);
-    const percentRetainedSize = this.formatPercent(getPercentSize(item.retainedSize));
+    const retainedSize = formatNumber(item.retainedSize);
+    const percentRetainedSize = formatPercent(getPercentSize(item.retainedSize));
 
-    const shallowSize = this.formatNumber(item.shallowSize);
-    const percentShallowSize = this.formatPercent(getPercentSize(item.shallowSize));
+    const shallowSize = formatNumber(item.shallowSize);
+    const percentShallowSize = formatPercent(getPercentSize(item.shallowSize));
 
     // Build up our label UI as an array of each label piece, which is either a
     // string or a frame, and separators in between them.

devtools/client/memory/test/unit/test_utils.js

@@ -3,7 +3,8 @@
 
 /**
  * Tests the task creator `takeSnapshotAndCensus()` for the whole flow of
- * taking a snapshot, and its sub-actions.
+ * taking a snapshot, and its sub-actions. Tests the formatNumber and
+ * formatPercent methods.
  */
 
 let utils = require("devtools/client/memory/utils");
@@ -48,4 +49,28 @@ add_task(function *() {
 
   ok(utils.breakdownEquals(utils.getCustomBreakdowns()["My Breakdown"], custom),
     "utils.getCustomBreakdowns() returns custom breakdowns");
+
+  ok(true, "test formatNumber util functions");
+  equal(utils.formatNumber(12), "12", "formatNumber returns 12 for 12");
+
+  equal(utils.formatNumber(0), "0", "formatNumber returns 0 for 0");
+  equal(utils.formatNumber(-0), "0", "formatNumber returns 0 for -0");
+  equal(utils.formatNumber(+0), "0", "formatNumber returns 0 for +0");
+
+  equal(utils.formatNumber(1234567), "1 234 567",
+    "formatNumber adds a space every 3rd digit");
+  equal(utils.formatNumber(12345678), "12 345 678",
+    "formatNumber adds a space every 3rd digit");
+  equal(utils.formatNumber(123456789), "123 456 789",
+    "formatNumber adds a space every 3rd digit");
+
+  equal(utils.formatNumber(12, true), "+12",
+    "formatNumber can display number sign");
+  equal(utils.formatNumber(-12, true), "-12",
+    "formatNumber can display number sign (negative)");
+
+  ok(true, "test formatPercent util functions");
+  equal(utils.formatPercent(12), "12%", "formatPercent returns 12% for 12");
+  equal(utils.formatPercent(12345), "12 345%",
+    "formatPercent returns 12 345% for 12345");
 });

devtools/client/memory/utils.js

@@ -517,6 +517,42 @@ exports.openFilePicker = function({ title, filters, defaultName, mode }) {
   });
 };
 
+/**
+ * Format the provided number with a space every 3 digits, and optionally
+ * prefixed by its sign.
+ *
+ * @param {Number} number
+ * @param {Boolean} showSign (defaults to false)
+ */
+exports.formatNumber = function(number, showSign = false) {
+  const rounded = Math.round(number);
+  if (rounded === 0 || rounded === -0) {
+    return "0";
+  }
+
+  const abs = String(Math.abs(rounded));
+  // replace every digit followed by (sets of 3 digits) by (itself and a space)
+  const formatted = abs.replace(/(\d)(?=(\d{3})+$)/g, "$1 ");
+
+  if (showSign) {
+    const sign = rounded < 0 ? "-" : "+";
+    return sign + formatted;
+  }
+  return formatted;
+};
+
+/**
+ * Format the provided percentage following the same logic as formatNumber and
+ * an additional % suffix.
+ *
+ * @param {Number} percent
+ * @param {Boolean} showSign (defaults to false)
+ */
+exports.formatPercent = function(percent, showSign = false) {
+  return exports.L10N.getFormatStr("tree-item.percent",
+                           exports.formatNumber(percent, showSign));
+};
+
 /**
  * Creates a hash map mapping node IDs to its parent node.
  *