From 2874b926c35898a45a70224e6fc03210e1e74fa2 Mon Sep 17 00:00:00 2001 From: Kartik_Murthy Date: Wed, 2 Apr 2025 11:47:04 +0530 Subject: [PATCH 1/4] fix(Select): resolve focus, drag, pointer issues - Fixed onFocus being triggered excessively (#44505) - Enabled drag-and-release to select items (#45374) - Addressed pointer cancellation (WCAG 2.5.2) failure (#45301) --- .../mui-material/src/Select/SelectInput.js | 134 ++++++++++++++---- 1 file changed, 110 insertions(+), 24 deletions(-) diff --git a/packages/mui-material/src/Select/SelectInput.js b/packages/mui-material/src/Select/SelectInput.js index 58255517fe8eac..1e625dc4a6fbfc 100644 --- a/packages/mui-material/src/Select/SelectInput.js +++ b/packages/mui-material/src/Select/SelectInput.js @@ -1,20 +1,20 @@ 'use client'; -import * as React from 'react'; -import { isFragment } from 'react-is'; -import PropTypes from 'prop-types'; -import clsx from 'clsx'; import composeClasses from '@mui/utils/composeClasses'; -import useId from '@mui/utils/useId'; import refType from '@mui/utils/refType'; -import ownerDocument from '../utils/ownerDocument'; -import capitalize from '../utils/capitalize'; -import Menu from '../Menu/Menu'; -import { StyledSelectSelect, StyledSelectIcon } from '../NativeSelect/NativeSelectInput'; +import useId from '@mui/utils/useId'; +import clsx from 'clsx'; +import PropTypes from 'prop-types'; +import * as React from 'react'; +import { isFragment } from 'react-is'; import { isFilled } from '../InputBase/utils'; -import { styled } from '../zero-styled'; +import Menu from '../Menu/Menu'; +import { StyledSelectIcon, StyledSelectSelect } from '../NativeSelect/NativeSelectInput'; import slotShouldForwardProp from '../styles/slotShouldForwardProp'; -import useForkRef from '../utils/useForkRef'; +import capitalize from '../utils/capitalize'; +import ownerDocument from '../utils/ownerDocument'; import useControlled from '../utils/useControlled'; +import useForkRef from '../utils/useForkRef'; +import { styled } from '../zero-styled'; import selectClasses, { getSelectUtilityClasses } from './selectClasses'; const SelectSelect = styled(StyledSelectSelect, { @@ -162,6 +162,17 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) { const anchorElement = displayNode?.parentNode; + const [isPointerDown, setIsPointerDown] = React.useState(false); + const dragSelectRef = React.useRef({ + isDragging: false, + startedOn: null, + }); + + const focusTrackingRef = React.useRef({ + isFocused: false, + pendingBlur: false, + }); + React.useImperativeHandle( handleRef, () => ({ @@ -210,20 +221,23 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) { return undefined; }, [labelId]); - const update = (open, event) => { - if (open) { - if (onOpen) { - onOpen(event); + const update = React.useCallback( + (open, event) => { + if (open) { + if (onOpen) { + onOpen(event); + } + } else if (onClose) { + onClose(event); } - } else if (onClose) { - onClose(event); - } - if (!isOpenControlled) { - setMenuMinWidthState(autoWidth ? null : anchorElement.clientWidth); - setOpenState(open); - } - }; + if (!isOpenControlled) { + setMenuMinWidthState(autoWidth ? null : anchorElement.clientWidth); + setOpenState(open); + } + }, + [autoWidth, anchorElement, isOpenControlled, onOpen, onClose, setOpenState], + ); const handleMouseDown = (event) => { // Ignore everything but left-click @@ -234,6 +248,11 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) { event.preventDefault(); displayRef.current.focus(); + // Mark that we've initiated a pointer interaction + setIsPointerDown(true); + dragSelectRef.current.startedOn = displayRef.current; + + // Open the menu immediately update(true, event); }; @@ -258,6 +277,59 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) { } }; + React.useEffect(() => { + if (isPointerDown) { + const doc = ownerDocument(displayRef.current); + + const handleGlobalMouseUp = (event) => { + setIsPointerDown(false); + + if (dragSelectRef.current.isDragging) { + // If we're dragging and the mouse is released, check where it was released + dragSelectRef.current.isDragging = false; + + // Check if mouse is over a menu item + const targetElement = event.target; + let menuItem = null; + + // Find if we released on a menu item (checking up the parent chain) + let current = targetElement; + while (current && !menuItem) { + // Check if this element has role="option" + if (current.getAttribute && current.getAttribute('role') === 'option') { + menuItem = current; + } + current = current.parentElement; + } + + if (menuItem) { + // Simulate a click on the menu item if we released on one + menuItem.click(); + } else { + // If released outside menu items, close the menu + update(false, event); + } + } + }; + + const handleGlobalMouseMove = () => { + if (isPointerDown) { + dragSelectRef.current.isDragging = true; + } + }; + + doc.addEventListener('mouseup', handleGlobalMouseUp); + doc.addEventListener('mousemove', handleGlobalMouseMove); + + return () => { + doc.removeEventListener('mouseup', handleGlobalMouseUp); + doc.removeEventListener('mousemove', handleGlobalMouseMove); + }; + } + + return undefined; + }, [isPointerDown, update]); + const handleItemClick = (child) => (event) => { let newValue; @@ -326,9 +398,23 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) { const open = displayNode !== null && openState; + const handleFocus = (event) => { + // Skip duplicate focus events + if (!focusTrackingRef.current.isFocused) { + focusTrackingRef.current.isFocused = true; + focusTrackingRef.current.pendingBlur = false; + + if (onFocus) { + onFocus(event); + } + } + }; + const handleBlur = (event) => { // if open event.stopImmediatePropagation if (!open && onBlur) { + focusTrackingRef.current.pendingBlur = false; + focusTrackingRef.current.isFocused = false; // Preact support, target is read only property on a native event. Object.defineProperty(event, 'target', { writable: true, value: { value, name } }); onBlur(event); @@ -509,7 +595,7 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) { onKeyDown={handleKeyDown} onMouseDown={disabled || readOnly ? null : handleMouseDown} onBlur={handleBlur} - onFocus={onFocus} + onFocus={handleFocus} {...SelectDisplayProps} ownerState={ownerState} className={clsx(SelectDisplayProps.className, classes.select, className)} From a757802cd1feb62f19364672f44ac71daf43cb71 Mon Sep 17 00:00:00 2001 From: Kartik_Murthy Date: Wed, 2 Apr 2025 22:24:24 +0530 Subject: [PATCH 2/4] fix: revert import order - Restore original import order as per project conventions --- .../mui-material/src/Select/SelectInput.js | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/mui-material/src/Select/SelectInput.js b/packages/mui-material/src/Select/SelectInput.js index 1e625dc4a6fbfc..04840b2996e951 100644 --- a/packages/mui-material/src/Select/SelectInput.js +++ b/packages/mui-material/src/Select/SelectInput.js @@ -1,20 +1,20 @@ 'use client'; -import composeClasses from '@mui/utils/composeClasses'; -import refType from '@mui/utils/refType'; -import useId from '@mui/utils/useId'; -import clsx from 'clsx'; -import PropTypes from 'prop-types'; import * as React from 'react'; import { isFragment } from 'react-is'; -import { isFilled } from '../InputBase/utils'; +import PropTypes from 'prop-types'; +import clsx from 'clsx'; +import composeClasses from '@mui/utils/composeClasses'; +import useId from '@mui/utils/useId'; +import refType from '@mui/utils/refType'; +import ownerDocument from '../utils/ownerDocument'; +import capitalize from '../utils/capitalize'; import Menu from '../Menu/Menu'; -import { StyledSelectIcon, StyledSelectSelect } from '../NativeSelect/NativeSelectInput'; +import { StyledSelectSelect, StyledSelectIcon } from '../NativeSelect/NativeSelectInput'; +import { isFilled } from '../InputBase/utils'; +import { styled } from '../zero-styled'; import slotShouldForwardProp from '../styles/slotShouldForwardProp'; -import capitalize from '../utils/capitalize'; -import ownerDocument from '../utils/ownerDocument'; -import useControlled from '../utils/useControlled'; import useForkRef from '../utils/useForkRef'; -import { styled } from '../zero-styled'; +import useControlled from '../utils/useControlled'; import selectClasses, { getSelectUtilityClasses } from './selectClasses'; const SelectSelect = styled(StyledSelectSelect, { From 8f7e2820fde1c2385397c70f68686a63a557f1f4 Mon Sep 17 00:00:00 2001 From: Kartik_Murthy Date: Tue, 15 Apr 2025 22:49:00 +0530 Subject: [PATCH 3/4] fix(Select): resolve pointer cancellation issues for WCAG 2.5.2 compliance (#45301) --- .../mui-material/src/Select/Select.test.js | 48 +++++++++++++++++++ .../mui-material/src/Select/SelectInput.js | 26 +--------- 2 files changed, 50 insertions(+), 24 deletions(-) diff --git a/packages/mui-material/src/Select/Select.test.js b/packages/mui-material/src/Select/Select.test.js index 4599548c848d92..c3818ad13c4d93 100644 --- a/packages/mui-material/src/Select/Select.test.js +++ b/packages/mui-material/src/Select/Select.test.js @@ -33,6 +33,54 @@ describe(' + none + Ten + , + ); + const trigger = getByRole('combobox'); + + // Open the menu with left mouse button + fireEvent.mouseDown(trigger, { button: 0 }); + expect(getByRole('listbox')).not.to.equal(null); + + // Simulate mouse move to initiate drag + fireEvent.mouseMove(document.body); + + // Simulate mouse up outside any menu items + fireEvent.mouseUp(document.body); + + // Menu should be closed now + expect(queryByRole('listbox', { hidden: false })).to.equal(null); + }); + + it('should not close the menu when releasing on a menu item after dragging', () => { + const { getByRole, getAllByRole } = render( + , + ); + const trigger = getByRole('combobox'); + + // Open the menu + fireEvent.mouseDown(trigger, { button: 0 }); + const options = getAllByRole('option'); + + // Simulate mouse move to initiate drag + fireEvent.mouseMove(document.body); + + // Simulate mouse up on a menu item + fireEvent.mouseUp(options[0]); + + // Menu should still be open + expect(getByRole('listbox')).not.to.equal(null); + }); + }); + describe('prop: inputProps', () => { it('should be able to provide a custom classes property', () => { render( diff --git a/packages/mui-material/src/Select/SelectInput.js b/packages/mui-material/src/Select/SelectInput.js index 04840b2996e951..fb9e7bfb3e2ce2 100644 --- a/packages/mui-material/src/Select/SelectInput.js +++ b/packages/mui-material/src/Select/SelectInput.js @@ -168,11 +168,6 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) { startedOn: null, }); - const focusTrackingRef = React.useRef({ - isFocused: false, - pendingBlur: false, - }); - React.useImperativeHandle( handleRef, () => ({ @@ -302,10 +297,7 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) { current = current.parentElement; } - if (menuItem) { - // Simulate a click on the menu item if we released on one - menuItem.click(); - } else { + if (!menuItem) { // If released outside menu items, close the menu update(false, event); } @@ -398,23 +390,9 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) { const open = displayNode !== null && openState; - const handleFocus = (event) => { - // Skip duplicate focus events - if (!focusTrackingRef.current.isFocused) { - focusTrackingRef.current.isFocused = true; - focusTrackingRef.current.pendingBlur = false; - - if (onFocus) { - onFocus(event); - } - } - }; - const handleBlur = (event) => { // if open event.stopImmediatePropagation if (!open && onBlur) { - focusTrackingRef.current.pendingBlur = false; - focusTrackingRef.current.isFocused = false; // Preact support, target is read only property on a native event. Object.defineProperty(event, 'target', { writable: true, value: { value, name } }); onBlur(event); @@ -595,7 +573,7 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) { onKeyDown={handleKeyDown} onMouseDown={disabled || readOnly ? null : handleMouseDown} onBlur={handleBlur} - onFocus={handleFocus} + onFocus={onFocus} {...SelectDisplayProps} ownerState={ownerState} className={clsx(SelectDisplayProps.className, classes.select, className)} From d91d5ba05335fe61f8299200c4c57255dcd2fc8b Mon Sep 17 00:00:00 2001 From: Kartik_Murthy Date: Wed, 16 Apr 2025 20:20:30 +0530 Subject: [PATCH 4/4] chore(Select): remove unused startedOn property from dragSelectRef --- packages/mui-material/src/Select/SelectInput.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/mui-material/src/Select/SelectInput.js b/packages/mui-material/src/Select/SelectInput.js index fb9e7bfb3e2ce2..7fe7f28933eb86 100644 --- a/packages/mui-material/src/Select/SelectInput.js +++ b/packages/mui-material/src/Select/SelectInput.js @@ -165,7 +165,6 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) { const [isPointerDown, setIsPointerDown] = React.useState(false); const dragSelectRef = React.useRef({ isDragging: false, - startedOn: null, }); React.useImperativeHandle( @@ -245,7 +244,6 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) { // Mark that we've initiated a pointer interaction setIsPointerDown(true); - dragSelectRef.current.startedOn = displayRef.current; // Open the menu immediately update(true, event);