Skip to content
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

Allow null and NaN in text templates #7360

Merged
merged 5 commits into from
Feb 17, 2025
Merged

Conversation

alexcjohnson
Copy link
Collaborator

Fixes #7317

There can be good reasons to leave null values in the data to be shown in hover and text templates, but we were treating this as a broken template string. This allows those values to stay and get stringified.

@alexcjohnson alexcjohnson added the bug something broken label Feb 9, 2025
keep the original isValidTextValue except in templating
@gvwilson gvwilson requested a review from emilykl February 10, 2025 14:47
@gvwilson gvwilson added P1 needed for current cycle fix fixes something broken and removed bug something broken labels Feb 10, 2025
Copy link
Contributor

@emilykl emilykl left a comment

Choose a reason for hiding this comment

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

Looks good to me! 🚀

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

@alexcjohnson alexcjohnson merged commit 08da201 into master Feb 17, 2025
1 check passed
@alexcjohnson alexcjohnson deleted the alex/template-null branch February 17, 2025 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix fixes something broken P1 needed for current cycle
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hovertemplate should allow nil values from arrays
3 participants