Skip to content

Fixes select warning on falsy value in EuiSelect#1254

Merged
cqliu1 merged 2 commits into
elastic:masterfrom
cqliu1:fix/select-value-check
Oct 19, 2018
Merged

Fixes select warning on falsy value in EuiSelect#1254
cqliu1 merged 2 commits into
elastic:masterfrom
cqliu1:fix/select-value-check

Conversation

@cqliu1
Copy link
Copy Markdown
Contributor

@cqliu1 cqliu1 commented Oct 19, 2018

When I passed an empty string, 0 or false to the value prop in the EuiSelect component, I would get this React warning:
Warning: Select elements must be either controlled or uncontrolled (specify either the value prop, or the defaultValue prop, but not both). Decide between using a controlled or uncontrolled select element and remove one of these props. More info: https://fb.me/react-controlled-components

By switching from a falsy check to a check for null equality, these warnings no longer pop up for the falsy values.

@cqliu1 cqliu1 added the bug label Oct 19, 2018
@cqliu1 cqliu1 requested a review from chandlerprall October 19, 2018 17:59
@cqliu1 cqliu1 changed the title Changes from falsy check for value prop in select component to null equality Fixes select warning on falsy values Oct 19, 2018
@cqliu1 cqliu1 changed the title Fixes select warning on falsy values Fixes select warning on falsy value in EuiSelect Oct 19, 2018
@ryankeairns
Copy link
Copy Markdown
Contributor

LGTM! TGIF

Copy link
Copy Markdown
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

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

needs a bugfix changelog, then LGTM!

@cqliu1 cqliu1 force-pushed the fix/select-value-check branch from 406f284 to 01d0a0c Compare October 19, 2018 18:53
@cqliu1
Copy link
Copy Markdown
Contributor Author

cqliu1 commented Oct 19, 2018

@chandlerprall I added the bug fix to the changelog 👍

@cqliu1 cqliu1 force-pushed the fix/select-value-check branch from 01d0a0c to f524fb1 Compare October 19, 2018 18:54
Copy link
Copy Markdown
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @cqliu1 !

@cqliu1 cqliu1 merged commit ccd7f95 into elastic:master Oct 19, 2018
@cqliu1 cqliu1 deleted the fix/select-value-check branch October 25, 2018 04:25
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.

3 participants