[Select] Fix focus visible always set on menu item (@silviuaavram)

packages/mui-material/src/MenuItem/MenuItem.js

@@ -15,6 +15,25 @@ import { dividerClasses } from '../Divider';
 import { listItemIconClasses } from '../ListItemIcon';
 import { listItemTextClasses } from '../ListItemText';
 import menuItemClasses, { getMenuItemUtilityClass } from './menuItemClasses';
+import { useSelectFocusSource } from '../Select';
+
+/**
+ * If autoFocus is an object, it will attempt to call `element.focus()` with the options argument.
+ * If the browser doesn't support the options argument, it will fall back to a simple `element.focus()` call.
+ */
+function focusWithVisible(element, focusSource) {
+  if (focusSource == null) {
+    element.focus();
+    return;
+  }
+
+  try {
+    element.focus({ focusVisible: focusSource === 'keyboard' });
+  } catch (error) {
+    // If the browser doesn't support the focus options argument, fall back to a simple focus call.
+    element.focus();
+  }
+}
 
 export const overridesResolver = (props, styles) => {
   const { ownerState } = props;
@@ -176,6 +195,7 @@ const MenuItem = React.forwardRef(function MenuItem(inProps, ref) {
     ...other
   } = props;
 
+  const focusSource = useSelectFocusSource();
   const context = React.useContext(ListContext);
   const childContext = React.useMemo(
     () => ({
@@ -189,13 +209,14 @@ const MenuItem = React.forwardRef(function MenuItem(inProps, ref) {
   useEnhancedEffect(() => {
     if (autoFocus) {
       if (menuItemRef.current) {
-        menuItemRef.current.focus();
+        focusWithVisible(menuItemRef.current, focusSource);
       } else if (process.env.NODE_ENV !== 'production') {
         console.error(
           'MUI: Unable to set focus to a MenuItem whose component has not been rendered.',
         );
       }
     }
+    // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [autoFocus]);
 
   const ownerState = {

packages/mui-material/src/Select/SelectInput.js

@@ -16,6 +16,8 @@ import slotShouldForwardProp from '../styles/slotShouldForwardProp';
 import useForkRef from '../utils/useForkRef';
 import useControlled from '../utils/useControlled';
 import selectClasses, { getSelectUtilityClasses } from './selectClasses';
+import { areEqualValues, isEmpty, getOpenInteractionType } from './utils';
+import { SelectFocusSourceProvider } from './utils/SelectFocusSourceContext';
 
 const SelectSelect = styled(StyledSelectSelect, {
   name: 'MuiSelect',
@@ -68,19 +70,6 @@ const SelectNativeInput = styled('input', {
   boxSizing: 'border-box',
 });
 
-function areEqualValues(a, b) {
-  if (typeof b === 'object' && b !== null) {
-    return a === b;
-  }
-
-  // The value could be a number, the DOM will stringify it anyway.
-  return String(a) === String(b);
-}
-
-function isEmpty(display) {
-  return display == null || (typeof display === 'string' && !display.trim());
-}
-
 const useUtilityClasses = (ownerState) => {
   const { classes, variant, disabled, multiple, open, error } = ownerState;
 
@@ -153,7 +142,7 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) {
   const [displayNode, setDisplayNode] = React.useState(null);
   const { current: isOpenControlled } = React.useRef(openProp != null);
   const [menuMinWidthState, setMenuMinWidthState] = React.useState();
-
+  const [openInteractionType, setOpenInteractionType] = React.useState(null);
   const handleRef = useForkRef(ref, inputRefProp);
 
   const handleDisplayRef = React.useCallback((node) => {
@@ -238,11 +227,17 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) {
 
   const update = (openParam, event) => {
     if (openParam) {
+      setOpenInteractionType(getOpenInteractionType(event));
+
       if (onOpen) {
         onOpen(event);
       }
-    } else if (onClose) {
-      onClose(event);
+    } else {
+      setOpenInteractionType(null);
+
+      if (onClose) {
+        onClose(event);
+      }
     }
 
     if (!isOpenControlled) {
@@ -577,41 +572,43 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) {
         ownerState={ownerState}
       />
       <SelectIcon as={IconComponent} className={classes.icon} ownerState={ownerState} />
-      <Menu
-        id={`menu-${name || ''}`}
-        anchorEl={anchorElement}
-        open={open}
-        onClose={handleClose}
-        anchorOrigin={{
-          vertical: 'bottom',
-          horizontal: 'center',
-        }}
-        transformOrigin={{
-          vertical: 'top',
-          horizontal: 'center',
-        }}
-        {...MenuProps}
-        slotProps={{
-          ...MenuProps.slotProps,
-          list: {
-            'aria-labelledby': labelId,
-            role: 'listbox',
-            'aria-multiselectable': multiple ? 'true' : undefined,
-            disableListWrap: true,
-            id: listboxId,
-            ...listProps,
-          },
-          paper: {
-            ...paperProps,
-            style: {
-              minWidth: menuMinWidth,
-              ...(paperProps != null ? paperProps.style : null),
+      <SelectFocusSourceProvider value={openInteractionType}>
+        <Menu
+          id={`menu-${name || ''}`}
+          anchorEl={anchorElement}
+          open={open}
+          onClose={handleClose}
+          anchorOrigin={{
+            vertical: 'bottom',
+            horizontal: 'center',
+          }}
+          transformOrigin={{
+            vertical: 'top',
+            horizontal: 'center',
+          }}
+          {...MenuProps}
+          slotProps={{
+            ...MenuProps.slotProps,
+            list: {
+              'aria-labelledby': labelId,
+              role: 'listbox',
+              'aria-multiselectable': multiple ? 'true' : undefined,
+              disableListWrap: true,
+              id: listboxId,
+              ...listProps,
             },
-          },
-        }}
-      >
-        {items}
-      </Menu>
+            paper: {
+              ...paperProps,
+              style: {
+                minWidth: menuMinWidth,
+                ...(paperProps != null ? paperProps.style : null),
+              },
+            },
+          }}
+        >
+          {items}
+        </Menu>
+      </SelectFocusSourceProvider>
     </React.Fragment>
   );
 });

packages/mui-material/src/Select/index.d.ts

@@ -1,5 +1,6 @@
 export { default } from './Select';
 export * from './Select';
+export * from './utils';
 
 export { default as selectClasses } from './selectClasses';
 export * from './selectClasses';

packages/mui-material/src/Select/index.js

@@ -1,4 +1,5 @@
 export { default } from './Select';
+export * from './utils';
 
 export { default as selectClasses } from './selectClasses';
 export * from './selectClasses';

packages/mui-material/src/Select/utils/SelectFocusSourceContext.ts

@@ -0,0 +1,18 @@
+'use client';
+import * as React from 'react';
+
+const SelectFocusSourceContext = React.createContext<'keyboard' | 'mouse' | 'touch' | null>(null);
+
+if (process.env.NODE_ENV !== 'production') {
+  SelectFocusSourceContext.displayName = 'SelectFocusSourceContext';
+}
+
+function useSelectFocusSource() {
+  const context = React.useContext(SelectFocusSourceContext);
+
+  return context;
+}
+
+const SelectFocusSourceProvider = SelectFocusSourceContext.Provider;
+
+export { useSelectFocusSource, SelectFocusSourceProvider };

packages/mui-material/src/Select/utils/areEqualValues.ts

@@ -0,0 +1,8 @@
+export default function areEqualValues(a: unknown, b: unknown): boolean {
+  if (typeof b === 'object' && b !== null) {
+    return a === b;
+  }
+
+  // The value could be a number, the DOM will stringify it anyway.
+  return String(a) === String(b);
+}

packages/mui-material/src/Select/utils/getOpenInteractionType.ts

@@ -0,0 +1,17 @@
+export default function getOpenInteractionType(
+  event: MouseEvent | KeyboardEvent | TouchEvent | PointerEvent | null,
+): 'keyboard' | 'pointer' | null {
+  if (!event) {
+    return null;
+  }
+
+  if (event.type === 'mousedown' || event.type === 'pointerdown' || event.type === 'touchstart') {
+    return 'pointer';
+  }
+
+  if (event.type === 'keydown' || (event.type === 'click' && event.detail === 0)) {
+    return 'keyboard';
+  }
+
+  return null;
+}

packages/mui-material/src/Select/utils/index.ts

@@ -0,0 +1,4 @@
+export { default as getOpenInteractionType } from './getOpenInteractionType';
+export { default as isEmpty } from './isEmpty';
+export { default as areEqualValues } from './areEqualValues';
+export { useSelectFocusSource, SelectFocusSourceProvider } from './SelectFocusSourceContext';

packages/mui-material/src/Select/utils/isEmpty.ts

@@ -0,0 +1,3 @@
+export default function isEmpty(display: unknown) {
+  return display == null || (typeof display === 'string' && !display.trim());
+}

test/README.md

@@ -158,7 +158,7 @@ When running this command you should get under `coverage/index.html` a full cove
 
 ### DOM API level
 
-#### Run the browser test suit
+#### Run the browser test suite
 
 `pnpm test:browser`
 

test/e2e/fixtures/Select/SelectFocusVisible.tsx

@@ -0,0 +1,13 @@
+import * as React from 'react';
+import Select from '@mui/material/Select';
+import MenuItem from '@mui/material/MenuItem';
+
+export default function SelectFocusVisible() {
+  return (
+    <Select defaultValue={10} data-testid="select">
+      <MenuItem value={10}>Ten</MenuItem>
+      <MenuItem value={20}>Twenty</MenuItem>
+      <MenuItem value={30}>Thirty</MenuItem>
+    </Select>
+  );
+}

test/e2e/index.test.ts

@@ -264,4 +264,40 @@ describe('e2e', () => {
       await errorSelector.waitFor();
     });
   });
+
+  describe('<Select />', () => {
+    it('should not show focus-visible on menu item when opened by mouse', async () => {
+      await renderFixture('Select/SelectFocusVisible');
+
+      const trigger = page.getByRole('combobox');
+      await trigger.click();
+
+      await page.waitForSelector('[role="listbox"]');
+
+      const selectedItem = page.locator('[role="option"][aria-selected="true"]');
+      await expect(selectedItem).toBeFocused();
+      const hasVisible = await selectedItem.evaluate((el) =>
+        el.classList.contains('Mui-focusVisible'),
+      );
+      expect(hasVisible).toEqual(false);
+    });
+
+    it('should show focus-visible on menu item when opened by keyboard', async () => {
+      await renderFixture('Select/SelectFocusVisible');
+
+      await page.keyboard.press('Tab');
+      const trigger = page.getByRole('combobox');
+      await expect(trigger).toBeFocused();
+
+      await page.keyboard.press('Enter');
+      await page.waitForSelector('[role="listbox"]');
+
+      const selectedItem = page.locator('[role="option"][aria-selected="true"]');
+      await expect(selectedItem).toBeFocused();
+      const hasVisible = await selectedItem.evaluate((el) =>
+        el.classList.contains('Mui-focusVisible'),
+      );
+      expect(hasVisible).toEqual(true);
+    });
+  });
 });

test/setupVitest.ts

@@ -1,3 +1,30 @@
+import { beforeAll, afterAll } from 'vitest';
 import setupVitest from '@mui/internal-test-utils/setupVitest';
 
 setupVitest({ emotion: true });
+
+// In Firefox, calling focus() with arguments (e.g. focusOptions) fails silently,
+// which causes focus-visible related tests to fail as a consequence.
+// This override is only applied in a browser environment running Firefox.
+if (typeof globalThis.navigator !== 'undefined' && !navigator.userAgent.includes('jsdom')) {
+  const isFirefox = navigator.userAgent.toLowerCase().includes('firefox');
+
+  if (isFirefox) {
+    const originalFocus = HTMLElement.prototype.focus;
+
+    beforeAll(() => {
+      Object.defineProperty(HTMLElement.prototype, 'focus', {
+        configurable: true,
+        value: function focusWithoutArguments() {
+          originalFocus.call(this); // always call without arguments
+        },
+      });
+    });
+
+    afterAll(() => {
+      Object.defineProperty(HTMLElement.prototype, 'focus', {
+        value: originalFocus,
+      });
+    });
+  }
+}