Fix issues with nested modal/non-modal layers

.changeset/huge-foxes-serve.md

@@ -0,0 +1,14 @@
+---
+"@radix-ui/react-focus-scope": patch
+"@radix-ui/react-dialog": patch
+"@radix-ui/react-popover": patch
+"@radix-ui/react-menu": patch
+"@radix-ui/react-hover-card": patch
+"radix-ui": patch
+---
+
+Fixed nested, portalled layers being unusable inside a modal layer. A non-modal popover rendered inside a modal Dialog previously broke some user interactions because the modal layer's trapped `FocusScope` reclaimed focus, and its `RemoveScroll` only allowed scrolling within the modal content.
+
+`FocusScope` now accepts branches that are treated as part of the scope, and modal layers register nested portalled layers as branches so focus and scroll work as expected. This works for any depth of nesting between modal containers.
+
+See: https://github.com/radix-ui/primitives/issues/3423

apps/storybook/stories/dialog.stories.tsx

@@ -1,5 +1,5 @@
 import * as React from 'react';
-import { Dialog } from 'radix-ui';
+import { Dialog, DropdownMenu, HoverCard, Popover } from 'radix-ui';
 import styles from './dialog.stories.module.css';
 
 export default { title: 'Components/Dialog' };
@@ -536,3 +536,157 @@ export const Cypress = () => {
     </>
   );
 };
+
+/**
+ * Verification for https://github.com/radix-ui/primitives/issues/3423
+ *
+ * A `Popover` (mimicking a cmdk `Command` with an input + scrollable list) nested inside a *modal*
+ * `Dialog`. You should be able to type in the input and scroll the list with `Popover modal` either
+ * ON or OFF:
+ *
+ * - OFF (non-modal popover, the shadcn default): the popover registers its content as a branch of
+ *   the Dialog, so the Dialog's trapped `FocusScope` no longer reclaims focus and its `RemoveScroll`
+ *   treats the popover as a scroll shard.
+ * - ON (modal popover): the popover mounts its own `RemoveScroll` + trapped `FocusScope`, which
+ *   pauses the Dialog's.
+ *
+ * Before the fix, the OFF case could neither be typed in nor scrolled.
+ */
+export const PopoverInModalDialog = () => {
+  const [popoverModal, setPopoverModal] = React.useState(false);
+  return (
+    <Dialog.Root>
+      <Dialog.Trigger>open dialog</Dialog.Trigger>
+      <Dialog.Portal>
+        <Dialog.Overlay className={styles.overlay} />
+        <Dialog.Content className={styles.contentDefault}>
+          <Dialog.Title>Dialog with a nested Popover</Dialog.Title>
+          <Dialog.Description>
+            Try to type in the popover's input and scroll its list.
+          </Dialog.Description>
+
+          <label style={{ display: 'block', margin: '10px 0' }}>
+            <input
+              type="checkbox"
+              checked={popoverModal}
+              onChange={(event) => setPopoverModal(event.target.checked)}
+            />{' '}
+            Popover <code>modal</code>
+          </label>
+
+          <Popover.Root modal={popoverModal}>
+            <Popover.Trigger>open popover</Popover.Trigger>
+            <Popover.Portal>
+              <Popover.Content
+                sideOffset={5}
+                style={{
+                  background: 'white',
+                  border: '1px solid #ccc',
+                  borderRadius: 6,
+                  boxShadow: '0 2px 10px rgb(0 0 0 / 0.12)',
+                  padding: 8,
+                  width: 220,
+                }}
+              >
+                <input
+                  type="text"
+                  placeholder="type to filter…"
+                  style={{ width: '100%', boxSizing: 'border-box', marginBottom: 8 }}
+                />
+                <div style={{ maxHeight: 120, overflow: 'auto' }}>
+                  {Array.from({ length: 40 }, (_, i) => (
+                    <div key={i} style={{ padding: '4px 8px' }}>
+                      Item {i + 1}
+                    </div>
+                  ))}
+                </div>
+              </Popover.Content>
+            </Popover.Portal>
+          </Popover.Root>
+
+          <br />
+          <Dialog.Close>close</Dialog.Close>
+        </Dialog.Content>
+      </Dialog.Portal>
+    </Dialog.Root>
+  );
+};
+
+/**
+ * Verification for https://github.com/radix-ui/primitives/issues/3423 (other primitives)
+ *
+ * A *non-modal* `DropdownMenu` with a scrollable list and a `HoverCard` with a focusable link,
+ * both nested inside a *modal* `Dialog`. Before the fix, the Dialog's trapped `FocusScope` would
+ * reclaim focus from these portalled layers (and `RemoveScroll` would block scrolling). They now
+ * register their content as branches of the Dialog, so keyboard navigation, focusing the link, and
+ * scrolling all work.
+ */
+export const NestedLayersInModalDialog = () => {
+  return (
+    <Dialog.Root>
+      <Dialog.Trigger>open dialog</Dialog.Trigger>
+      <Dialog.Portal>
+        <Dialog.Overlay className={styles.overlay} />
+        <Dialog.Content className={styles.contentDefault}>
+          <Dialog.Title>Dialog with nested menu / hover card</Dialog.Title>
+          <Dialog.Description>
+            Open the menu and arrow-key through / scroll it, and hover the card to focus its link.
+          </Dialog.Description>
+
+          <div style={{ display: 'flex', gap: 12, margin: '10px 0' }}>
+            <DropdownMenu.Root modal={false}>
+              <DropdownMenu.Trigger>open menu</DropdownMenu.Trigger>
+              <DropdownMenu.Portal>
+                <DropdownMenu.Content
+                  sideOffset={5}
+                  style={{
+                    background: 'white',
+                    border: '1px solid #ccc',
+                    borderRadius: 6,
+                    boxShadow: '0 2px 10px rgb(0 0 0 / 0.12)',
+                    padding: 4,
+                    maxHeight: 160,
+                    overflow: 'auto',
+                  }}
+                >
+                  {Array.from({ length: 30 }, (_, i) => (
+                    <DropdownMenu.Item key={i} style={{ padding: '4px 8px', outline: 'none' }}>
+                      Item {i + 1}
+                    </DropdownMenu.Item>
+                  ))}
+                </DropdownMenu.Content>
+              </DropdownMenu.Portal>
+            </DropdownMenu.Root>
+
+            <HoverCard.Root openDelay={0}>
+              <HoverCard.Trigger href="#" style={{ alignSelf: 'center' }}>
+                hover me
+              </HoverCard.Trigger>
+              <HoverCard.Portal>
+                <HoverCard.Content
+                  sideOffset={5}
+                  style={{
+                    background: 'white',
+                    border: '1px solid #ccc',
+                    borderRadius: 6,
+                    boxShadow: '0 2px 10px rgb(0 0 0 / 0.12)',
+                    padding: 12,
+                    width: 220,
+                  }}
+                >
+                  This card has a{' '}
+                  <a href="https://radix-ui.com" target="_blank" rel="noreferrer">
+                    focusable link
+                  </a>
+                  .
+                </HoverCard.Content>
+              </HoverCard.Portal>
+            </HoverCard.Root>
+          </div>
+
+          <Dialog.Close>close</Dialog.Close>
+        </Dialog.Content>
+      </Dialog.Portal>
+    </Dialog.Root>
+  );
+};

