FYI@alexshoe.

@camdecoster A couple high-level comments before I dig into the gritty details:

• I'm starting to feel that- might be a better default fallback value than empty string, because it indicates that it's a placeholder forsomething, which looks less awkward when you're using the templated value as part of a sentence. To take an example from the Jasmine tests,'- of -' looks less awkward than' of '.

• We've discussed that there are two separate scenarios where a value may end up asundefined: (1) is where the attribute named in the template simply doesn't exist, and (2) is where the attribute exists, but some individual data values happen to be undefined. I do think these two cases should be handled differently. The first is an error on the chart developer's part, and they should be alerted to it so they can fix it; the second is a normal quirk of the data and should be handled seamlessly. Open to discussing further but at the moment my suggestion is:

  • If the attribute named in the hovertemplate does not exist (e.g. dev has provided hovertemplate'value: %{foo:.2f}' buttrace.foo is not an existing attribute):Maintain existing behavior -- raw template string is rendered in hover tooltip
  • If attribute named in hovertemplatedoes exist, but the individual data value happens to be undefined (e.g. dev has provided hovertemplate'value: %{foo:.2f}' andtrace.foo is[1, undefined, 2]:Template string is replaced withhovertemplatefallback, AND no warning is emitted. So basically the behavior implemented in this PR, but without the warning. My feeling is that the dev has not done anything wrong and doesn't need to change anything, so a warning is just log noise at that point.

@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'value: foo not found in bar'?

I'm fine with delineating between the two cases, so I'll work on that. Using'-' as the default also seems reasonable as the empty string may not be interpreted as missing data; it could just look wrong.

@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'value: %{foo:.2f}' would become'value: ?foo?' ?

@camdecoster My above comments still stand but I've finished reviewing the code and everything looks great otherwise 👍

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 sethovertemplatefallback ortexttemplatefallback tofalse.

Let's see how the community responds (if they do) and we can adjust if necessary.