Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Menu, Select] Closes on mouseup while cursor is still on trigger if clickable bounds are larger than trigger rect #1232

Closed
atomiks opened this issue Dec 26, 2024 · 5 comments · Fixed by #1250
Closed

[Menu, Select] Closes on mouseup while cursor is still on trigger if clickable bounds are larger than trigger rect #1232

atomiks opened this issue Dec 26, 2024 · 5 comments · Fixed by #1250
Assignees
@onehanddev
Labels
bug 🐛 Something doesn't work component: menu This is the name of the generic UI component, not the React module! component: select This is the name of the generic UI component, not the React module!

Comments

@atomiks
Copy link
Contributor

atomiks commented Dec 26, 2024

Note: alignItemToTrigger must be false for Select

On the docs the Select uses ::before and ::after pseudo-elements to make the clickable target larger than the actual highlighted trigger itself. This area is clickable, but the mouseup check can't "measure" this part since getBoundingClientRect measures the inner trigger part, and so it closes on mouseup unexpectedly if you click inside the extended area.

Screen.Recording.2024-12-26.at.6.31.55.pm.mov

base-ui/packages/react/src/select/trigger/useSelectTrigger.ts

Line 110 in 1d4206e

const triggerRect = triggerRef.current.getBoundingClientRect();

@atomiks atomiks added component: select This is the name of the generic UI component, not the React module! component: menu This is the name of the generic UI component, not the React module! bug 🐛 Something doesn't work labels Dec 26, 2024
@onehanddev
Copy link
Contributor

Hi! @atomiks 👋
I’d like to pick this up. Could you please assign it to me?

@atomiks
Copy link
Contributor Author

atomiks commented Dec 26, 2024

Sure @onehanddev

@onehanddev
Copy link
Contributor

Hi @atomiks I have tried a fix and it works, but I was curious to know whats the purpose of having a :before and :after pseudo selector ? Is it possible to just use the padding to ensure the bounding box respects the extra spacing ?

@atomiks
Copy link
Contributor Author

atomiks commented Dec 30, 2024

@onehanddev we need to support these cases for users of the library regardless of our docs' design

@onehanddev
Copy link
Contributor

Yeah that makes sense, Thanks for the clarification 👍

onehanddev added a commit to onehanddev/base-ui that referenced this issue Dec 31, 2024
@onehanddev
fix(Select): handle pseudo-element bounds in mouseup detection
d1629c2 
- Add getPseudoElementPadding utility to measure ::before and ::after dimensions
- Update mouseup detection logic to include pseudo-element padding in clickable area
- Fix unexpected Select closing when clicking within extended pseudo-element bounds

Fixes mui#1232
@onehanddev onehanddev mentioned this issue Dec 31, 2024
Merged
1 task
onehanddev added a commit to onehanddev/base-ui that referenced this issue Dec 31, 2024
@onehanddev
fix(Menu): handle pseudo-element bounds in trigger click detection
4c17eea 
- Add getPseudoElementPadding utility to compute ::before/::after dimensions
- Refactor mouseup handler to check extended clickable area
- Update click detection logic to include pseudo-element padding
- Improve trigger bounds calculation for more accurate click handling

Fixes mui#1232 - Prevents menu from closing unexpectedly when clicking within
the pseudo-element extended area of the trigger.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@onehanddev onehanddev

Labels
bug 🐛 Something doesn't work component: menu This is the name of the generic UI component, not the React module! component: select This is the name of the generic UI component, not the React module!
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Select, Menu] Handle pseudo-element bounds in mouseup detection onehanddev/base-ui
2 participants
@onehanddev @atomiks