Skip to content

fix(compactSelect): Escape quotes inside option values #51007

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

Merged
merged 4 commits into from
Jun 15, 2023

Conversation

vuluongj20
Copy link
Member

@vuluongj20 vuluongj20 commented Jun 14, 2023

CompactSelect crashes when there's a quote inside any option's value string, e.g. {value: 'hello"', label: …}

Screen.Recording.2023-06-14.at.4.10.06.PM.mov

This happens because react-aria's useOverlay() hook query-selects option containers using a CSS selector string that contains the option's value:
image

To fix this, we can escape each option's value string (using CSS.escape()), making them safe for query selection. The escaped version is only used internally. The onChange() callback will still return options with their initial, unescaped values.

After the fix, quoted option values no longer cause a crash:

Screen.Recording.2023-06-14.at.4.14.46.PM.mov

@vuluongj20 vuluongj20 requested a review from a team as a code owner June 14, 2023 23:14
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Jun 14, 2023
@vuluongj20 vuluongj20 requested a review from 0Calories June 14, 2023 23:16
Copy link
Member

@0Calories 0Calories left a comment

Choose a reason for hiding this comment

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

🔥

@vuluongj20 vuluongj20 merged commit d54889a into master Jun 15, 2023
@vuluongj20 vuluongj20 deleted the vl/compact-select-quoted-values branch June 15, 2023 21:04
@github-actions github-actions bot locked and limited conversation to collaborators Jul 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Frontend Automatically applied to PRs that change frontend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants