fix(Select): display label for empty string value #543
Conversation
|
You must have Developer access to commit code to ClickHouse on Vercel. If you contact an administrator and receive Developer access, commit again to see your changes. Learn more: https://vercel.com/docs/accounts/team-members-and-roles/access-roles#team-level-roles |
| @@ -21,9 +21,6 @@ const SingleSelectValue = ({ | |||
| value: string; | |||
| }) => { | |||
| const { icon, iconDir, children, label } = valueNode ?? {}; | |||
| if (!value) { | |||
| return null; | |||
| } | |||
There was a problem hiding this comment.
Why did we have it, tho? What was the case for this if?
There was a problem hiding this comment.
It was initially there after the major refactoring: #137
I don't see a reason for it. The tests are green.
@vineethasok do you remember if there is a specific case for this check?
There was a problem hiding this comment.
This was inorder to not have any content at all when value is empty and not render the IconContainer component
If it is not causing any issue we can remove it but I added this for this reason
There was a problem hiding this comment.
an empty string is a valid value, and we should treat it as any other string
I can keep check for undefined and null though, if you think it's needed. As of now, I don't see a way to pass these values here
There was a problem hiding this comment.
you know what, I'll keep the check for undefined, it just makes sense
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| item && fireEvent.click(item); | ||
| expect(selectTrigger.textContent).toBe(OPTION_TEXT); | ||
| }); | ||
|
|
Resolves #490
Why
Putting empty string as a value of
Select.Itemresults in an empty Select:What
The
valueprop ofSingleSelectValueis typed as required. Also by usage, it gets rendered only whenselectedValuesarray is not empty. Thus in normal circumstances it can't be undefined or null. Seems like the emptiness check just causes the bug while not doing anything else.