good to go

packages/material-ui/src/Menu/Menu.js

@@ -143,7 +143,6 @@ const Menu = React.forwardRef(function Menu(props, ref) {
         },
       }}
       open={open}
-      disableAutoFocus={!autoFocus}
       ref={ref}
       transitionDuration={transitionDuration}
       TransitionProps={{ onEntering: handleEntering, ...TransitionProps }}

packages/material-ui/src/Unstable_TrapFocus/Unstable_TrapFocus.d.ts

@@ -16,7 +16,7 @@ export interface TrapFocusProps {
    * For instance, you can provide the "tabbable" npm dependency.
    * @param {HTMLElement} root
    */
-  getTabbable: (root: HTMLElement) => string[];
+  getTabbable?: (root: HTMLElement) => string[];
   /**
    * Do we still want to enforce the focus?
    * This prop helps nesting TrapFocus elements.

packages/material-ui/src/Unstable_TrapFocus/Unstable_TrapFocus.js

@@ -68,7 +68,7 @@ function isFocusable(node) {
   return true;
 }
 
-function defaultTabbable(root) {
+export function defaultGetTabbable(root) {
   const regularTabNodes = [];
   const orderedTabNodes = [];
 
@@ -108,45 +108,29 @@ function Unstable_TrapFocus(props) {
     disableEnforceFocus = false,
     disableRestoreFocus = false,
     getDoc,
-    getTabbable = defaultTabbable,
+    getTabbable = defaultGetTabbable,
     isEnabled,
     open,
   } = props;
   const ignoreNextEnforceFocus = React.useRef();
-  const lastEvent = React.useRef(null);
   const sentinelStart = React.useRef(null);
   const sentinelEnd = React.useRef(null);
   const nodeToRestore = React.useRef();
   const reactFocusEventTarget = React.useRef(null);
   // This variable is useful when disableAutoFocus is true.
   // It waits for the active element to move into the component to activate.
   const activated = React.useRef(false);
+
   const rootRef = React.useRef(null);
   const handleRef = useForkRef(children.ref, rootRef);
+  const lastKeydown = React.useRef(null);
 
   const prevOpenRef = React.useRef();
   React.useEffect(() => {
-    const doc = ownerDocument(rootRef.current);
-
-    if (
-      !prevOpenRef.current &&
-      open &&
-      rootRef.current &&
-      ((rootRef.current.contains(doc.activeElement) && disableAutoFocus) || !disableAutoFocus) &&
-      !ignoreNextEnforceFocus.current
-    ) {
-      if (!nodeToRestore.current) {
-        nodeToRestore.current = doc.activeElement;
-      }
-
-      activated.current = true;
-      onSentinelFocus('start')();
-    }
-
     prevOpenRef.current = open;
-  }, [disableAutoFocus, open]);
+  }, [open]);
 
-  if (!prevOpenRef.current && open && typeof window !== 'undefined' && !disableRestoreFocus) {
+  if (!prevOpenRef.current && open && typeof window !== 'undefined' && !disableAutoFocus) {
     // WARNING: Potentially unsafe in concurrent mode.
     // The way the read on `nodeToRestore` is setup could make this actually safe.
     // Say we render `open={false}` -> `open={true}` but never commit.
@@ -189,8 +173,13 @@ function Unstable_TrapFocus(props) {
         rootRef.current.setAttribute('tabIndex', -1);
       }
 
-      if (activated.current && !ignoreNextEnforceFocus.current) {
-        rootRef.current.focus();
+      if (activated.current) {
+        const tabbable = getTabbable(rootRef.current);
+        if (tabbable.length > 0) {
+          tabbable[0].focus();
+        } else {
+          rootRef.current.focus();
+        }
       }
     }
 
@@ -214,28 +203,6 @@ function Unstable_TrapFocus(props) {
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [open]);
 
-  const onSentinelFocus = React.useCallback(
-    (position) => () => {
-      activated.current = true;
-      const tabbable = getTabbable(rootRef.current);
-
-      if (tabbable.length === 0) {
-        return rootRef.current.focus();
-      }
-
-      const shiftTab = Boolean(lastEvent.current?.shiftKey && lastEvent.current?.key === 'Tab');
-
-      if (position === 'start' && shiftTab) {
-        const focusEnd = tabbable[tabbable.length - 1];
-        return focusEnd.focus();
-      }
-
-      const focusStart = tabbable[0];
-      return focusStart.focus();
-    },
-    [getTabbable],
-  );
-
   React.useEffect(() => {
     // We might render an empty child.
     if (!open || !rootRef.current) {
@@ -277,29 +244,41 @@ function Unstable_TrapFocus(props) {
           return;
         }
 
-        rootElement.focus();
-      } else {
-        activated.current = true;
+        const tabbable = getTabbable(rootRef.current);
+
+        if (tabbable.length > 0) {
+          const isShiftTab = Boolean(
+            lastKeydown.current?.shiftKey && lastKeydown.current?.key === 'Tab',
+          );
+
+          const focusNext = tabbable[0];
+          const focusPrevious = tabbable[tabbable.length - 1];
+
+          if (isShiftTab) {
+            focusPrevious.focus();
+          } else {
+            focusNext.focus();
+          }
+        } else {
+          rootElement.focus();
+        }
       }
     };
 
     const loopFocus = (nativeEvent) => {
-      lastEvent.current = nativeEvent;
+      lastKeydown.current = nativeEvent;
 
       if (disableEnforceFocus || !isEnabled() || nativeEvent.key !== 'Tab') {
         return;
       }
 
       // Make sure the next tab starts from the right place.
-      if (doc.activeElement === rootRef.current) {
+      // doc.activeElement referes to the origin.
+      if (doc.activeElement === rootRef.current && nativeEvent.shiftKey) {
         // We need to ignore the next contain as
         // it will try to move the focus back to the rootRef element.
         ignoreNextEnforceFocus.current = true;
-        if (nativeEvent.shiftKey) {
-          sentinelEnd.current.focus();
-        } else {
-          sentinelStart.current.focus();
-        }
+        sentinelEnd.current.focus();
       }
     };
 
@@ -327,17 +306,9 @@ function Unstable_TrapFocus(props) {
   }, [disableAutoFocus, disableEnforceFocus, disableRestoreFocus, isEnabled, open]);
 
   const onFocus = (event) => {
-    if (
-      !activated.current &&
-      rootRef.current &&
-      event.relatedTarget &&
-      !rootRef.current.contains(event.relatedTarget) &&
-      event.relatedTarget !== sentinelStart.current &&
-      event.relatedTarget !== sentinelEnd.current
-    ) {
+    if (!activated.current) {
       nodeToRestore.current = event.relatedTarget;
     }
-
     activated.current = true;
     reactFocusEventTarget.current = event.target;
 
@@ -347,21 +318,23 @@ function Unstable_TrapFocus(props) {
     }
   };
 
+  const handleFocusSentinel = (event) => {
+    if (!activated.current) {
+      nodeToRestore.current = event.relatedTarget;
+    }
+    activated.current = true;
+  };
+
   return (
     <React.Fragment>
       <div
-        onFocus={onSentinelFocus('start')}
         tabIndex={0}
+        onFocus={handleFocusSentinel}
         ref={sentinelStart}
         data-test="sentinelStart"
       />
       {React.cloneElement(children, { ref: handleRef, onFocus })}
-      <div
-        onFocus={onSentinelFocus('end')}
-        tabIndex={0}
-        ref={sentinelEnd}
-        data-test="sentinelEnd"
-      />
+      <div tabIndex={0} onFocus={handleFocusSentinel} ref={sentinelEnd} data-test="sentinelEnd" />
     </React.Fragment>
   );
 }
@@ -409,7 +382,7 @@ Unstable_TrapFocus.propTypes = {
    * For instance, you can provide the "tabbable" npm dependency.
    * @param {HTMLElement} root
    */
-  getTabbable: PropTypes.func.isRequired,
+  getTabbable: PropTypes.func,
   /**
    * Do we still want to enforce the focus?
    * This prop helps nesting TrapFocus elements.

packages/material-ui/src/Unstable_TrapFocus/Unstable_TrapFocus.test.js

@@ -2,7 +2,7 @@ import * as React from 'react';
 import * as ReactDOM from 'react-dom';
 import { useFakeTimers } from 'sinon';
 import { expect } from 'chai';
-import { act, createClientRender, fireEvent, screen, userEvent } from 'test/utils';
+import { act, createClientRender, screen, userEvent } from 'test/utils';
 import TrapFocus from './Unstable_TrapFocus';
 import Portal from '../Portal';
 
@@ -38,7 +38,7 @@ describe('<TrapFocus />', () => {
     expect(getByTestId('auto-focus')).toHaveFocus();
 
     initialFocus.focus();
-    expect(getByTestId('modal')).toHaveFocus();
+    expect(getByTestId('auto-focus')).toHaveFocus();
   });
 
   it('should not return focus to the children when disableEnforceFocus is true', () => {
@@ -110,14 +110,13 @@ describe('<TrapFocus />', () => {
     expect(screen.getByText('cancel')).toHaveFocus();
     userEvent.tab();
     expect(screen.getByText('ok')).toHaveFocus();
+    userEvent.tab();
+    expect(screen.getByText('x')).toHaveFocus();
 
     initialFocus.focus();
-    fireEvent.keyDown(screen.getByTestId('modal'), {
-      key: 'Tab',
-      shiftKey: true,
-    });
-
     expect(screen.getByText('x')).toHaveFocus();
+    userEvent.tab({ shift: true });
+    expect(screen.getByText('ok')).toHaveFocus();
   });
 
   it('should focus on first focus element after last has received a tab click', () => {
@@ -148,13 +147,7 @@ describe('<TrapFocus />', () => {
         </div>
       </TrapFocus>,
     );
-
-    const modalEl = screen.getByTestId('modal');
-    fireEvent.keyDown(modalEl, {
-      key: 'Tab',
-    });
-
-    expect(modalEl).toHaveFocus();
+    expect(screen.getByTestId('modal')).toHaveFocus();
   });
 
   it('should focus checked radio button if present in radio group', () => {
@@ -413,7 +406,7 @@ describe('<TrapFocus />', () => {
       });
 
       it('should trap once the focus moves inside', () => {
-        const { getByRole, getByTestId } = render(
+        const { getByRole } = render(
           <div>
             <input />
             <TrapFocus {...defaultProps} open disableAutoFocus>
@@ -426,13 +419,16 @@ describe('<TrapFocus />', () => {
 
         expect(initialFocus).toHaveFocus();
 
+        userEvent.tab();
+        expect(getByRole('textbox')).toHaveFocus();
+
         // the trap activates
-        getByTestId('modal').focus();
-        expect(getByTestId('modal')).toHaveFocus();
+        userEvent.tab();
+        expect(screen.getByTestId('focus-input')).toHaveFocus();
 
         // the trap prevent to escape
         getByRole('textbox').focus();
-        expect(screen.getByTestId('modal')).toHaveFocus();
+        expect(screen.getByTestId('focus-input')).toHaveFocus();
       });
 
       it('should restore the focus', () => {

test/utils/user-event/index.js

@@ -1,4 +1,6 @@
 import { fireEvent, getConfig } from '@testing-library/dom';
+// eslint-disable-next-line no-restricted-imports
+import { defaultGetTabbable as getTabbable } from '@material-ui/core/Unstable_TrapFocus/Unstable_TrapFocus';
 
 // Absolutely NO events fire on label elements that contain their control
 // if that control is disabled. NUTS!
@@ -81,56 +83,14 @@ function tab({ shift = false, focusTrap } = {}) {
     focusTrap = document;
   }
 
-  const focusableElements = focusTrap.querySelectorAll(FOCUSABLE_SELECTOR);
+  const tabbable = getTabbable(focusTrap);
 
-  const enabledElements = [...focusableElements].filter(
-    (el) => el.getAttribute('tabindex') !== '-1' && !el.disabled,
-  );
-
-  if (enabledElements.length === 0) {
+  if (tabbable.length === 0) {
     return;
   }
 
-  const orderedElements = enabledElements
-    .map((el, idx) => ({ el, idx }))
-    .sort((a, b) => {
-      const tabIndexA = a.el.getAttribute('tabindex');
-      const tabIndexB = b.el.getAttribute('tabindex');
-
-      const diff = tabIndexA - tabIndexB;
-
-      return diff === 0 ? a.idx - b.idx : diff;
-    })
-    .map(({ el }) => el);
-
-  if (shift) {
-    orderedElements.reverse();
-  }
-
-  // keep only the checked or first element in each radio group
-  const prunedElements = [];
-  orderedElements.forEach((el) => {
-    if (el.type === 'radio' && el.name) {
-      const replacedIndex = prunedElements.findIndex(({ name }) => name === el.name);
-
-      if (replacedIndex === -1) {
-        prunedElements.push(el);
-      } else if (el.checked) {
-        prunedElements.splice(replacedIndex, 1);
-        prunedElements.push(el);
-      }
-    } else {
-      prunedElements.push(el);
-    }
-  });
-
-  if (shift) {
-    prunedElements.reverse();
-  }
-
-  const index = prunedElements.findIndex((el) => el === el.ownerDocument.activeElement);
-
-  const nextElement = getNextElement(index, shift, prunedElements, focusTrap);
+  const index = tabbable.findIndex((el) => el === el.ownerDocument.activeElement);
+  const nextElement = getNextElement(index, shift, tabbable, focusTrap);
 
   const shiftKeyInit = {
     key: 'Shift',