fix(Popover): better handle esc when dialog is open (#4788)

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Michael Marszalek <mimarz@gmail.com>

Author: 3 people

.changeset/strange-yaks-burn.md

@@ -0,0 +1,5 @@
+---
+"@digdir/designsystemet-react": patch
+---
+
+**Popover:** No longer closes with "Escape" key if a Dialog element is open above the popover

packages/react/src/components/dropdown/dropdown.stories.tsx

@@ -221,3 +221,51 @@ WithNestedDialog.play = async (ctx) => {
   await expect(dialog).toBeInTheDocument();
   await waitFor(() => expect(dialog).toBeVisible());
 };
+
+export const WithAdjacentDialog: StoryFn<typeof Dropdown> = (args) => {
+  return (
+    <>
+      <Dropdown.TriggerContext>
+        <Dropdown.Trigger data-color={args['data-color']}>
+          Dropdown
+        </Dropdown.Trigger>
+        <Dropdown {...args}>
+          <Dropdown.List>
+            <Dropdown.Item>
+              <Dropdown.Button command='show-modal' commandFor='adjacent-modal'>
+                Dialog
+              </Dropdown.Button>
+            </Dropdown.Item>
+          </Dropdown.List>
+        </Dropdown>
+      </Dropdown.TriggerContext>
+      <Dialog id='adjacent-modal'>Min dialog</Dialog>
+    </>
+  );
+};
+
+export const WithNestedDropdown: StoryFn<typeof Dropdown> = (args) => {
+  return (
+    <Dropdown.TriggerContext>
+      <Dropdown.Trigger data-color={args['data-color']}>
+        Dropdown
+      </Dropdown.Trigger>
+      <Dropdown {...args}>
+        <Dropdown.List>
+          <Dropdown.Item>
+            <Dropdown.TriggerContext>
+              <Dropdown.Trigger variant='tertiary'>Dropdown</Dropdown.Trigger>
+              <Dropdown>
+                <Dropdown.List>
+                  <Dropdown.Item>
+                    <Dropdown.Button>Nested</Dropdown.Button>
+                  </Dropdown.Item>
+                </Dropdown.List>
+              </Dropdown>
+            </Dropdown.TriggerContext>
+          </Dropdown.Item>
+        </Dropdown.List>
+      </Dropdown>
+    </Dropdown.TriggerContext>
+  );
+};

packages/react/src/components/popover/popover.test.tsx

@@ -3,6 +3,7 @@ import { act, render, screen } from '@testing-library/react';
 import { useState } from 'react';
 
 import { Button } from '../';
+import { Dialog } from '../dialog';
 import { Popover } from './';
 
 const CompControlled = () => {
@@ -69,7 +70,7 @@ describe('Popover', () => {
     expect(screen.getByText(contentText)).toBeVisible();
   });
 
-  it('should close when we click the button twitce', async () => {
+  it('should close when we click the button twice', async () => {
     render(defaultPopover);
     const popoverTrigger = screen.getByRole('button');
 
@@ -104,6 +105,49 @@ describe('Popover', () => {
     expect(screen.getByText(contentText)).not.toBeVisible();
   });
 
+  it('should not close on ESC when a Dialog is open above the popover', async () => {
+    const onClose = vi.fn();
+    render(
+      <>
+        <Button popovertarget='popover'>popover trigger</Button>
+        <Popover id='popover' onClose={onClose}>
+          {contentText}
+          <Button command='show-modal' commandfor='dialog'>
+            dialog trigger
+          </Button>
+        </Popover>
+        <Dialog
+          id='dialog'
+          closeButton={false}
+          style={{ inset: 0, width: '100vw', height: '100vh', margin: 0 }}
+          aria-label='covering dialog'
+        >
+          dialog content
+        </Dialog>
+      </>,
+    );
+
+    const popoverTrigger = screen.getByRole('button');
+    await act(async () => popoverTrigger.click());
+
+    const dialogTrigger = screen.getByRole('button', {
+      name: 'dialog trigger',
+    });
+    await act(async () => dialogTrigger.click());
+
+    expect(screen.getByText(contentText)).toBeVisible();
+    expect(screen.getByRole('dialog')).toBeVisible();
+
+    const esc = new KeyboardEvent('keydown', { key: 'Escape', bubbles: true });
+    await act(async () => document.body.dispatchEvent(esc));
+
+    expect(onClose).not.toHaveBeenCalled();
+    expect(screen.getByText(contentText)).toBeVisible();
+
+    expect(screen.queryByRole('dialog')).toBeNull();
+    expect(screen.getByText('dialog content')).not.toBeVisible();
+  });
+
   it('should close when we press SPACE', async () => {
     render(defaultPopover);
 

packages/react/src/components/popover/popover.tsx

@@ -74,7 +74,6 @@ export type PopoverProps = MergeRight<
  *   Content
  * </Popover>
  */
-
 export const Popover = forwardRef<HTMLDivElement, PopoverProps>(
   function Popover(
     {
@@ -118,16 +117,12 @@ export const Popover = forwardRef<HTMLDivElement, PopoverProps>(
       };
 
       const handleKeydown = (event: KeyboardEvent) => {
-        if (event.key !== 'Escape' || !controlledOpen) return;
-        const isOpen =
-          popoverRef.current?.matches(':popover-open') ||
-          popoverRef.current?.classList.contains(':popover-open'); // Polyfill support
-
-        if (!isOpen) return;
-        event.preventDefault(); // Prevent closing fullscreen in Safari
-        document.querySelector<HTMLElement>(trigger)?.focus?.(); // Move focus back to trigger since `popoover="manual"` doesn't do this
-        setInternalOpen(false);
-        onClose?.();
+        if (event.key === 'Escape' && controlledOpen && isTopLayer(popover)) {
+          event.preventDefault(); // Prevent closing fullscreen in Safari
+          document.querySelector<HTMLElement>(trigger)?.focus?.(); // Move focus back to trigger since `popover="manual"` doesn't do this
+          setInternalOpen(false);
+          onClose?.();
+        }
       };
 
       popover?.togglePopover?.(controlledOpen);
@@ -163,3 +158,20 @@ export const Popover = forwardRef<HTMLDivElement, PopoverProps>(
     );
   },
 );
+
+// NOTE: This is not able to check if the popover is the most recently added #topLayer,
+// so we need another method in time, or remove the controlled popover="manual" in a v2
+const isTopLayer = (checkElement?: Element | null) => {
+  if (!checkElement) return false;
+  const { x, y, width, height } = checkElement.getBoundingClientRect();
+  const topElement = document.elementFromPoint(x + width / 2, y + height / 2);
+
+  // If the element on top is on browser #top-layer but not same as provided element, then provided element is not on top
+  for (let el = topElement; el; el = el.parentElement) {
+    if (checkElement === el) return true; // If the topElement is same as provided element, it's on top
+    if (el instanceof HTMLDialogElement && el.open) return false; // Check for open dialog
+    if (el.classList.contains(':popover-open')) return false; // Polyfill support
+    if (el.matches(':popover-open')) return false; // Native support
+  }
+  return false;
+};