[Select] Fix mismatch with hidden native select #1421
Conversation
| const nativeOption = React.useMemo( | ||
| () => ( | ||
| <option key={itemContext.value} value={itemContext.value} disabled={itemContext.disabled}> | ||
| {textContent} | ||
| </option> | ||
| ), | ||
| [itemContext.disabled, itemContext.value, textContent] | ||
| ); |
There was a problem hiding this comment.
I am building the option element directly here, this has 2 advantages in my opinion:
- it makes it more clear/explicit what this is used for, seeing the native option collocated here
- it means I don't need to invent a new data structure like
{ value, label: textContent, disabled }just to pass it up and interpolate it higher up.
There was a problem hiding this comment.
haha yeah.. my initial gut reaction was to ask why we don't render closer, I think I'm just conditioned by typically seeing these built closer to the render. But:
it means I don't need to invent a new data structure like { value, label: textContent, disabled } just to pass it up and interpolate it higher up.
Actually makes much more sense, it's easier to follow when you realise the alternative is just going around the houses for no great reason.
| const nativeOption = React.useMemo( | ||
| () => ( | ||
| <option key={itemContext.value} value={itemContext.value} disabled={itemContext.disabled}> | ||
| {textContent} | ||
| </option> | ||
| ), | ||
| [itemContext.disabled, itemContext.value, textContent] | ||
| ); |
There was a problem hiding this comment.
haha yeah.. my initial gut reaction was to ask why we don't render closer, I think I'm just conditioned by typically seeing these built closer to the render. But:
it means I don't need to invent a new data structure like { value, label: textContent, disabled } just to pass it up and interpolate it higher up.
Actually makes much more sense, it's easier to follow when you realise the alternative is just going around the houses for no great reason.
Co-authored-by: Andy Hook <[email protected]>
* [Select] Fix mismatch with hidden native select Closes radix-ui#1223 * PR feedback * Update packages/react/select/src/Select.tsx Co-authored-by: Andy Hook <[email protected]> Co-authored-by: Andy Hook <[email protected]>
Closes #1223