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

fix: Escape Quotes in tooltip title #9019

Merged
merged 3 commits into from
Jul 19, 2023
Merged

fix: Escape Quotes in tooltip title #9019

merged 3 commits into from
Jul 19, 2023

Conversation

lsh
Copy link
Member

@lsh lsh commented Jul 19, 2023

Currently titles such as "\"mytitle\"" will cause the editor to break because it generates fields that look like:

({ ""mytitle"":  ... })

This PR escapes quotes to allow this case to pass.

@lsh lsh requested review from domoritz and kanitw July 19, 2023 23:05
@lsh lsh requested a review from yhoonkim July 19, 2023 23:05
@lsh lsh requested a review from fandu-db July 19, 2023 23:05
Copy link
Member

@kanitw kanitw left a comment

Choose a reason for hiding this comment

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

Can you add an example in examples/specs to show that this really does make it renderable?

@lsh lsh requested a review from kanitw July 19, 2023 23:41
@kanitw kanitw merged commit f7f7735 into main Jul 19, 2023
@kanitw kanitw deleted the lsh/quote-title-fix branch July 19, 2023 23:51
@@ -88,7 +88,7 @@ export function tooltipData(
};

const title = fieldDef.title || defaultTitle(fieldDef, formatConfig);
const key = array(title).join(', ');
const key = array(title).join(', ').replaceAll(/"/g, '\\"');
Copy link
Member

Choose a reason for hiding this comment

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

Huh, do we not have a utility method for this already? Does this problem not happen for other texts, axis labels or titles?

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't see one, but maybe I didn't look in the right place. I'm not sure the other areas have the same problem since this one specifically generates a dictionary rather than acts as a value.

BradyJ27 pushed a commit to BradyJ27/vega-lite that referenced this pull request Oct 19, 2023
Co-authored-by: GitHub Actions Bot <vega-actions-bot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants