fix(core): one focus return for Popover, Dialog and DropdownMenu that never strands focus (AST-116 FR17–FR21)

.changeset/layer-focus-return.md

@@ -0,0 +1,7 @@
+---
+'@astryxdesign/core': patch
+---
+
+[fix] Popover, Dialog and DropdownMenu return focus through one rule when they close, and focus no longer falls to the page when the control that opened them is gone. Focus goes to the opener; if the opener is gone, it goes to its replacement (a trigger that remounted, or the control that took its place in the same group), then the anchor, then the nearest surviving ancestor, then the layer that opened this one, then the page's `main` region. A Dialog opened from a menu row returns focus to the menu's trigger. Focus that you or the person already moved outside the closing layer stays where it is. There is no option to turn focus return off; to send focus somewhere specific, focus it in `onOpenChange` or `onHide`. Focus returns before the browser's own restore, so the trigger gets a focus ring only after keyboard input.
+
+@nynexman4464

packages/core/src/Dialog/Dialog.doc.mjs

@@ -143,6 +143,11 @@ export const docs = {
     description:
       'Dialog displays a modal overlay that blocks interaction with the page until the user responds. Use it for delete confirmations, edit forms, terms acceptance, or any decision that should not be skipped.\n\nFor cases where you want to show a dialog without managing open state, use the `useImperativeDialog` hook: call `dialog.show(content)` and render `dialog.element` in your tree.',
     bestPractices: [
+      {
+        guidance: false,
+        description:
+          "Don't add your own focus return or try to turn it off. When the dialog closes, focus goes back to the opener, or to the nearest control that stands in for it (a dialog opened from a menu row returns focus to the menu's trigger), never to the page. To send focus somewhere specific, focus that element in onOpenChange.",
+      },
       {
         guidance: true,
         description:
@@ -214,6 +219,11 @@ export const docsZh = {
     description:
       'Dialog displays a modal overlay that blocks interaction with the page until the user responds. Use it for delete confirmations, edit forms, terms acceptance, or any decision that should not be skipped.',
     bestPractices: [
+      {
+        guidance: false,
+        description:
+          '不要自行实现或关闭焦点返回。对话框关闭时，焦点会回到打开它的元素，或回到最近的替代控件（从菜单项打开的对话框会把焦点还给菜单的触发器），绝不会落到页面上。若要把焦点移到特定位置，请在 onOpenChange 中聚焦该元素。',
+      },
       {
         guidance: true,
         description:

packages/core/src/Dialog/Dialog.test.tsx

@@ -1173,6 +1173,10 @@ describe('Dialog', () => {
         fireEvent.pointerDown(document.body);
         opener.focus();
         rerender(<SecondDocumentDialog isOpen={true} />);
+        // A modal dialog holds focus while it is open, as showModal() makes it.
+        within(secondDocument.body)
+          .getByRole('button', {name: 'Inside', hidden: true})
+          .focus();
         fireEvent.keyDown(secondDocument.body, {key: 'Escape'});
         const focus = vi.spyOn(
           secondDocument.defaultView!.HTMLElement.prototype,

packages/core/src/Dialog/Dialog.tsx

@@ -89,11 +89,17 @@ import type {DialogVariantMap} from './index';
 import {focusOutlineProps} from '../utils/focusOutline.stylex';
 import {useInteractionModalityTracking} from '../utils/interactionModality';
 import {
+  cancelLayerFocus,
   focusForLastInput,
-  prepareFocusReturnOnRemoval,
-  returnFocusForLastInput,
+  focusForLastInputTemporarily,
+  isLayerFocusStranded,
+  prepareFocusRepairOnRemoval,
+  restoreLayerFocus,
+  trackLayerFocus,
+  type LayerFocusRecord,
 } from '../utils/focusReturn';
 import {ModalAnnounceProvider} from '../hooks/ModalAnnounceProvider';
+import {isHTMLElement} from '../utils/nodeType';
 
 import {useMergedRefs} from '../hooks/useMergedRefs';
 
@@ -603,6 +609,8 @@ export function Dialog({
   // Capture the element that was focused when the dialog opened, for
   // directional animation origin and focus restoration on close.
   const triggerElementRef = useRef<HTMLElement | null>(null);
+  // The shared layer focus return for the current opening.
+  const focusRecordRef = useRef<LayerFocusRecord | null>(null);
 
   // This runs in a layout effect so the capture lands before any descendant's
   // passive mount effect: all layout effects run before any passive effect,
@@ -630,6 +638,20 @@ export function Dialog({
       const dialogDocument = dialogRef.current?.ownerDocument ?? document;
       const doc = dialogDocument.hasFocus() ? dialogDocument : document;
       triggerElementRef.current = doc.activeElement as HTMLElement | null;
+      // The shared layer focus return starts here, with the same opener.
+      if (focusRecordRef.current != null) {
+        cancelLayerFocus(focusRecordRef.current);
+      }
+      const opener = triggerElementRef.current;
+      focusRecordRef.current = trackLayerFocus({
+        getLayer: () => dialogRef.current,
+        opener: isHTMLElement(opener) ? opener : null,
+        doc,
+        isFocusMovedInOnOpen: true,
+        // The ring follows the last input in the opener's own document.
+        focus: element =>
+          focusForLastInputTemporarily(element, {preventScroll: true}),
+      });
     }
     wasOpenRef.current = isOpenNow;
   }, [isOpen, isInline]);
@@ -703,14 +725,20 @@ export function Dialog({
       // unmount.
       return holdEntranceInput(dialog) ?? undefined;
     } else {
+      // The page stays inert until `close()`, so focus can return only after
+      // it. Whether focus would be stranded is read first. The browser's own
+      // restore reaches only an opener that is still there, with a ring
+      // whatever the last input was; the shared return then picks the opener
+      // or its nearest stand-in, with the ring the last input calls for.
+      const record = focusRecordRef.current;
+      focusRecordRef.current = null;
+      const wasStranded = record != null && isLayerFocusStranded(record);
       if (dialog.open) {
         dialog.close();
       }
-      // Return focus to the element that opened the dialog, with the ring
-      // the last input in that element's own document calls for.
-      const trigger = triggerElementRef.current;
-      if (trigger) {
-        returnFocusForLastInput(trigger);
+      if (record != null) {
+        restoreLayerFocus(record, {wasStranded});
+        cancelLayerFocus(record);
       }
       triggerElementRef.current = null;
     }
@@ -724,8 +752,23 @@ export function Dialog({
     if (isInline || dialog == null) {
       return;
     }
-    const returnFocusOnRemoval = prepareFocusReturnOnRemoval(dialog);
-    return () => returnFocusOnRemoval(triggerElementRef.current);
+    const repairOnRemoval = prepareFocusRepairOnRemoval(dialog);
+    return () => {
+      const record = focusRecordRef.current;
+      if (record == null) {
+        return;
+      }
+      repairOnRemoval(() => {
+        restoreLayerFocus(record, {warnsWhenNothingQualifies: false});
+      });
+      // After the repair's own check: a dialog StrictMode only pretended to
+      // unmount is still in the document and keeps its record.
+      queueMicrotask(() => {
+        if (!dialog.isConnected) {
+          cancelLayerFocus(record);
+        }
+      });
+    };
   }, [isInline]);
 
   // Host app-global surfaces (toasts) inside this modal while it is open, so

packages/core/src/DropdownMenu/DropdownMenu.test.tsx

@@ -3985,37 +3985,37 @@ describe('DropdownMenu focus return after a pointer pick', () => {
       expect(focus.mock.calls.at(-1)).toEqual([{preventScroll: true}]);
     });
 
-    it('clears a ring the browser already drew before refocusing after a pointer pick', async () => {
+    it('returns focus without a ring after a pointer pick, before the browser can draw one', async () => {
       const user = userEvent.setup();
       const trigger = renderCustomMenu();
       await user.click(trigger);
-      // WebKit's own popover focus restoration can focus the trigger, with a
-      // ring, before the menu moves focus, and focusing the focused element
-      // changes nothing.
+      // The browser's own popover focus restoration focuses the trigger, with
+      // a ring, while focus is still inside the closing popover. The menu
+      // returns focus as it starts to close, so that restore has nothing to do.
       const hidePopover = HTMLElement.prototype.hidePopover;
       HTMLElement.prototype.hidePopover = vi.fn(function (this: HTMLElement) {
+        const hadFocus = this.contains(document.activeElement);
         hidePopover.call(this);
-        trigger.focus();
+        if (hadFocus) {
+          trigger.focus();
+        }
       });
       const matches = HTMLElement.prototype.matches;
       vi.spyOn(HTMLElement.prototype, 'matches').mockImplementation(function (
         this: HTMLElement,
         selector: string,
       ) {
         return selector === ':focus-visible'
-          ? this === trigger
+          ? this === trigger && document.activeElement === trigger
           : matches.call(this, selector);
       });
       const blur = vi.spyOn(trigger, 'blur');
       const focus = vi.spyOn(trigger, 'focus');
       await user.click(item('Edit'));
-      expect(blur).toHaveBeenCalledTimes(1);
+      expect(blur).not.toHaveBeenCalled();
       expect(focus.mock.calls.at(-1)).toEqual([
         {preventScroll: true, focusVisible: false},
       ]);
-      expect(blur.mock.invocationCallOrder[0]).toBeLessThan(
-        focus.mock.invocationCallOrder.at(-1) ?? 0,
-      );
       expect(trigger).toHaveFocus();
     });
   });

packages/core/src/DropdownMenu/DropdownMenu.tsx

@@ -47,7 +47,7 @@ import React, {
 } from 'react';
 import {readAccessibleName} from '../utils/accessibleName';
 import * as stylex from '@stylexjs/stylex';
-import {usePopover} from '../Popover/usePopover';
+import {usePopoverInternal} from '../Popover/usePopover';
 import {holdEntranceInput} from '../Layer/holdEntranceInput';
 import {
   claimPress,
@@ -1033,27 +1033,21 @@ function DropdownMenuPopover({
     if (!isControlled) {
       setInternalIsOpen(false);
     }
-    const trigger = buttonRef.current;
-    if (!trigger) {
-      return;
-    }
-    const active = document.activeElement;
-    const focusLandedElsewhere =
-      active instanceof HTMLElement &&
-      active !== document.body &&
-      active !== trigger &&
-      !contentRef.current?.contains(active);
-    if (focusLandedElsewhere) {
-      return;
-    }
-    prepareFocusReturn();
-    // After pointer input the trigger takes focus without the focus-visible
-    // ring (`component:DropdownMenu/DEC-1`). The style the helper above adds
-    // reaches only the design system's own Button; the browser's focus option
-    // also reaches a control the caller renders, which the browser would
-    // otherwise ring (Chromium after a mouse pick, WebKit after a touch).
-    returnFocusForLastInput(trigger);
-  }, [hideOpenFlyouts, isControlled, onOpenChange, prepareFocusReturn]);
+  }, [hideOpenFlyouts, isControlled, onOpenChange]);
+
+  // How returned focus moves. After pointer input the trigger takes focus
+  // without a ring. The helper's style reaches only the design system's own
+  // Button. The browser's focus option also reaches a control the caller
+  // renders, which Chromium rings after a mouse pick and WebKit after a touch.
+  const restoreFocusTo = useCallback(
+    (element: HTMLElement) => {
+      if (element === buttonRef.current) {
+        prepareFocusReturn();
+      }
+      returnFocusForLastInput(element);
+    },
+    [prepareFocusReturn],
+  );
 
   // Defer item focus until the layer has committed open, so focus restore
   // captures the trigger instead of the first menu item.
@@ -1108,7 +1102,8 @@ function DropdownMenuPopover({
         ? button.label
         : undefined;
 
-  const popover = usePopover({
+  const popover = usePopoverInternal({
+    restoreFocusTo,
     onHide: handleLayerHide,
     onShow: handleLayerShow,
     hasLightDismiss: true,

packages/core/src/Popover/Popover.doc.mjs

@@ -242,6 +242,11 @@ export const docs = {
     description:
       'A click-triggered overlay anchored to a button or trigger element. Use it for secondary actions, inline confirmations, or supplementary information that does not warrant a full dialog. For hover previews use HoverCard, for brief helper text use Tooltip. Under a finger (a coarse primary pointer), it is presented like a native popover: it grows out of its trigger and shrinks back into it on the theme\u2019s motion tokens; with reduced motion it cross-fades, opacity only, in 200 ms or less.',
     bestPractices: [
+      {
+        guidance: false,
+        description:
+          "Don't add your own focus return or try to turn it off. When the popover closes with focus inside it, focus goes back to the opener, or to the nearest control that stands in for it, never to the page. To send focus somewhere specific, focus that element in onOpenChange.",
+      },
       {
         guidance: true,
         description:
@@ -479,6 +484,11 @@ export const docsZh = {
     description:
       'A click-triggered overlay anchored to a button or trigger element. Use it for secondary actions, inline confirmations, or supplementary information that does not warrant a full dialog. For hover previews use HoverCard, for brief helper text use Tooltip. Under a finger (a coarse primary pointer), it is presented like a native popover: it grows out of its trigger and shrinks back into it on the theme\u2019s motion tokens; with reduced motion it cross-fades, opacity only, in 200 ms or less.',
     bestPractices: [
+      {
+        guidance: false,
+        description:
+          '不要自行实现或关闭焦点返回。弹出框在焦点位于其内部时关闭，焦点会回到打开它的元素，或回到最近的替代控件，绝不会落到页面上。若要把焦点移到特定位置，请在 onOpenChange 中聚焦该元素。',
+      },
       {
         guidance: true,
         description:

packages/core/src/Popover/usePopover.tsx

@@ -567,6 +567,10 @@ function usePopoverImplementation(
 
   // Track the trigger element for returning focus
   const triggerElementRef = useRef<HTMLElement | null>(null);
+  // The caller's `anchor`, and the element it is bound to now.
+  const anchorOptionRef = useRef(anchor);
+  anchorOptionRef.current = anchor;
+  const boundAnchorRef = useRef<Element | null>(null);
 
   // Track whether to skip auto-focus for the current open event
   const skipAutoFocusRef = useRef(false);
@@ -585,12 +589,24 @@ function usePopoverImplementation(
   // A caller that may cancel an outside dismissal gets the outside-press
   // watch in place of native light dismiss, which no script can cancel.
   const watchesOutsidePress = hasLightDismiss && onOutsidePress != null;
+  // Focus returns as the popover starts to close, while it still holds focus:
+  // the browser restores focus only for an auto popover whose opener is still
+  // there, and then with a focus ring whatever the last input was. The trap
+  // below supplies the return; it is read through a ref because the layer
+  // exists first.
+  const returnFocusRef = useRef<() => void>(() => {});
+  const onCloseStartRef = useRef(onCloseStart);
+  onCloseStartRef.current = onCloseStart;
+  const handleCloseStart = useCallback(() => {
+    returnFocusRef.current();
+    onCloseStartRef.current?.();
+  }, []);
   const layer = useLayerInternal({
     mode: 'context',
     lightDismiss: hasLightDismiss && !watchesOutsidePress,
     onShow: handleLayerShow,
     onHide,
-    onCloseStart,
+    onCloseStart: handleCloseStart,
     hidesWithAnchor: anchorOutOfView === 'hide',
   });
 
@@ -603,14 +619,22 @@ function usePopoverImplementation(
   const getOpenerRef = useRef(getOpener);
   getOpenerRef.current = getOpener;
   const getRestoreTarget = useCallback(() => openerRef.current, []);
-  const {containerRef: contentRef, focusFirst} =
-    useFocusTrapInternal<HTMLDivElement>({
-      isActive: layer.isOpen,
-      onEscape: hasEscapeDismiss || hasLightDismiss ? layer.hide : undefined,
-      getRestoreTarget,
-      getRestoreFallback,
-      restoreFocusTo,
-    });
+  const {
+    containerRef: contentRef,
+    focusFirst,
+    returnFocus,
+  } = useFocusTrapInternal<HTMLDivElement>({
+    isActive: layer.isOpen,
+    onEscape: hasEscapeDismiss || hasLightDismiss ? layer.hide : undefined,
+    getRestoreTarget,
+    getRestoreFallback,
+    restoreFocusTo,
+    getTrigger: () => triggerElementRef.current,
+    getAnchor: () =>
+      anchorOptionRef.current == null ? null : boundAnchorRef.current,
+    layerId: layer.id,
+  });
+  returnFocusRef.current = returnFocus;
 
   const focusInitialTarget = useCallback(() => {
     const container = contentRef.current;
@@ -667,10 +691,7 @@ function usePopoverImplementation(
   const layerRef = layer.ref;
   // With a separate `anchor`, the trigger keeps its role but is not the
   // layer's anchor: the element `anchor` resolves to is.
-  const anchorOptionRef = useRef(anchor);
-  anchorOptionRef.current = anchor;
   const hasSeparateAnchor = anchor != null;
-  const boundAnchorRef = useRef<Element | null>(null);
   const isAnchorBoundRef = useRef(false);
   const [isAnchorMissing, setIsAnchorMissing] = useState(false);
   const triggerRef = useCallback(