vega / vega-tooltip

Tooltip Plugin for Vega-Lite
http://vega.github.io/vega-tooltip/
BSD 3-Clause "New" or "Revised" License
79 stars 45 forks source link

Title/image special handling shouldn't be on by default (or at least should be documented and have a way to disable) #694

Open kanitw opened 2 years ago

kanitw commented 2 years ago

The following blocks in https://github.com/vega/vega-tooltip/blob/next/src/formatValue.ts#L19-25 seems pretty problematic for me

    if (title) {
      content += `<h2>${valueToHtml(title)}</h2>`;
    }

    if (image) {
      content += `<img src="${valueToHtml(image)}">`;
    }

doesn't seem to be documented and may cause unintended effect.

For example, if there is a field in the data source called title, it will automatically becomes "title" in the output even though users may not intend to do so.

image

spec -->

(Note: I need to add calculate to simulate that there is a field "title" in the example.)

I don't think we should do this by default since it will surprise people more.

cc: @domoritz @nyurik (You might have an idea why we had this block.)

domoritz commented 2 years ago

I agree we should document the default handling of special fields and add an option to disable it. When we added it, it made sense as a quick way to have nicer tooltips.

kanitw commented 2 years ago

I agree it's a nice feature to have, but I think it's not a good default since it's too magical that these two fields are treated inconsistently.

domoritz commented 2 years ago

I don't feel strongly about it but I do like it as a default since it is hard for people to know how to configure the tooltips from a spec alone (many don't have direct access to Vega Tooltip in their applications).

kanitw commented 2 years ago

many don't have direct access to Vega Tooltip in their applications

That's fair.

I think I'm ok with if the key is more esoteric like tooltipTitle, tooltip:title or __tooltipTitle__ or something. (same for image).

There are data tables with a field title everywhere (like movies title), so having this title by default seems pretty problematic.

kanitw commented 2 years ago

I think what we can do is to introduce titleKey and imageKey config and default to our preferred format that's hard to collide (e.g., tooltip:title).

For existing applications that already rely on "title" and "image" as a trick can set titleKey to "title" if they prefer.

domoritz commented 2 years ago

Agreed. For imageKey, it should be a set imageKeys since there can be more than one image.