fix(container-menu): ensure outside clicks always close dropdown menu (#699)

Author: ze-flo

packages/menu/src/MenuContainer.spec.tsx

@@ -149,7 +149,7 @@ describe('MenuContainer', () => {
     return (
       <MenuContainer
         {...props}
-        items={items}
+        items={[...items]} // create a new reference on each render to simulate cases similar to Garden Menu
         onChange={handleChange}
         triggerRef={triggerRef}
         menuRef={menuRef}
@@ -911,6 +911,29 @@ describe('MenuContainer', () => {
 
         expect(getByText('Previous')).toHaveFocus();
       });
+
+      it('closes menu and returns focus to trigger when clicking on document.body after navigating to nested menu', async () => {
+        const { getByText, getByTestId } = render(<TestMenuNested />);
+        const trigger = getByTestId('trigger');
+        const menu = getByTestId('menu');
+
+        trigger.focus();
+
+        await waitFor(async () => {
+          await user.keyboard('{ArrowDown}');
+          await user.keyboard('{Enter}');
+        });
+
+        expect(getByText('Previous')).toHaveFocus();
+        expect(menu).toBeVisible();
+
+        await waitFor(async () => {
+          await user.click(document.body);
+        });
+
+        expect(trigger).toHaveFocus();
+        expect(menu).not.toBeVisible();
+      });
     });
   });
 

packages/menu/src/useMenu.ts

@@ -409,8 +409,8 @@ export const useMenu = <T extends HTMLElement = HTMLElement, M extends HTMLEleme
     (event: React.FocusEvent) => {
       const win = environment || window;
 
-      setTimeout(() => {
-        // Timeout is required to ensure blur is handled after focus
+      // Timeout is required to ensure blur is handled after focus
+      const timeoutId = setTimeout(() => {
         const activeElement = win.document.activeElement;
         const isMenuOrTriggerFocused =
           menuRef.current?.contains(activeElement) || triggerRef.current?.contains(activeElement);
@@ -427,6 +427,10 @@ export const useMenu = <T extends HTMLElement = HTMLElement, M extends HTMLEleme
 
           closeMenu(StateChangeTypes.MenuBlur);
         }
+
+        return () => {
+          clearTimeout(timeoutId);
+        };
       });
     },
     [closeMenu, controlledIsExpanded, environment, menuRef, returnFocusToTrigger, triggerRef]
@@ -598,16 +602,16 @@ export const useMenu = <T extends HTMLElement = HTMLElement, M extends HTMLEleme
       }
     },
     [
+      environment,
+      getNextFocusedValue,
       getSelectedItems,
       isExpandedControlled,
+      isFocusedValueControlled,
       isSelectedItemsControlled,
+      onChange,
       returnFocusToTrigger,
-      environment,
       rtl,
-      getNextFocusedValue,
-      isFocusedValueControlled,
-      state.nestedPathIds,
-      onChange
+      state.nestedPathIds
     ]
   );
 
@@ -674,16 +678,19 @@ export const useMenu = <T extends HTMLElement = HTMLElement, M extends HTMLEleme
         ref = itemRefs[values[0]].current;
       }
 
-      ref && ref.focus();
+      if (ref) {
+        ref.focus();
+
+        dispatch({ type: StateChangeTypes.FnInternalUpdate, payload: { focusOnOpen: false } });
+      }
     }
   }, [
     values,
     menuVisible,
     itemRefs,
     controlledFocusedValue,
     state.focusOnOpen,
-    controlledIsExpanded,
-    triggerRef
+    controlledIsExpanded
   ]);
 
   /**
@@ -694,7 +701,7 @@ export const useMenu = <T extends HTMLElement = HTMLElement, M extends HTMLEleme
 
     if (valuesChanged && !state.isTransitionNext && !state.isTransitionPrevious) {
       dispatch({
-        type: StateChangeTypes.FnSetStateRefs,
+        type: StateChangeTypes.FnInternalUpdate,
         payload: { valuesRef: values }
       });
     }

packages/menu/src/utils.ts

@@ -26,7 +26,7 @@ export const triggerLink = (element: HTMLAnchorElement, view: Window) => {
 };
 
 export const StateChangeTypes: Record<string, string> = {
-  FnSetStateRefs: 'fn:setStateRefs',
+  FnInternalUpdate: 'fn:internalUpdate',
   FnMenuTransitionFinish: 'fn:menuTransitionFinish',
   TriggerClick: 'trigger:click',
   TriggerKeyDownEnter: `trigger:keyDown:${KEYS.ENTER}`,
@@ -98,12 +98,6 @@ type ReducerAction = {
 export const stateReducer: Reducer<ReducerState, ReducerAction> = (state, action) => {
   let changes: ReducerState | null = null;
 
-  // Reset `focusOnOpen` if it was previously set to true; this prevents
-  // forced focus on the initial item in future state updates.
-  if (state.focusOnOpen) {
-    changes = { ...state, focusOnOpen: false };
-  }
-
   switch (action.type) {
     case StateChangeTypes.MenuBlur:
     case StateChangeTypes.MenuKeyDownEscape:
@@ -118,7 +112,7 @@ export const stateReducer: Reducer<ReducerState, ReducerAction> = (state, action
 
       if (stateChanges) {
         changes = {
-          ...(changes || state),
+          ...state,
           ...stateChanges
         };
       }
@@ -150,7 +144,7 @@ export const stateReducer: Reducer<ReducerState, ReducerAction> = (state, action
 
       if (stateChanges) {
         changes = {
-          ...(changes || state),
+          ...state,
           ...stateChanges
         };
       }
@@ -183,7 +177,7 @@ export const stateReducer: Reducer<ReducerState, ReducerAction> = (state, action
 
       if (stateChanges) {
         changes = {
-          ...(changes || state),
+          ...state,
           ...stateChanges
         };
       }
@@ -197,7 +191,7 @@ export const stateReducer: Reducer<ReducerState, ReducerAction> = (state, action
 
       if (stateChanges) {
         changes = {
-          ...(changes || state),
+          ...state,
           ...stateChanges,
           transitionType: null,
           isTransitionNext: false,
@@ -208,10 +202,10 @@ export const stateReducer: Reducer<ReducerState, ReducerAction> = (state, action
       break;
     }
 
-    case StateChangeTypes.FnSetStateRefs: {
+    case StateChangeTypes.FnInternalUpdate: {
       const { ...props } = action.payload;
 
-      changes = { ...(changes || state), ...props };
+      changes = { ...state, ...props };
 
       break;
     }