-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix #6809: Make sure editable dropdown opens when user types #6810
Fix #6809: Make sure editable dropdown opens when user types #6810
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Ignored Deployments
|
Thanks a lot for your contribution! But, PR does not seem to be linked to any issues. Please manually link to an issue or mention it in the description using #<issue_id>. |
@peconomou929 can you retest the issue in #6027 that caused me to make the original change please? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I must have had a good reason to make this change
@melloware I am testing my changes with a dropdown that has |
Nice! From looking at this code again it has me wondering. Why don't show() and hide() just check the overlayvisible state instead of everywhere in the code? Like for example: const show = () => {
if (overlayVisibleState) return;
setFocusedOptionIndex(focusedOptionIndex !== -1 ? focusedOptionIndex : props.autoOptionFocus ? findFirstFocusedOptionIndex() : props.editable ? -1 : findSelectedOptionIndex());
setOverlayVisibleState(true);
}; I know its not related to your PR but doesn't it make sense that show() and hide() would just check the overlayVisibleState? |
Hmm, I see your point. It might make sense to do that, but I'm not entirely sure it's a good idea (for now at least). For example, the
In addition to setting the visible state, it also sets the |
ok i will take a look! |
Fix #6809.
This bug did not always exist. It was first introduced in #6032.