Fix selection for controlled combobox#865
Conversation
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 43e4d0c:
|
| let id = useId(props.id); | ||
| let listboxId = id ? makeId("listbox", id) : "listbox"; | ||
|
|
||
| let [isControlled, setIsControlled] = React.useState(false); |
There was a problem hiding this comment.
Is there a reason we need new state for this? Could we not store this as a ref instead as we do lower in the tree? I don't believe we ever need it for rendering.
let isControlledRef = React.useRef(false);
// in ComboboxInput:
let isControlled = controlledValue != null;
React.useEffect(() => {
isControlledRef.current = isControlled;
}, [isControlled]);
// In ComboboxOption:
let { isControlledRef } = React.useContext(ComboboxContext);
let handleClick = () => {
onSelect && onSelect(value);
transition(SELECT_WITH_CLICK, { value, isControlled: isControlledRef.current });
};There was a problem hiding this comment.
@chaance that's exactly what I did originally, but decided to go with state, since setting that ref would not trigger a re-render, so I figured state would be more safe.
It would almost certainly work either way though. If you think a ref would be better I could change to that.
There was a problem hiding this comment.
@chaance ok I've got it ref based now. I'm putting just the boolean in context, as opposed to the ref itself. If you'd like me to remove the updater function, and just store the ref object, lemme know and I will.
|
Thanks for the changes, looks good 👍 |
Here's the context. In the existing Storybook, go to combobox controlled (third one). Then repeatedly type in the box, and select something. Occasionally (roughly 1 in 5 times) you'll see the text of the thing you select flash in the input, before being cleared. To make this more pronounced, and easier to see, edit this code:
to this
This PR fixes that, by changing the value to null when the combobox is used in controlled mode.
Thank you for contributing to Reach UI! Please fill in this template before submitting your PR to help us process your request more quickly.
yarn test).yarn start).This pull request:
If creating a new package:
examplesdirectorysrcdirectory with anindex.tsxentry filestyle.cssfile (if needed by the new package)