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.

Sign up for GitHub

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

Allow null and NaN in text templates #7360

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
94f76cb
allow null and NaN values to be shown in hovertemplates and other tex…
alexcjohnson Feb 9, 2025
a0b8e17
tighter scope for changes to hover label logic
alexcjohnson Feb 9, 2025
655282a
draftlog for 7360
alexcjohnson Feb 9, 2025
fa9867c
Merge branch 'master' into alex/template-null
alexcjohnson Feb 14, 2025
a348c24
nestedProperty: support retainNull for array get all
alexcjohnson Feb 17, 2025
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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