Skip to content

Allow null and NaN in text templates #7360

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Feb 17, 2025
Merged
Changes from all 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
@@ -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;
}
6 changes: 3 additions & 3 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
@@ -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;
};
}
35 changes: 32 additions & 3 deletions test/jasmine/tests/lib_test.js
Original file line number Diff line number Diff line change
@@ -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}', {});