fix(menu): Safari issue on closing menu clicking on trigger (#2338)

* fix(menu): add fix for closing menu clicking on trigger on Safari

* docs: add comment on mouseUp code

* refactor(menu): use ref to check if MouseDown instead of manually focusing

* docs(menu): improve comment about issue on safari

* fix(popover): apply fix on popover component

* chore: add changeset

.changeset/old-owls-lick.md

@@ -0,0 +1,6 @@
+---
+"@contentful/f36-menu": patch
+"@contentful/f36-popover": patch
+---
+
+fix(menu): Safari issue on closing menu clicking on trigger

packages/components/menu/src/Menu.tsx

@@ -150,12 +150,25 @@ export function Menu(props: MenuProps) {
     [closeAndFocusTrigger, handleArrowsKeyDown],
   );
 
+  // Safari has an issue with the relatedTarget that we use on the onBlur for menuListProps,
+  // which was causing the menu to close and reopen when clicking on the trigger.
+  // We will use the isMouseDown to prevent triggering blur in the cases where the user clicks on the trigger.
+  const isMouseDown = useRef<Boolean>(false);
+
   const contextValue: MenuContextType = useMemo(
     () => ({
       isOpen,
       menuId,
       focusMenuItem,
       getTriggerProps: (_props = {}, _ref = null) => ({
+        onMouseDown: (event) => {
+          isMouseDown.current = true;
+          _props.onMouseDown?.(event);
+        },
+        onMouseUp: (event) => {
+          isMouseDown.current = false;
+          _props.onMouseUp?.(event);
+        },
         onClick: (event) => {
           // if the user made component controlled by providing isOpen prop
           // but onOpen callback is not provided, we won't add toggle logic
@@ -194,7 +207,8 @@ export function Menu(props: MenuProps) {
             menuListRef.current?.contains(relatedTarget);
           const targetIsTrigger =
             triggerRef.current === relatedTarget ||
-            triggerRef.current?.contains(relatedTarget);
+            triggerRef.current?.contains(relatedTarget) ||
+            isMouseDown.current;
           const targetIsSubmenu =
             relatedTarget?.parentElement?.dataset.parentMenu === menuId;
 

packages/components/popover/src/Popover.tsx

@@ -1,4 +1,10 @@
-import React, { useMemo, useState, useEffect, useCallback } from 'react';
+import React, {
+  useMemo,
+  useState,
+  useEffect,
+  useCallback,
+  useRef,
+} from 'react';
 import { useId, mergeRefs, type ExpandProps } from '@contentful/f36-core';
 import type { Placement, Modifier } from '@popperjs/core';
 import { PopoverContextProvider, PopoverContextType } from './PopoverContext';
@@ -186,12 +192,25 @@ export function Popover(props: ExpandProps<PopoverProps>) {
     setTimeout(() => triggerElement?.focus({ preventScroll: true }), 0);
   }, [onClose, triggerElement]);
 
+  // Safari has an issue with the relatedTarget that we use on the onBlur for getPopoverProps,
+  // which was causing the popover to close and reopen when clicking on the trigger.
+  // We will use the isMouseDown to prevent triggering blur in the cases where the user clicks on the trigger.
+  const isMouseDown = useRef<Boolean>(false);
+
   const contextValue: PopoverContextType = useMemo(
     () => ({
       isOpen: Boolean(isOpen),
       usePortal,
       renderOnlyWhenOpen,
-      getTriggerProps: (_ref = null) => ({
+      getTriggerProps: (_props = {}, _ref = null) => ({
+        onMouseDown: (event) => {
+          isMouseDown.current = true;
+          _props.onMouseDown?.(event);
+        },
+        onMouseUp: (event) => {
+          isMouseDown.current = false;
+          _props.onMouseUp?.(event);
+        },
         ref: mergeRefs(setTriggerElement, _ref),
         ['aria-expanded']: Boolean(isOpen),
         ['aria-controls']: popoverId,
@@ -220,7 +239,8 @@ export function Popover(props: ExpandProps<PopoverProps>) {
             popoverElement?.contains(relatedTarget);
           const targetIsTrigger =
             triggerElement === relatedTarget ||
-            triggerElement?.contains(relatedTarget);
+            triggerElement?.contains(relatedTarget) ||
+            isMouseDown.current;
 
           if (targetIsPopover || targetIsTrigger) {
             return;

packages/components/popover/src/PopoverContext.ts

@@ -8,7 +8,10 @@ export type PopoverContextType = {
     _props: HTMLProps<HTMLDivElement>,
     _ref: React.Ref<HTMLDivElement>,
   ) => HTMLProps<HTMLDivElement>;
-  getTriggerProps: (_ref: React.Ref<HTMLElement>) => HTMLProps<HTMLElement>;
+  getTriggerProps: (
+    _props: HTMLProps<HTMLElement>,
+    _ref: React.Ref<HTMLElement>,
+  ) => HTMLProps<HTMLElement>;
 };
 
 const PopoverContext = React.createContext<PopoverContextType | undefined>(

packages/components/popover/src/PopoverTrigger/PopoverTrigger.tsx

@@ -13,7 +13,7 @@ export const PopoverTrigger = (props: PopoverTriggerProps) => {
   const { getTriggerProps } = usePopoverContext();
 
   return React.cloneElement(child, {
-    ...getTriggerProps(child.ref),
+    ...getTriggerProps(child.props, child.ref),
     'aria-haspopup': child.props['aria-haspopup'] ?? 'dialog',
   });
 };