Merge pull request #547 from plotly/pie-separators

Pie: number separators formatting

Author: mdtusz

src/lib/index.js

@@ -541,3 +541,47 @@ lib.objectFromPath = function(path, value) {
 
     return obj;
 };
+
+/**
+ * Converts value to string separated by the provided separators.
+ *
+ * @example
+ * lib.numSeparate(2016, '.,');
+ * // returns '2016'
+ *
+ * @example
+ * lib.numSeparate(1234.56, '|,')
+ * // returns '1,234|56'
+ *
+ * @param   {string|number} value       the value to be converted
+ * @param   {string}    separators  string of decimal, then thousands separators
+ *
+ * @return  {string}    the value that has been separated
+ */
+lib.numSeparate = function(value, separators) {
+
+    if(typeof separators !== 'string' || separators.length === 0) {
+        throw new Error('Separator string required for formatting!');
+    }
+
+    if(typeof value === 'number') {
+        value = String(value);
+    }
+
+    var thousandsRe = /(\d+)(\d{3})/,
+        decimalSep = separators.charAt(0),
+        thouSep = separators.charAt(1);
+
+    var x = value.split('.'),
+        x1 = x[0],
+        x2 = x.length > 1 ? decimalSep + x[1] : '';
+
+    // Years are ignored for thousands separators
+    if(thouSep && (x.length > 1 || x1.length>4)) {
+        while(thousandsRe.test(x1)) {
+            x1 = x1.replace(thousandsRe, '$1' + thouSep + '$2');
+        }
+    }
+
+    return x1 + x2;
+};

src/plots/cartesian/axes.js

@@ -1105,7 +1105,7 @@ function numFormat(v, ax, fmtoverride, hover) {
             if(dp) v = v.substr(0, dp + tickRound).replace(/\.?0+$/, '');
         }
         // insert appropriate decimal point and thousands separator
-        v = numSeparate(v, ax._gd._fullLayout.separators);
+        v = Lib.numSeparate(v, ax._gd._fullLayout.separators);
     }
 
     // add exponent
@@ -1141,27 +1141,6 @@ function numFormat(v, ax, fmtoverride, hover) {
     return v;
 }
 
-// add arbitrary decimal point and thousands separator
-var findThousands = /(\d+)(\d{3})/;
-function numSeparate(nStr, separators) {
-    // separators - first char is decimal point,
-    // next char is thousands separator if there is one
-
-    var dp = separators.charAt(0),
-        thou = separators.charAt(1),
-        x = nStr.split('.'),
-        x1 = x[0],
-        x2 = x.length > 1 ? dp + x[1] : '';
-    // even if there is a thousands separator, don't use it on
-    // 4-digit integers (like years)
-    if(thou && (x.length > 1 || x1.length>4)) {
-        while(findThousands.test(x1)) {
-            x1 = x1.replace(findThousands, '$1' + thou + '$2');
-        }
-    }
-    return x1 + x2;
-}
-
 
 axes.subplotMatch = /^x([0-9]*)y([0-9]*)$/;
 

src/traces/pie/calc.js

@@ -107,14 +107,15 @@ module.exports = function calc(gd, trace) {
             hasText = trace.textinfo.indexOf('text') !== -1,
             hasValue = trace.textinfo.indexOf('value') !== -1,
             hasPercent = trace.textinfo.indexOf('percent') !== -1,
+            separators = fullLayout.separators,
             thisText;
 
         for(i = 0; i < cd.length; i++) {
             pt = cd[i];
             thisText = hasLabel ? [pt.label] : [];
             if(hasText && trace.text[pt.i]) thisText.push(trace.text[pt.i]);
-            if(hasValue) thisText.push(helpers.formatPieValue(pt.v));
-            if(hasPercent) thisText.push(helpers.formatPiePercent(pt.v / vTotal));
+            if(hasValue) thisText.push(helpers.formatPieValue(pt.v, separators));
+            if(hasPercent) thisText.push(helpers.formatPiePercent(pt.v / vTotal, separators));
             pt.text = thisText.join('<br>');
         }
     }

src/traces/pie/helpers.js

@@ -8,14 +8,20 @@
 
 'use strict';
 
