fix(popover): popover focus issue (#3187)

* fix(popover): move useDialog to popover-content

* fix(popover): move useDialog to free-solo-popover

* refactor(popover): use const instead

* feat(changset): add changeset

* feat(popover): popover focus test

* refactor(popover): getDialogProps

.changeset/clever-gifts-joke.md

@@ -0,0 +1,5 @@
+---
+"@nextui-org/popover": patch
+---
+
+Fix popover focus issue (#3171, #2992)

packages/components/popover/__tests__/popover.test.tsx

@@ -213,4 +213,63 @@ describe("Popover", () => {
     // assert that the second popover is open
     expect(popover2).toHaveAttribute("aria-expanded", "true");
   });
+
+  it("should focus on dialog when opened", async () => {
+    const wrapper = render(
+      <Popover>
+        <PopoverTrigger>
+          <Button disableRipple data-testid="trigger-test">
+            Open popover
+          </Button>
+        </PopoverTrigger>
+        <PopoverContent>
+          <p>This is the content of the popover.</p>
+        </PopoverContent>
+      </Popover>,
+    );
+
+    const trigger = wrapper.getByTestId("trigger-test");
+
+    // open popover
+    await act(async () => {
+      await userEvent.click(trigger);
+    });
+
+    const {getByRole} = wrapper;
+
+    let dialog = getByRole("dialog");
+
+    // assert that the focus is on the dialog
+    expect(dialog).toHaveFocus();
+  });
+
+  it("should restore focus on trigger when closed", async () => {
+    const wrapper = render(
+      <Popover>
+        <PopoverTrigger>
+          <Button disableRipple data-testid="trigger-test">
+            Open popover
+          </Button>
+        </PopoverTrigger>
+        <PopoverContent>
+          <p>This is the content of the popover.</p>
+        </PopoverContent>
+      </Popover>,
+    );
+
+    const trigger = wrapper.getByTestId("trigger-test");
+
+    // open popover
+    await act(async () => {
+      await userEvent.click(trigger);
+    });
+
+    // close popover
+    await act(async () => {
+      await userEvent.click(trigger);
+    });
+
+    // assert that the focus is restored back to trigger
+    expect(trigger).toHaveFocus();
+  });
 });

packages/components/popover/src/free-solo-popover.tsx

@@ -14,6 +14,7 @@ import {domAnimation, HTMLMotionProps, LazyMotion, m} from "framer-motion";
 import {mergeProps} from "@react-aria/utils";
 import {getTransformOrigins} from "@nextui-org/aria-utils";
 import {TRANSITION_VARIANTS} from "@nextui-org/framer-utils";
+import {useDialog} from "@react-aria/dialog";
 
 import {usePopover, UsePopoverProps, UsePopoverReturn} from "./use-popover";
 
@@ -92,7 +93,6 @@ const FreeSoloPopover = forwardRef<"div", FreeSoloPopoverProps>(
       state,
       placement,
       backdrop,
-      titleProps,
       portalContainer,
       disableAnimation,
       motionProps,
@@ -106,6 +106,13 @@ const FreeSoloPopover = forwardRef<"div", FreeSoloPopoverProps>(
       ref,
     });
 
