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

Controls: Fix select / multiselect when value contains multiple spaces #22334

Merged
merged 7 commits into from Oct 9, 2023

Conversation

oxcened
Copy link
Contributor

@oxcened oxcened commented May 1, 2023

Closes #17051.

Analysis

Select and multiselect controls won’t work when selecting an option with multiple spaces, causing the select to reset and the URL reference to be removed or added as "undefined", in the case of multiselect. This is due to the options not receiving the value prop. According to the HTML specs:

The value of an option element is the value of the value content attribute, if there is one, or, if there is not, the value of the element's text IDL attribute.

So HTML is involved, which is notoriously unreliable about multiple white spaces.

What I did

I've added the value to both the select and multiselect components.

How to test

  1. Run a sandbox for template, e.g. yarn task --task sandbox --start-from auto --template react-vite/default-ts.
  2. Open Storybook in your browser.
  3. Head to addons > controls > basics > undefined story or /?path=/story/addons-controls-basics--undefined.
  4. Scroll down to the select control, click on it and select the option "double space".
  5. Do the same for the multiselect control.
  6. It should be selected.

Or just run the E2E tests I've added.

Checklist

  • Make sure your changes are tested (stories and/or unit, integration, or end-to-end tests)
  • Make sure to add/update documentation regarding your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Maintainers

  • If this PR should be tested against many or all sandboxes,
    make sure to add the ci:merged or ci:daily GH label to it.
  • Make sure this PR contains one of the labels below.

["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]

@JReinhold JReinhold added addon: controls ci:daily Run the CI jobs that normally run in the daily job. labels Aug 29, 2023
@JReinhold JReinhold changed the title Fix select and multiselect controls not working when value contains multiple spaces Controls: Fix select and multiselect controls not working when value contains multiple spaces Aug 29, 2023
@JReinhold JReinhold added the bug label Aug 29, 2023
@yannbf
Copy link
Member

yannbf commented Sep 19, 2023

Hey @oxcened thank you so much for your contribution! @JReinhold could you check it out once you have time? Thanks!

It might take some time as we are quite busy, we really appreciate your efforts and patience <3

@ndelangen ndelangen changed the title Controls: Fix select and multiselect controls not working when value contains multiple spaces Controls: Fix select and multiselect controls when value contains multiple spaces Sep 19, 2023
@ndelangen ndelangen changed the title Controls: Fix select and multiselect controls when value contains multiple spaces Controls: Fix select / multiselect when value contains multiple spaces Sep 19, 2023
@valentinpalkovic valentinpalkovic removed their request for review October 5, 2023 09:46
@ndelangen ndelangen merged commit 77d2da2 into storybookjs:next Oct 9, 2023
86 of 87 checks passed
@yannbf yannbf mentioned this pull request Oct 17, 2023
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addon: controls bug ci:daily Run the CI jobs that normally run in the daily job.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Select" control does not support string with extra spaces as an option
4 participants