Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions draftlogs/7360_fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Fix hoverlabels and other text labels with null values templated in [[#7360](https://github.com/plotly/plotly.js/pull/7360)]
12 changes: 4 additions & 8 deletions src/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 : '';
});
};

Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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;
}
Expand Down
4 changes: 2 additions & 2 deletions src/lib/nested_property.js
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexcjohnson I'm curious why line 90 doesn't need to be

out[j] = npGet(curCont[j], parts.slice(i + 1))(retainNull);

i.e. passing the retainNull value to the recursive function call.

Not important or a blocker, just idle curiousity!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch @emilykl! I'm not sure we actually use array index -1 for "get properties inside each array element" anywhere (I suppose users could access this via Plotly.restyle or Plotly.relayout but that way they don't have access to nestedProperty directly), and I certainly didn't need it for my use case, but since it's there we should support it -> a348c24

Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
};
}
Expand Down
28 changes: 28 additions & 0 deletions test/jasmine/tests/lib_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2443,6 +2443,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() {
Expand Down Expand Up @@ -2471,6 +2479,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;');
});
Expand Down Expand Up @@ -2537,6 +2555,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}', {});
Expand Down