-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
feat: Add template string fallback attributes #7577
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
base: master
Are you sure you want to change the base?
feat: Add template string fallback attributes #7577
Conversation
|
FYI @alexshoe. |
|
@camdecoster A couple high-level comments before I dig into the gritty details:
|
| ].join(' ') | ||
| }); | ||
|
|
||
| exports.templatefallbackAttrs = ({ editType = 'none' } = {}) => ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to set the correct editType here, I think it should be calc (or possibly plot?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After reviewing the changes again, I think it should be 'none' for hovertemplatefallback, 'calc' for texttemplatefallback, and 'arraydraw' for the shape text template fallback. I'll go through and double check all of them.
|
@emilykl in the case of a Studio app showing this, the creator isn't necessarily going to understand what a hover template is and would think that the output is a bug showing garbled data (which is how this issue was found in the first place). Maybe we could use a different default value in the case of a missing key name. Something like I'm fine with delineating between the two cases, so I'll work on that. Using |
|
@camdecoster Yeah I do feel strongly about differentiating the two cases (and issuing a warning only in the case where the variable name does not exist) but worth discussing what is the correct string to display in the "variable does not exist" case. Ideally something that doesn't use English words... what about the variable name surrounded by question marks? So that hovertemplate |
src/plots/template_attributes.js
Outdated
| 'Note that this will override `hoverinfo`.', | ||
| templateFormatStringDescription({ supportOther: true }), | ||
| 'The variables available in `hovertemplate` are the ones emitted as event data described at this link https://plotly.com/javascript/plotlyjs-events/#event-data.', | ||
| 'Additionally, every attributes that can be specified per-point (the ones that are `arrayOk: true`) are available.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| 'Additionally, every attributes that can be specified per-point (the ones that are `arrayOk: true`) are available.', | |
| 'Additionally, all attributes that can be specified per-point (the ones that are `arrayOk: true`) are available.', |
src/plots/template_attributes.js
Outdated
| 'Template string used for rendering the information text that appears on points.', | ||
| 'Note that this will override `textinfo`.', | ||
| templateFormatStringDescription(), | ||
| 'Every attributes that can be specified per-point (the ones that are `arrayOk: true`) are available.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| 'Every attributes that can be specified per-point (the ones that are `arrayOk: true`) are available.', | |
| 'All attributes that can be specified per-point (the ones that are `arrayOk: true`) are available.', |
src/plots/template_attributes.js
Outdated
| valType: 'string', | ||
| dflt: '', | ||
| editType, | ||
| description: "Fallback value that's displayed when a variable referenced in a template can't be found." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a comment to flag that this description should be updated once the final behavior is decided (e.g. Fallback value that's displayed when a variable referenced in a template has an undefined value)
| * @param {array} options.data - Data objects containing substitution values | ||
| * @param {string} options.fallback - Fallback value when substitution fails | ||
| * @param {object} options.labels - Data object containing fallback text when no formatting is specified, ex.: {yLabel: 'formattedYValue'} | ||
| * @param {object} options.locale - D3 locale for formatting |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: IMO this argument should continue to be called d3locale for clarity
|
@camdecoster My above comments still stand but I've finished reviewing the code and everything looks great otherwise 👍 |
…ty-string-template-format-function
This reverts commit 84fc044.
src/lib/index.js
Outdated
| if (!SIMPLE_PROPERTY_REGEX.test(key)) { | ||
| // true here means don't convert null to undefined | ||
| value = lib.nestedProperty(obj, key).get(true); | ||
| keyIsMissing = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hm this is a tough one, the question is whether lib.nestedProperty(obj, key).get() provides a way to distinguish between "the value of this key path is undefined" vs. "this key path was not found in the object"
|
I looked into providing a different value for missing vs. undefined values but that didn't work. When a value is undefined, the key for that variable might be missing from the data object passed in. So I moved forward with the fallback value getting used all the time when a value is missing. To return the specifier (the current behavior), you must set Let's see how the community responds (if they do) and we can adjust if necessary. |
Description
Add template string fallback attributes.
Closes #7564
Closes #7565
Changes
hovertemplatefallbackandtexttemplatefallbackattributestemplateFormatString(and derived functions) to pass fallback value in, use in case of missing variableDemo video or screenshots
Testing
{ "data": [ { "hovertemplate": "Time: %{x}<br>User: %{y}<br>Usage: $%{z:.2f}<extra></extra>", "hovertemplatefallback": "Citra Hops", "showscale": true, "z": { "dtype": "f8", "bdata": "fDCpKCrK3z8qBd4FpbLBv2heJFDdueQ/K6oBjlRe+D9AplffvPjNvzvbcCYz+M2/CHRAqHRE+T/z1/BG047oP4ueeUveC96/AAAAAAAA+H9wG8Guoqjdv+fvFiuEzt2/cPmepZ74zj++Ne6+y5z+vwAAAAAAAPh/YzyhakL+4b95UaNnjjTwvx+GFtigHNQ/+QOLgYgO7b/HExTEy5j2v0ENnx9Mc/c/2Zhm4TzmzL/GNbFGh0mxP2InDcHEy/a/KPrKUZVr4b81BjE/bGW8PxX1zj14avK/6N19s28L2D/SOz2hbjjjvwAAAAAAAPh/p/2pOS5B479X9hhz7qL9P4aDsNZupIu/AAAAAAAA+H83Lh+1SVLqPxx64lmTiPO/tWnv0gq8yj/DMFAPz1r/v/9asANAQPW/QDr0vL8yyT++nUiqhKHnPwwupFVl78U/XzGyNiCbvb/hr1BvSEXTvwa4sKwGqPe/J6aouPYI578AAAAAAAD4f2LC1/746fA/iEEAkdf91T9OFQ2YaTX8vwAAAAAAAPh/QA8qJjCl2L9NOahTWKnlv/nsCSfakuM/1Pz0Wvl+8D8enOD2C83tP1MVD7Xe2uq/zylztCLK07+KTA+JazPVPwBoGGqqN+8/VWYobMqq3r8AAAAAAAD4fzBuPE2Ms/G/9PWajqkj87+w1UonNgDqP/3Ng74os/U/+iMrXUFvsr9qgA6DeA7wP0KkKG0LJdc/rwoBL9Kk5L+P6aQIGyHXPwAAAAAAAPh/UhmlZMpXor8AAAAAAAD4f19fz+s89QTAv1gZewZN6j8hrkt3t0i2P8Dt4LnvItO/OIpGXqJ9tz/iHOwPFc3/v8FsDls1Hsy/04nor+7a1j8AAAAAAAD4fwAAAAAAAPh/S3Lu+S3f6b+hdpfJZA7gvwAAAAAAAPh/gMUgGEIK1T/02ryry/PgvwNc6tKvbOA/AAAAAAAA+H9TvHjHI//uP3hIyAw4d+a/l5Lxpmr41L8apYzLTBjZvxD0dqaOave/IzBrdqLz0j8UgR0sIbXQPw06yBDZ8XQ/hE70gPMGzr8EJo3KW6X2v3SGH1za69q/CzQW3gjv1b98MTRhQazpv8BzdZwCpcS/Mxz8H/jb2T8bAn5E0S3+P5EnXNaQWMY/AAAAAAAA+H9gVYk14w6zv+1yI3JJs/6/OEbHD3Qmm78AAAAAAAD4f7gI2ke4tANA5IZhukifyL9yMGE5jUzTPwgyV7e9xaG/zxHyveey8r9cb1eTAEnyP1Z/Bd3VD+g/", "shape": "10, 12" }, "type": "heatmap" } ] }hovertemplatefallbackto befalseand reload the mockhovertemplatefallbackfrom the mock and reload the mockNotes
'-'falsewill result in the specifier being returned (the current behavior)TODO