[Select] Retain highlight while transitioning out (#972)

mui · Dec 11, 2024 · 961523e · 961523e
1 parent fa9515b
commit 961523e
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 22 deletions.
diff --git a/packages/react/src/select/item/SelectItem.test.tsx b/packages/react/src/select/item/SelectItem.test.tsx
@@ -4,6 +4,8 @@ import { fireEvent, flushMicrotasks, screen, waitFor } from '@mui/internal-test-
 import { createRenderer, describeConformance } from '#test-utils';
 import { expect } from 'chai';
 
+const isJSDOM = /jsdom/.test(window.navigator.userAgent);
+
 describe('<Select.Item />', () => {
   const { render } = createRenderer();
 
@@ -79,7 +81,7 @@ describe('<Select.Item />', () => {
   });
 
   it('should select item when Enter key is pressed', async function test(t = {}) {
-    if (!/jsdom/.test(window.navigator.userAgent)) {
+    if (!isJSDOM) {
       // @ts-expect-error to support mocha and vitest
       // eslint-disable-next-line @typescript-eslint/no-unused-expressions
       this?.skip?.() || t?.skip();
@@ -102,10 +104,11 @@ describe('<Select.Item />', () => {
     const trigger = screen.getByTestId('trigger');
     const value = screen.getByTestId('value');
 
-    fireEvent.click(trigger);
+    await user.click(trigger);
 
     await flushMicrotasks();
 
+    await user.keyboard('{ArrowDown}');
     await user.keyboard('{ArrowDown}');
     await user.keyboard('{Enter}');
 
@@ -165,23 +168,19 @@ describe('<Select.Item />', () => {
 
     await flushMicrotasks();
 
-    await user.keyboard('{ArrowDown}');
-    await user.keyboard('{ArrowUp}');
-    await user.keyboard('{ArrowUp}');
-
     await user.click(screen.getByRole('option', { name: 'three' }));
     await user.click(trigger);
 
     await flushMicrotasks();
 
     await waitFor(() => {
-      expect(screen.getByRole('option', { name: 'three', hidden: false })).toHaveFocus();
+      expect(screen.getByRole('option', { name: 'three' })).toHaveFocus();
     });
   });
 
   describe('style hooks', () => {
     it('should apply data-highlighted attribute when item is highlighted', async function test(t = {}) {
-      if (!/jsdom/.test(window.navigator.userAgent)) {
+      if (!isJSDOM) {
         // @ts-expect-error to support mocha and vitest
         // eslint-disable-next-line @typescript-eslint/no-unused-expressions
         this?.skip?.() || t?.skip();

diff --git a/packages/react/src/select/item/useSelectItem.ts b/packages/react/src/select/item/useSelectItem.ts
@@ -33,17 +33,6 @@ export function useSelectItem(params: useSelectItem.Parameters): useSelectItem.R
 
   const mergedRef = useForkRef(externalRef, ref);
 
-  // Manually set the tabindex.
-  // Workaround `enableFocusInside` in Floating UI setting `tabindex=0` of a non-highlighted
-  // option upon close when tabbing out: https://github.com/floating-ui/floating-ui/pull/3004/files#diff-962a7439cdeb09ea98d4b622a45d517bce07ad8c3f866e089bda05f4b0bbd875R194-R199
-  React.useEffect(() => {
-    if (!ref.current) {
-      return;
-    }
-
-    ref.current.setAttribute('tabindex', highlighted || !open ? '0' : '-1');
-  }, [open, highlighted]);
-
   const { getButtonProps, buttonRef } = useButton({
     disabled,
     focusableWhenDisabled: true,
@@ -66,6 +55,7 @@ export function useSelectItem(params: useSelectItem.Parameters): useSelectItem.R
       return getButtonProps(
         mergeReactProps<'div'>(externalProps, {
           'aria-disabled': disabled || undefined,
+          tabIndex: highlighted ? 0 : -1,
           style: {
             pointerEvents: disabled ? 'none' : undefined,
           },
@@ -82,7 +72,7 @@ export function useSelectItem(params: useSelectItem.Parameters): useSelectItem.R
           },
           onMouseLeave() {
             const popup = popupRef.current;
-            if (!popup) {
+            if (!popup || !open) {
               return;
             }
 
@@ -157,6 +147,7 @@ export function useSelectItem(params: useSelectItem.Parameters): useSelectItem.R
       getButtonProps,
       highlighted,
       indexRef,
+      open,
       popupRef,
       selected,
       selectionRef,

diff --git a/packages/react/src/select/root/useSelectRoot.ts b/packages/react/src/select/root/useSelectRoot.ts
@@ -92,12 +92,28 @@ export function useSelectRoot<T>(params: useSelectRoot.Parameters<T>): useSelect
   const setOpen = useEventCallback((nextOpen: boolean, event?: Event) => {
     params.onOpenChange?.(nextOpen, event);
     setOpenUnwrapped(nextOpen);
+
+    // Workaround `enableFocusInside` in Floating UI setting `tabindex=0` of a non-highlighted
+    // option upon close when tabbing out due to `keepMounted=true`:
+    // https://github.com/floating-ui/floating-ui/pull/3004/files#diff-962a7439cdeb09ea98d4b622a45d517bce07ad8c3f866e089bda05f4b0bbd875R194-R199
+    // This otherwise causes options to retain `tabindex=0` incorrectly when the popup is closed
+    // when tabbing outside.
+    if (!nextOpen && activeIndex !== null) {
+      const activeOption = listRef.current[activeIndex];
+      // Wait for Floating UI's focus effect to have fired
+      queueMicrotask(() => {
+        activeOption?.setAttribute('tabindex', '-1');
+      });
+    }
   });
 
   useAfterExitAnimation({
     open,
     animatedElementRef: popupRef,
-    onFinished: () => setMounted(false),
+    onFinished() {
+      setMounted(false);
+      setActiveIndex(null);
+    },
   });
 
   const setValue = useEventCallback((nextValue: any, event?: Event) => {
@@ -155,7 +171,14 @@ export function useSelectRoot<T>(params: useSelectRoot.Parameters<T>): useSelect
     listRef,
     activeIndex,
     selectedIndex,
-    onNavigate: setActiveIndex,
+    onNavigate(nextActiveIndex) {
+      // Retain the highlight while transitioning out.
+      if (nextActiveIndex === null && !open) {
+        return;
+      }
+
+      setActiveIndex(nextActiveIndex);
+    },
     // Implement our own listeners since `onPointerLeave` on each option fires while scrolling with
     // the `alignItemToTrigger` prop enabled, causing a performance issue on Chrome.
     focusItemOnHover: false,