-exports.formatPiePercent = function formatPiePercent(v) {
+var Lib = require('../../lib');
+
+exports.formatPiePercent = function formatPiePercent(v, separators) {
     var vRounded = (v * 100).toPrecision(3);
-    if(vRounded.indexOf('.') !== -1) return vRounded.replace(/[.]?0+$/,'') + '%';
-    return vRounded + '%';
+    if(vRounded.lastIndexOf('.') !== -1) {
+        vRounded = vRounded.replace(/[.]?0+$/, '');
+    }
+    return Lib.numSeparate(vRounded, separators) + '%';
 };
 
-exports.formatPieValue = function formatPieValue(v) {
+exports.formatPieValue = function formatPieValue(v, separators) {
     var vRounded = v.toPrecision(10);
-    if(vRounded.indexOf('.') !== -1) return vRounded.replace(/[.]?0+$/,'');
-    return vRounded;
+    if(vRounded.lastIndexOf('.') !== -1) {
+        vRounded = vRounded.replace(/[.]?0+$/, '');
+    }
+    return Lib.numSeparate(vRounded, separators);
 };

src/traces/pie/plot.js

@@ -103,13 +103,15 @@ module.exports = function plot(gd, cdpie) {
                     var rInscribed = getInscribedRadiusFraction(pt, cd0),
                         hoverCenterX = cx + pt.pxmid[0] * (1 - rInscribed),
                         hoverCenterY = cy + pt.pxmid[1] * (1 - rInscribed),
+                        separators = fullLayout.separators,
                         thisText = [];
+
                     if(hoverinfo.indexOf('label') !== -1) thisText.push(pt.label);
                     if(trace2.text && trace2.text[pt.i] && hoverinfo.indexOf('text') !== -1) {
                         thisText.push(trace2.text[pt.i]);
                     }
-                    if(hoverinfo.indexOf('value') !== -1) thisText.push(helpers.formatPieValue(pt.v));
-                    if(hoverinfo.indexOf('percent') !== -1) thisText.push(helpers.formatPiePercent(pt.v / cd0.vTotal));
+                    if(hoverinfo.indexOf('value') !== -1) thisText.push(helpers.formatPieValue(pt.v, separators));
+                    if(hoverinfo.indexOf('percent') !== -1) thisText.push(helpers.formatPiePercent(pt.v / cd0.vTotal, separators));
 
                     Fx.loneHover({
                         x0: hoverCenterX - rInscribed * cd0.r,

test/image/baselines/pie_fonts.png

@@ -2,8 +2,8 @@
   "data": [
     {
       "values": [
-        1,
-        9
+        1.151,
+        8.134
       ],
       "text": [
         "inherit from<br><b>global...</b>",
@@ -17,8 +17,8 @@
     },
     {
       "values": [
-        1,
-        9
+        1.151,
+        8.134
       ],
       "text": [
         "a font of...",
@@ -37,8 +37,8 @@
     },
     {
       "values": [
-        1,
-        9
+        1.151,
+        8.134
       ],
       "text": [
         "outside and textfont<br>both modify...",
@@ -58,8 +58,8 @@
     },
     {
       "values": [
-        1,
-        9
+        1.151,
+        8.134
       ],
       "text": [
         "independent fonts<br>outside and...",
@@ -89,6 +89,7 @@
       "family": "Old Standard TT, serif",
       "size": 20
     },
-    "showlegend": true
+    "showlegend": true,
+    "separators": "∆|"
   }
 }

test/image/mocks/pie_fonts.json

@@ -111,4 +111,57 @@ describe('pie hovering', function() {
             }, 100);
         });
     });
+
+    describe('labels', function() {
+
+        var gd,
+            mockCopy;
+
+        beforeEach(function() {
+            gd = createGraphDiv();
+            mockCopy = Lib.extendDeep({}, mock);
+        });
+
+        afterEach(destroyGraphDiv);
+
+        it('should show the default selected values', function(done) {
+
+            var expected = ['4', '5', '33.3%'];
+
+            Plotly.plot(gd, mockCopy.data, mockCopy.layout).then(function() {
+
+                mouseEvent('mouseover', 230, 150);
+
+                var labels = Plotly.d3.selectAll('.hovertext .nums .line');
+
+                expect(labels[0].length).toBe(3);
+
+                labels.each(function(_, i) {
+                    expect(Plotly.d3.select(this).text()).toBe(expected[i]);
+                });
+            }).then(done);
+        });
+
+        it('should show the correct separators for values', function(done) {
+
+            var expected = ['0', '12|345|678@91', '99@9%'];
+
+            mockCopy.layout.separators = '@|';
+            mockCopy.data[0].values[0] = 12345678.912;
+            mockCopy.data[0].values[1] = 10000;
+
+            Plotly.plot(gd, mockCopy.data, mockCopy.layout).then(function() {
+
+                mouseEvent('mouseover', 230, 150);
+
+                var labels = Plotly.d3.selectAll('.hovertext .nums .line');
+
+                expect(labels[0].length).toBe(3);
+
+                labels.each(function(_, i) {
+                    expect(Plotly.d3.select(this).text()).toBe(expected[i]);
+                });
+            }).then(done);
+        });
+    });
 });

test/jasmine/tests/hover_pie_test.js

@@ -930,4 +930,35 @@ describe('Test lib.js:', function() {
 
         });
     });
+
+    describe('numSeparate', function() {
+
+        it('should work on numbers and strings', function() {
+            expect(Lib.numSeparate(12345.67, '.,')).toBe('12,345.67');
+            expect(Lib.numSeparate('12345.67', '.,')).toBe('12,345.67');
+        });
+
+        it('should ignore years', function() {
+            expect(Lib.numSeparate(2016, '.,')).toBe('2016');
+        });
+
+        it('should work for multiple thousands', function() {
+            expect(Lib.numSeparate(1000000000, '.,')).toBe('1,000,000,000');
+        });
+
+        it('should work when there\'s only one separator', function() {
+            expect(Lib.numSeparate(12.34, '|')).toBe('12|34');
+            expect(Lib.numSeparate(1234.56, '|')).toBe('1234|56');
+        });
+
+        it('should throw an error when no separator is provided', function() {
+            expect(function() {
+                Lib.numSeparate(1234);
+            }).toThrowError('Separator string required for formatting!');
+
+            expect(function() {
+                Lib.numSeparate(1234, '');
+            }).toThrowError('Separator string required for formatting!');
+        });
+    });
 });