Merge pull request #7360 from plotly/alex/template-null

Allow null and NaN in text templates

Author: alexcjohnson

draftlogs/7360_fix.md

@@ -0,0 +1 @@
+- Fix hoverlabels and other text labels with null values templated in [[#7360](https://github.com/plotly/plotly.js/pull/7360)]

src/lib/index.js

@@ -1067,9 +1067,9 @@ lib.templateString = function(string, obj) {
             v = obj[key];
         } else {
             getterCache[key] = getterCache[key] || lib.nestedProperty(obj, key).get;
-            v = getterCache[key]();
+            v = getterCache[key](true);  // true means don't replace undefined with null
         }
-        return lib.isValidTextValue(v) ? v : '';
+        return (v !== undefined) ? v : '';
     });
 };
 
@@ -1132,9 +1132,6 @@ function templateFormatString(string, labels, d3locale) {
     var opts = this;
     var args = arguments;
     if(!labels) labels = {};
-    // Not all that useful, but cache nestedProperty instantiation
-    // just in case it speeds things up *slightly*:
-    var getterCache = {};
 
     return string.replace(lib.TEMPLATE_STRING_REGEX, function(match, rawKey, format) {
         var isOther =
@@ -1185,9 +1182,8 @@ function templateFormatString(string, labels, d3locale) {
                 }
 
                 if(!SIMPLE_PROPERTY_REGEX.test(key)) {
-                    value = lib.nestedProperty(obj, key).get();
-                    value = getterCache[key] || lib.nestedProperty(obj, key).get();
-                    if(value) getterCache[key] = value;
+                    // true here means don't convert null to undefined
+                    value = lib.nestedProperty(obj, key).get(true);
                 }
                 if(value !== undefined) break;
             }

src/lib/nested_property.js

@@ -73,7 +73,7 @@ module.exports = function nestedProperty(container, propStr) {
 };
 
 function npGet(cont, parts) {
-    return function() {
+    return function(retainNull) {
         var curCont = cont;
         var curPart;
         var allSame;
@@ -87,7 +87,7 @@ function npGet(cont, parts) {
                 allSame = true;
                 out = [];
                 for(j = 0; j < curCont.length; j++) {
-                    out[j] = npGet(curCont[j], parts.slice(i + 1))();
+                    out[j] = npGet(curCont[j], parts.slice(i + 1))(retainNull);
                     if(out[j] !== out[0]) allSame = false;
                 }
                 return allSame ? out[0] : out;
@@ -105,7 +105,7 @@ function npGet(cont, parts) {
         if(typeof curCont !== 'object' || curCont === null) return undefined;
 
         out = curCont[parts[i]];
-        if(out === null) return undefined;
+        if(!retainNull && (out === null)) return undefined;
         return out;
     };
 }

test/jasmine/tests/lib_test.js

@@ -340,11 +340,12 @@ describe('Test lib.js:', function() {
         });
 
         it('should access properties of objects in an array with index -1', function() {
-            var obj = {arr: [{a: 1}, {a: 2}, {b: 3}]};
+            var obj = {arr: [{a: 1}, {a: null}, {b: 3}]};
             var prop = np(obj, 'arr[-1].a');
 
-            expect(prop.get()).toEqual([1, 2, undefined]);
-            expect(obj).toEqual({arr: [{a: 1}, {a: 2}, {b: 3}]});
+            expect(prop.get()).toEqual([1, undefined, undefined]);
+            expect(prop.get(true)).toEqual([1, null, undefined]);
+            expect(obj).toEqual({arr: [{a: 1}, {a: null}, {b: 3}]});
 
             prop.set(5);
             expect(prop.get()).toBe(5);
@@ -2443,6 +2444,14 @@ describe('Test lib.js:', function() {
         it('should work with the number *0* (nested case)', function() {
             expect(Lib.templateString('%{x.y}', {x: {y: 0}})).toEqual('0');
         });
+
+        it('preserves null and NaN', function() {
+            expect(Lib.templateString(
+                '%{a} %{b} %{c.d} %{c.e} %{f[0]} %{f[1]}',
+                {a: null, b: NaN, c: {d: null, e: NaN}, f: [null, NaN]}
+            ))
+            .toEqual('null NaN null NaN null NaN');
+        });
     });
 
     describe('hovertemplateString', function() {
@@ -2471,6 +2480,16 @@ describe('Test lib.js:', function() {
             expect(Lib.hovertemplateString('%{x.y}', {}, locale, {x: {y: 0}})).toEqual('0');
         });
 
+        it('preserves null and NaN', function() {
+            expect(Lib.hovertemplateString(
+                '%{a} %{b} %{c.d} %{c.e} %{f[0]} %{f[1]}',
+                {},
+                locale,
+                {a: null, b: NaN, c: {d: null, e: NaN}, f: [null, NaN]}
+            ))
+            .toEqual('null NaN null NaN null NaN');
+        });
+
         it('subtitutes multiple matches', function() {
             expect(Lib.hovertemplateString('foo %{group} %{trace}', {}, locale, {group: 'asdf', trace: 'jkl;'})).toEqual('foo asdf jkl;');
         });
@@ -2537,6 +2556,16 @@ describe('Test lib.js:', function() {
             expect(Lib.texttemplateString('y: %{y}', {yLabel: '0.1'}, locale, {y: 0.123})).toEqual('y: 0.1');
         });
 
+        it('preserves null and NaN', function() {
+            expect(Lib.texttemplateString(
+                '%{a} %{b} %{c.d} %{c.e} %{f[0]} %{f[1]}',
+                {},
+                locale,
+                {a: null, b: NaN, c: {d: null, e: NaN}, f: [null, NaN]}
+            ))
+            .toEqual('null NaN null NaN null NaN');
+        });
+
         it('warns user up to 10 times if a variable cannot be found', function() {
             spyOn(Lib, 'warn').and.callThrough();
             Lib.texttemplateString('%{idontexist}', {});