Changed the error message displayed when a select element has props.multiple set to true and value set to null #11141
Conversation
| 'for uncontrolled components.%s', | ||
| type, | ||
| type === 'select' ? 'array' : 'string', |
There was a problem hiding this comment.
This doesn't seem to check for .multiple.
|
Can you please rebase the branch of master instead of merging? This way unrelated commits won't be in the history. |
nealwright
force-pushed
the
select-null-input-value-error-message
branch
from
9d8f6d9 to
53e7f7e
Compare
|
@gaearon Okay I've squashed my commits and rebased so you should now see only my changes. I also added the check for the property |
nealwright
force-pushed
the
select-null-input-value-error-message
branch
2 times, most recently
from
20d12a7 to
28a6063
Compare
nealwright
force-pushed
the
select-null-input-value-error-message
branch
from
28a6063 to
10b8636
Compare
|
I just rebased with master to fix the merge conflicts that arose. @gaearon Is there anything else I need to do for this pull request? |
|
|
||
| expectDev(console.error.calls.argsFor(0)[0]).toContain( | ||
| '`value` prop on `select` should not be null. ' + | ||
| 'Consider using an empty array when multiple is ' + |
There was a problem hiding this comment.
Nit: value and select are backticked but multiple isn't. Let's backtick it too.
| expectDev(console.error.calls.argsFor(0)[0]).toContain( | ||
| '`value` prop on `select` should not be null. ' + | ||
| 'Consider using an empty array when multiple is ' + | ||
| 'set to true to clear the component or `undefined` ' + |
nealwright
force-pushed
the
select-null-input-value-error-message
branch
from
10b8636 to
5e90c3e
Compare
nealwright
force-pushed
the
select-null-input-value-error-message
branch
from
5e90c3e to
908ad12
Compare
|
@gaearon I've added the requested changes. Let me know if you need anything else, thanks! |
| if (props != null && props.value === null && !didWarnValueNull) { | ||
|
|
||
| var isMultipleSelect = type === 'select' && props.multiple === true; | ||
| var errorType = isMultipleSelect ? 'multiple' : 'default'; |
There was a problem hiding this comment.
I don't think this is important. I'd prefer a single boolean to deduplicate (even though deduplicating one will influence the other). They're rare enough.
Then we would move the isMultipleSelect check inside the condition. So it's only checked in error case.
|
I made a few changes:
|
|
@gaearon Thanks, sorry I wasn't able to get to this faster! |
…ultiple set to true and value set to null (facebook#11141) * Corrects error message for select with props.multiple set to true and a null value. * Don't bother deduplicating based on type * Make the code a bit simpler (and more verbose)
Addresses issue #9038
I changed the error message shown when a select element with props.multiple set to true has a value of
nullto recommend an empty array instead of an empty string. Given that "the empty string" is a common term but "the empty array" is less so, I also changed the null input value error message shown for textarea and input to use "an empty string" instead of "the empty string" and changed tests to match.