mui
[Select] Implement pointer cancellation
Description This PR resolves pointer cancellation issues in the Select component to ensure compliance with WCAG 2.5.2 accessibility requirements.
Changes
- Added proper pointer event handling to address accessibility concerns
- Included test cases to verify WCAG 2.5.2 compliance
Fixes #45301
Aditional
- [X] I have followed (at least) the PR section of the contributing guide.
Netlify deploy preview
https://deploy-preview-45789--material-ui.netlify.app/
Bundle size report
| Bundle | Parsed Size | Gzip Size |
|---|---|---|
| @mui/material | 🔺+518B(+0.10%) | 🔺+181B(+0.12%) |
| @mui/lab | 0B(0.00%) | 0B(0.00%) |
| @mui/system | 0B(0.00%) | 0B(0.00%) |
| @mui/utils | 0B(0.00%) | 0B(0.00%) |
Generated by :no_entry_sign: dangerJS against aff2e307d4e9e59362927266ae05a8461cd867ee
Thank you @ZeeshanTamboli for the guidance! I've revised the PR to focus solely on fixing issue #45301 . I've removed the changes related to the other issues as requested and added test cases to verify the behavior. Let me know if you'd like to see any additional changes.
Hey @Kartik-Murthy, thanks for working on this, sorry for the delay.
@ZeeshanTamboli, we can move forward with this review. I don't have an estimate for Material UI being rebuilt on top of Base UI.
About the solution, I wonder if there's a way of handling the mouse up from the items, instead of having to set a global event listener. Did you explore that idea?
About the solution, I wonder if there's a way of handling the mouse up from the items, instead of having to set a global event listener. Did you explore that idea?
@DiegoAndai I am not sure how we can do that. We need to handle the case where the mouse is released outside the menu entirely — and that can't be reliably detected unless you're listening on document. If we only attach mouseup on menu items, we'll miss clicks released outside the menu, meaning the menu may remain open when it shouldn’t.
@DiegoAndai I’ve moved the pointer cancellation logic into the mousedown handler instead of using a useEffect and detecting over there if the pointer is down or not. While the effect-based approach technically works, handling it directly in the event feels cleaner and could be more performant. What do you think?
While the effect-based approach technically works, handling it directly in the event feels cleaner and could be more performant. What do you think?
I agree, it's also how Base UI does it 👍🏼: https://github.com/mui/base-ui/blob/master/packages/react/src/select/trigger/SelectTrigger.tsx#L161
@ZeeshanTamboli good work, this looks almost ready for me. I'm only missing the selection when onMouseUp happens on a select item. I think we should add this, following Base's implementation which adds it to the items onMouseUp handler: https://github.com/mui/base-ui/blob/master/packages/react/src/select/item/SelectItem.tsx#L182
Would that be possible?
This is what I'm referencing, selecting with a single click:
https://github.com/user-attachments/assets/1cef8792-637e-484b-b740-3582971acfe5
@ZeeshanTamboli good work, this looks almost ready for me. I'm only missing the selection when onMouseUp happens on a select item. I think we should add this, following Base's implementation which adds it to the items onMouseUp handler: https://github.com/mui/base-ui/blob/master/packages/react/src/select/item/SelectItem.tsx#L182
Would that be possible?
This is what I'm referencing, selecting with a single click:
Screen.Recording.2025-07-04.at.15.32.11.mov
@DiegoAndai Do you consider this to be part of the pointer cancellation feature we are doing in this PR? If not, we can handle it in a separate PR.
Do you consider this to be part of the pointer cancellation feature we are doing in this PR?
Yes, to me the "mouse down > mouse up" flow is not complete without item selection. I would rather merge all at once.
Do you consider this to be part of the pointer cancellation feature we are doing in this PR?
Yes, to me the "mouse down > mouse up" flow is not complete without item selection. I would rather merge all at once.
@DiegoAndai Done. Ready for further review.
Code looks good @ZeeshanTamboli!
Multiple select demos are broken though: https://deploy-preview-45789--material-ui.netlify.app/material-ui/react-select/#multiple-select
Base UI recently implemented multiple, maybe there are some clues there? https://github.com/mui/base-ui/pull/2173
Scary that these weren't caught by tests, lets add some after finding the root cause of why this PR broke the demos.
Code looks good @ZeeshanTamboli!
Multiple select demos are broken though: https://deploy-preview-45789--material-ui.netlify.app/material-ui/react-select/#multiple-select
Base UI recently implemented multiple, maybe there are some clues there? mui/base-ui#2173
@DiegoAndai Fixed now. It was occuring because both the mouseup and click events were triggering when we click on a menu item. It should trigger either mouseup or click. When clicking the item, it should trigger only click and cancel mouseup and when doing mouseup it should trigger only onMouseUp. I copied the pointer down logic from Base UI for this: https://github.com/mui/base-ui/blob/master/packages/react/src/select/item/SelectItem.tsx#L197C9-L197C26
Scary that these weren't caught by tests, lets add some after finding the root cause of why this PR broke the demos.
I think the bug wasn't caught because the tests didn't simulate both mouseup and click. However, surprisingly, there were no tests that directly checked item selection.
Thanks @ZeeshanTamboli!
think the bug wasn't caught because the tests didn't simulate both mouseup and click.
Could we use userEvent to add a test that would cover this?
cc @siriwatknp for awareness. We're close to merging this. May I ask you to review it? I want to be extra cautious with the Select as it's a priority component.
Could we use
userEventto add a test that would cover this?
@DiegoAndai Added. The test now uses userEvent.click, which covers the bug since it fires the full event sequence: mouseover → mousemove → mousedown → mouseup → click like a real user would in a browser.