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

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 this issue Sep 7, 2022 · 6 comments
Labels

Comments

@kanitw
Copy link
Member

kanitw commented Sep 7, 2022

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.)

@kanitw kanitw added the bug label Sep 7, 2022
@domoritz
Copy link
Member

domoritz commented Sep 7, 2022

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
Copy link
Member Author

kanitw commented Sep 8, 2022

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
Copy link
Member

domoritz commented Sep 8, 2022

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
Copy link
Member Author

kanitw commented Sep 8, 2022

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
Copy link
Member Author

kanitw commented Sep 8, 2022

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
Copy link
Member

domoritz commented Sep 9, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants