Edit popover component

packages/circuit-ui/components/Popover/Popover.module.css

@@ -17,77 +17,33 @@
   display: inline-block;
 }
 
-.menu {
+.content {
   box-sizing: border-box;
   max-height: var(--popover-max-height);
-  padding: var(--cui-spacings-byte) 0;
+  padding: 0;
+  margin: 0;
   overflow-y: auto;
+  pointer-events: none;
   visibility: hidden;
   background-color: var(--cui-bg-elevated);
-  border: 1px solid var(--cui-border-subtle);
+  border: none;
   border-radius: var(--cui-border-radius-byte);
   box-shadow: 0 3px 8px 0 rgb(0 0 0 / 20%);
   opacity: 0;
 }
 
-@media (max-width: 479px) {
-  .menu {
-    border-bottom-right-radius: 0;
-    border-bottom-left-radius: 0;
-    opacity: 1;
-    transition:
-      transform var(--cui-transitions-default),
-      visibility var(--cui-transitions-default);
-    transform: translateY(100%);
-  }
-}
-
-.menu.open {
+.content.open {
+  pointer-events: all;
   visibility: inherit;
   opacity: 1;
 }
 
-@media (max-width: 479px) {
-  .menu.open {
-    transform: translateY(0);
-  }
-}
-
 .divider {
   width: calc(100% - var(--cui-spacings-mega) * 2);
   margin: var(--cui-spacings-byte) var(--cui-spacings-mega);
 }
 
-@media (max-width: 479px) {
-  .overlay {
-    position: fixed;
-    top: 0;
-    right: 0;
-    bottom: 0;
-    left: 0;
-    visibility: hidden;
-    background-color: var(--cui-bg-overlay);
-    opacity: 0;
-    transition:
-      opacity var(--cui-transitions-default),
-      visibility var(--cui-transitions-default);
-  }
-
-  .overlay.open {
-    visibility: inherit;
-    opacity: 1;
-  }
-}
-
-.wrapper {
-  pointer-events: none;
-}
-
-.wrapper.open {
-  pointer-events: all;
-}
-
-.wrapper.open::after {
+.content.open::after {
   position: absolute;
   right: 0;
   bottom: 0;

packages/circuit-ui/components/Popover/Popover.spec.tsx

@@ -13,78 +13,12 @@
  * limitations under the License.
  */
 
-import type { FC } from 'react';
 import { afterEach, describe, expect, it, vi } from 'vitest';
-import { Delete, Add, Download, type IconProps } from '@sumup-oss/icons';
-
-import {
-  act,
-  axe,
-  render,
-  userEvent,
-  screen,
-  type RenderFn,
-} from '../../util/test-utils.js';
-import type { ClickEvent } from '../../types/events.js';
-
-import {
-  PopoverItem,
-  type PopoverItemProps,
-  Popover,
-  type PopoverProps,
-} from './Popover.js';
-
-describe('PopoverItem', () => {
-  function renderPopoverItem<T>(
-    renderFn: RenderFn<T>,
-    props: PopoverItemProps,
-  ) {
-    return renderFn(<PopoverItem {...props} />);
-  }
-
-  const baseProps = {
-    children: 'PopoverItem',
-    icon: Download as FC<IconProps>,
-  };
-
-  describe('Styles', () => {
-    it('should render as Link when an href (and onClick) is passed', () => {
-      const props = {
-        ...baseProps,
-        href: 'https://sumup.com',
-        onClick: vi.fn(),
-      };
-      const { container } = renderPopoverItem(render, props);
-      const anchorEl = container.querySelector('a');
-      expect(anchorEl).toBeVisible();
-    });
+import { createRef } from 'react';
 
-    it('should render as a `button` when an onClick is passed', () => {
-      const props = { ...baseProps, onClick: vi.fn() };
-      const { container } = renderPopoverItem(render, props);
-      const buttonEl = container.querySelector('button');
-      expect(buttonEl).toBeVisible();
-    });
-  });
+import { act, axe, render, userEvent, screen } from '../../util/test-utils.js';
 
-  describe('Logic', () => {
-    it('should call onClick when rendered as Link', async () => {
-      const props = {
-        ...baseProps,
-        href: 'https://sumup.com',
-        onClick: vi.fn((event: ClickEvent) => {
-          event.preventDefault();
-        }),
-      };
-      const { container } = renderPopoverItem(render, props);
-      const anchorEl = container.querySelector('a');
-      if (anchorEl) {
-        await userEvent.click(anchorEl);
-      }
-      expect(props.onClick).toHaveBeenCalledTimes(1);
-    });
-  });
-});
+import { Popover, type PopoverProps } from './Popover.js';
 
 describe('Popover', () => {
   afterEach(() => {
@@ -117,23 +51,18 @@ describe('Popover', () => {
 
   const baseProps: PopoverProps = {
     component: (triggerProps) => <button {...triggerProps}>Button</button>,
-    actions: [
-      {
-        onClick: vi.fn(),
-        children: 'Add',
-        icon: Add as FC<IconProps>,
-      },
-      { type: 'divider' },
-      {
-        onClick: vi.fn(),
-        children: 'Remove',
-        icon: Delete as FC<IconProps>,
-        destructive: true,
-      },
-    ],
+    children: <div>Popover content</div>,
     isOpen: true,
     onToggle: vi.fn(createStateSetter(true)),
   };
+
+  it('should forward a ref', () => {
+    const ref = createRef<HTMLDialogElement>();
+    renderPopover({ ...baseProps, ref });
+    const dialog = screen.getByRole('dialog', { hidden: true });
+    expect(ref.current).toBe(dialog);
+  });
+
   it('should open the popover when clicking the trigger element', async () => {
     const isOpen = false;
     const onToggle = vi.fn(createStateSetter(isOpen));
@@ -211,29 +140,26 @@ describe('Popover', () => {
     expect(baseProps.onToggle).toHaveBeenCalledTimes(1);
   });
 
-  it('should close the popover when clicking a popover item', async () => {
-    renderPopover(baseProps);
-
-    const popoverItems = screen.getAllByRole('menuitem');
-
-    await userEvent.click(popoverItems[0]);
-
-    expect(baseProps.onToggle).toHaveBeenCalledTimes(1);
-  });
-
   it('should move focus to the first popover item after opening', async () => {
     const isOpen = false;
     const onToggle = vi.fn(createStateSetter(isOpen));
 
-    const { rerender } = renderPopover({
+    const props = {
       ...baseProps,
       isOpen,
       onToggle,
-    });
+      children: (
+        <div>
+          <button type="button">Item</button>
+          <button type="button">Item</button>
+        </div>
+      ),
+    };
+    const { rerender } = renderPopover(props);
 
-    rerender(<Popover {...baseProps} isOpen />);
+    rerender(<Popover {...props} isOpen />);
 
-    const popoverItems = screen.getAllByRole('menuitem');
+    const popoverItems = screen.getAllByText('Item');
 
     expect(popoverItems[0]).toHaveFocus();
 
@@ -261,19 +187,8 @@ describe('Popover', () => {
     });
   });
 
-  it('should render the popover with menu semantics by default ', async () => {
-    renderPopover(baseProps);
-
-    const menu = screen.getByRole('menu');
-    expect(menu).toBeVisible();
-    const menuitems = screen.getAllByRole('menuitem');
-    expect(menuitems.length).toBe(2);
-
-    await flushMicrotasks();
-  });
-
   it('should render the popover without menu semantics ', async () => {
-    renderPopover({ ...baseProps, role: null });
+    renderPopover({ ...baseProps });
 
     const menu = screen.queryByRole('menu');
     expect(menu).toBeNull();
@@ -282,13 +197,4 @@ describe('Popover', () => {
 
     await flushMicrotasks();
   });
-
-  it('should hide dividers from the accessibility tree', async () => {
-    const { baseElement } = renderPopover(baseProps);
-
-    const dividers = baseElement.querySelectorAll('hr[aria-hidden="true"');
-    expect(dividers.length).toBe(1);
-
-    await flushMicrotasks();
-  });
 });

packages/circuit-ui/components/Popover/Popover.stories.tsx

@@ -70,7 +70,9 @@ export const Base = (args: PopoverProps) => {
             Open popover
           </Button>
         )}
-      />
+      >
+        <div style={{ padding: 'var(--cui-spacings-giga)' }}>Hello 👋</div>
+      </Popover>
     </PopoverWrapper>
   );
 };
@@ -93,7 +95,9 @@ export const Offset = (args: PopoverProps) => {
             Open popover
           </Button>
         )}
-      />
+      >
+        <div style={{ padding: 'var(--cui-spacings-giga)' }}>Hello 👋</div>
+      </Popover>
     </PopoverWrapper>
   );
 };