+    const dialogRef = React.useRef(null);
+    const {dialogProps: ariaDialogProps, titleProps} = useDialog({}, dialogRef);
+    const dialogProps = getDialogProps({
+      ref: dialogRef,
+      ...ariaDialogProps,
+    });
+
     const backdropContent = React.useMemo(() => {
       if (backdrop === "transparent") {
         return null;
@@ -138,7 +145,7 @@ const FreeSoloPopover = forwardRef<"div", FreeSoloPopoverProps>(
             placement={placement}
             tabIndex={-1}
             transformOrigin={transformOrigin}
-            {...getDialogProps()}
+            {...dialogProps}
           >
             {!isNonModal && <DismissButton onDismiss={state.close} />}
             <div {...getContentProps()}>

packages/components/popover/src/popover-content.tsx

@@ -1,14 +1,15 @@
 import type {AriaDialogProps} from "@react-aria/dialog";
 import type {HTMLMotionProps} from "framer-motion";
 
-import {DOMAttributes, ReactNode, useMemo, useCallback, ReactElement} from "react";
+import {DOMAttributes, ReactNode, useMemo, useCallback, ReactElement, useRef} from "react";
 import {forwardRef} from "@nextui-org/system";
 import {DismissButton} from "@react-aria/overlays";
 import {TRANSITION_VARIANTS} from "@nextui-org/framer-utils";
 import {m, domAnimation, LazyMotion} from "framer-motion";
 import {HTMLNextUIProps} from "@nextui-org/system";
 import {RemoveScroll} from "react-remove-scroll";
 import {getTransformOrigins} from "@nextui-org/aria-utils";
+import {useDialog} from "@react-aria/dialog";
 
 import {usePopoverContext} from "./popover-context";
 
@@ -27,7 +28,6 @@ const PopoverContent = forwardRef<"div", PopoverContentProps>((props, _) => {
     placement,
     backdrop,
     motionProps,
-    titleProps,
     disableAnimation,
     shouldBlockScroll,
     getPopoverProps,
@@ -38,10 +38,13 @@ const PopoverContent = forwardRef<"div", PopoverContentProps>((props, _) => {
     onClose,
   } = usePopoverContext();
 
-  const dialogProps = getDialogProps(otherProps);
-
-  // Not needed in the popover context, the popover role comes from getPopoverProps
-  delete dialogProps.role;
+  const dialogRef = useRef(null);
+  const {dialogProps: ariaDialogProps, titleProps} = useDialog({}, dialogRef);
+  const dialogProps = getDialogProps({
+    ref: dialogRef,
+    ...ariaDialogProps,
+    ...otherProps,
+  });
 
   const Component = as || OverlayComponent || "div";
 

packages/components/popover/src/use-popover.ts

@@ -19,7 +19,7 @@ import {popover} from "@nextui-org/theme";
 import {mergeProps, mergeRefs} from "@react-aria/utils";
 import {clsx, dataAttr, objectToDeps} from "@nextui-org/shared-utils";
 import {useMemo, useCallback, useRef} from "react";
-import {AriaDialogProps, useDialog} from "@react-aria/dialog";
+import {AriaDialogProps} from "@react-aria/dialog";
 
 import {useReactAriaPopover, ReactAriaPopoverProps} from "./use-aria-popover";
 
@@ -131,7 +131,6 @@ export function usePopover(originalProps: UsePopoverProps) {
 
   const domTriggerRef = useRef<HTMLElement>(null);
   const wasTriggerPressedRef = useRef(false);
-  const dialogRef = useRef(null);
   const triggerRef = triggerRefProp || domTriggerRef;
 
   const disableAnimation =
@@ -179,8 +178,6 @@ export function usePopover(originalProps: UsePopoverProps) {
 
   const {isFocusVisible, isFocused, focusProps} = useFocusRing();
 
-  const {dialogProps, titleProps} = useDialog({}, dialogRef);
-
   const slots = useMemo(
     () =>
       popover({
@@ -198,14 +195,15 @@ export function usePopover(originalProps: UsePopoverProps) {
   });
 
   const getDialogProps: PropGetter = (props = {}) => ({
-    ref: dialogRef,
+    // `ref` and `dialogProps` from `useDialog` are passed from props
+    // if we use `useDialog` here, dialogRef won't be focused on mount
     "data-slot": "base",
     "data-open": dataAttr(state.isOpen),
     "data-focus": dataAttr(isFocused),
     "data-arrow": dataAttr(showArrow),
     "data-focus-visible": dataAttr(isFocusVisible),
     "data-placement": getArrowPlacement(ariaPlacement, placementProp),
-    ...mergeProps(focusProps, dialogProps, dialogPropsProp, props),
+    ...mergeProps(focusProps, dialogPropsProp, props),
     className: slots.base({class: clsx(baseStyles)}),
     style: {
       // this prevent the dialog to have a default outline
@@ -316,7 +314,6 @@ export function usePopover(originalProps: UsePopoverProps) {
     triggerRef,
     placement,
     isNonModal,
-    titleProps,
     popoverRef: domRef,
     portalContainer,
     isOpen: state.isOpen,