packages/react/dialog/src/dialog.tsx

@@ -5,7 +5,11 @@ import { createContext, createContextScope } from '@radix-ui/react-context';
 import { useId } from '@radix-ui/react-id';
 import { useControllableState } from '@radix-ui/react-use-controllable-state';
 import { DismissableLayer } from '@radix-ui/react-dismissable-layer';
-import { FocusScope } from '@radix-ui/react-focus-scope';
+import {
+  FocusScope,
+  FocusScopeBranchProvider,
+  useFocusScopeBranchRegistry,
+} from '@radix-ui/react-focus-scope';
 import { Portal as PortalPrimitive } from '@radix-ui/react-portal';
 import { Presence } from '@radix-ui/react-presence';
 import { Primitive } from '@radix-ui/react-primitive';
@@ -15,6 +19,7 @@ import { hideOthers } from 'aria-hidden';
 import { createSlot } from '@radix-ui/react-slot';
 
 import type { Scope } from '@radix-ui/react-context';
+import type { FocusScopeBranchRegistry } from '@radix-ui/react-focus-scope';
 
 /* -------------------------------------------------------------------------------------------------
  * Dialog
@@ -35,6 +40,11 @@ type DialogContextValue = {
   onOpenChange(open: boolean): void;
   onOpenToggle(): void;
   modal: boolean;
+  // Nodes of nested, portalled layers (eg. a non-modal `Popover`) that should
+  // be treated as part of this Dialog for focus trapping and scroll locking.
+  // See https://github.com/radix-ui/primitives/issues/3423
+  branchNodes: HTMLElement[];
+  branchRegistry: FocusScopeBranchRegistry;
 };
 
 const [DialogProvider, useDialogContext] = createDialogContext<DialogContextValue>(DIALOG_NAME);
@@ -58,6 +68,7 @@ const Dialog: React.FC<DialogProps> = (props: ScopedProps<DialogProps>) => {
   } = props;
   const triggerRef = React.useRef<HTMLButtonElement>(null);
   const contentRef = React.useRef<DialogContentElement>(null);
+  const { nodes: branchNodes, registry: branchRegistry } = useFocusScopeBranchRegistry();
   const [open, setOpen] = useControllableState({
     prop: openProp,
     defaultProp: defaultOpen ?? false,
@@ -77,6 +88,8 @@ const Dialog: React.FC<DialogProps> = (props: ScopedProps<DialogProps>) => {
       onOpenChange={setOpen}
       onOpenToggle={React.useCallback(() => setOpen((prevOpen) => !prevOpen), [setOpen])}
       modal={modal}
+      branchNodes={branchNodes}
+      branchRegistry={branchRegistry}
     >
       {children}
     </DialogProvider>
@@ -201,9 +214,15 @@ const DialogOverlayImpl = React.forwardRef<DialogOverlayImplElement, DialogOverl
     const { __scopeDialog, ...overlayProps } = props;
     const context = useDialogContext(OVERLAY_NAME, __scopeDialog);
     return (
-      // Make sure `Content` is scrollable even when it doesn't live inside `RemoveScroll`
-      // ie. when `Overlay` and `Content` are siblings
-      <RemoveScroll as={Slot} allowPinchZoom shards={[context.contentRef]}>
+      // Make sure `Content` is scrollable even when it doesn't live inside
+      // `RemoveScroll` (eg. when `Overlay` and `Content` are siblings). Nested
+      // layers are registered as branches and added as shards so they remain
+      // scrollable too. See https://github.com/radix-ui/primitives/issues/3423
+      <RemoveScroll
+        as={Slot}
+        allowPinchZoom
+        shards={[context.contentRef, ...context.branchNodes.map((node) => ({ current: node }))]}
+      >
         <Primitive.div
           data-state={getState(context.open)}
           {...overlayProps}
@@ -386,6 +405,7 @@ interface DialogContentImplProps extends Omit<DismissableLayerProps, 'onDismiss'
 const DialogContentImpl = React.forwardRef<DialogContentImplElement, DialogContentImplProps>(
   (props: ScopedProps<DialogContentImplProps>, forwardedRef) => {
     const { __scopeDialog, trapFocus, onOpenAutoFocus, onCloseAutoFocus, ...contentProps } = props;
+    const { children, ...layerProps } = contentProps;
     const context = useDialogContext(CONTENT_NAME, __scopeDialog);
     const contentRef = React.useRef<HTMLDivElement>(null);
     const composedRefs = useComposedRefs(forwardedRef, contentRef);
@@ -400,6 +420,7 @@ const DialogContentImpl = React.forwardRef<DialogContentImplElement, DialogConte
           asChild
           loop
           trapped={trapFocus}
+          branches={context.branchNodes}
           onMountAutoFocus={onOpenAutoFocus}
           onUnmountAutoFocus={onCloseAutoFocus}
         >
@@ -409,10 +430,15 @@ const DialogContentImpl = React.forwardRef<DialogContentImplElement, DialogConte
             aria-describedby={context.descriptionId}
             aria-labelledby={context.titleId}
             data-state={getState(context.open)}
-            {...contentProps}
+            {...layerProps}
             ref={composedRefs}
             onDismiss={() => context.onOpenChange(false)}
-          />
+          >
+            {/* Lets nested, portalled layers register themselves as branches of this Dialog. */}
+            <FocusScopeBranchProvider value={context.branchRegistry}>
+              {children}
+            </FocusScopeBranchProvider>
+          </DismissableLayer>
         </FocusScope>
         {process.env.NODE_ENV !== 'production' && (
           <>

packages/react/focus-scope/src/focus-scope.test.tsx

@@ -92,6 +92,45 @@ describe('FocusScope', () => {
     });
   });
 
