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

Change warning for <select value={null} /> to suggest an empty array rather than empty string #9038

Closed
ryb73 opened this issue Feb 21, 2017 · 6 comments

Comments

@ryb73
Copy link

ryb73 commented Feb 21, 2017

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
Setting the value to "" on a <select> with size > 3 does not deselect the currently selected option.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://jsfiddle.net or similar (template: https://jsfiddle.net/reactjs/69z2wepo/).
JSFiddle: https://jsfiddle.net/rpbiwer/55jzq7dq/2/

What is the expected behavior?
After rendering a <select> with the value set to an empty string, I expect there to be no selection in the rendered select box. Mainly this expectation comes from this React warning when I use null instead of "":

Warning: `value` prop on `select` should not be null. Consider using the empty string to clear the component or `undefined` for uncontrolled components.

In the fiddle, if I change this line:

let newSelection = nowHidden ? "" : this.state.selected;

to

let newSelection = nowHidden ? undefined : this.state.selected;

I get the behavior I expect.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
I've gotten this with React 15.3.0 and whichever version is used in the fiddle. I don't know if it worked in previous versions.
This was tested using Chrome 56 on OS X.

@syranide
Copy link
Contributor

"In my opinion" this is/was intended behavior. Empty string is a valid option value. Also, at least earlier, not all browsers support having no option selected which means that for these browsers the controlled input would be broken as it would have an option selected visually for the user but the value would still be null. But if that has changed and all browsers now do support it, then having a way to set "no value" would be a good idea yes.

@ryb73
Copy link
Author

ryb73 commented Feb 22, 2017

@syranide thanks for pointing that out. I wasn't aware of how the browsers handle this sort of thing. If what you say is still true, I'd change the request in this issue from "make "" work how I want it to" to "revise the misleading 'Consider using the empty string' error message".

@gaearon
Copy link
Collaborator

gaearon commented Oct 4, 2017

I think the correct fix here is to emit a different warning for select with props.multiple set to true. In this case we want to suggest an empty array rather than an empty string.

@gaearon gaearon changed the title Setting <select> value to empty string doesn't deselect Change warning for <select value={null} /> to suggest an empty array rather than empty string Oct 4, 2017
@nealwright
Copy link
Contributor

Submitted a proposed fix: pull request #11141

@maecapozzi
Copy link

@Hendeca, it looks like your PR has been approved.

@gaearon
Copy link
Collaborator

gaearon commented Oct 31, 2017

Fixed in #11141.

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

No branches or pull requests

6 participants