-
Notifications
You must be signed in to change notification settings - Fork 846
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
Fixed Dropdown Menu trigger to work only onClick #2616
Conversation
Hi @unknown503, this change will break a feature where the user can select an item in one lifecycle click (down on trigger, move, up on item) |
on what story is that feature? |
I don't think there is one |
Can you tell me how to replicate the feature you mentioned? |
Can we have an option to use onClick because when I click I can "hold" without release then move to other screen area to reject any menu opening if I change my mind. |
I would like to have the menu appear directly above the trigger point and because of I think this should be configurable. Edit: ended up making it controlled and preventingDefault on the pointer down event. |
I think we need to stick with pointer events and distinguish between mouse and non-mouse devices. With a mouse, pointerdown is what we want, per @joaom00's comment. I agree the current behavior is broken for touch interactions. I dealt with this exact issue in Reach UI, in case you want to reference and update your fix: https://github.com/reach/reach-ui/blob/main/packages/listbox/src/reach-listbox.tsx#L1005-L1019 |
Fix was merged in |
@chaance the change to Select in the PR you reference doesn't fix the issue in DropDown right? |
Description
Changed
onPointerDown
event toonClick
event which triggers the dropdown only when you click and release the left mouse button.This was done because when you clicked without releasing the button, the dropdown opened, on pc this was not really an issue, but it was an annoying issue on mobile, where you could be scrolling down and unintentionally pressed any dropdown trigger, causing the menu to show even though it was not intended.
Further explanation can be found on this issue #173 which is closed by this PR. Tests were performed to verify the new changes.