+  describe('given a trapped FocusScope with branches', () => {
+    function BranchHarness({ withBranch }: { withBranch: boolean }) {
+      const [branch, setBranch] = React.useState<HTMLElement | null>(null);
+      return (
+        <div>
+          <FocusScope asChild loop trapped branches={withBranch && branch ? [branch] : []}>
+            <form>
+              <TestField label={INNER_NAME_INPUT_LABEL} />
+              <button>{INNER_SUBMIT_LABEL}</button>
+            </form>
+          </FocusScope>
+          {/* Simulates portalled content of a nested layer living outside the scope's subtree */}
+          <div ref={setBranch}>
+            <input aria-label="branch-input" />
+          </div>
+        </div>
+      );
+    }
+
+    it('reclaims focus moved into an unregistered outside node', async () => {
+      const rendered = render(<BranchHarness withBranch={false} />);
+      const inner = rendered.getByLabelText(INNER_NAME_INPUT_LABEL) as HTMLInputElement;
+      const branchInput = rendered.getByLabelText('branch-input') as HTMLInputElement;
+      inner.focus();
+      branchInput.focus();
+      await waitFor(() => expect(inner).toHaveFocus());
+      expect(branchInput).not.toHaveFocus();
+    });
+
+    it('keeps focus when it moves into a registered branch', async () => {
+      const rendered = render(<BranchHarness withBranch />);
+      const inner = rendered.getByLabelText(INNER_NAME_INPUT_LABEL) as HTMLInputElement;
+      const branchInput = rendered.getByLabelText('branch-input') as HTMLInputElement;
+      inner.focus();
+      branchInput.focus();
+      await waitFor(() => expect(branchInput).toHaveFocus());
+    });
+  });
+
   describe('given a FocusScope with internal focus handlers', () => {
     const handleLastFocusableElementBlur = vi.fn();
     let rendered: RenderResult;