fix(Popover): use native Popover API and CSS refinements (#2369)

## Uses Popover API - This fixes tab/keyboard navigation out of the box, meaning if you trigger a popover, press tab, the focus will enter the popover element regardless of DOM structure - This fixes z-index issues when paired with i.e. dialog, meaning we do not need the `portal` concept anymore - This simplifies the API meaning we can use `popovertarget="ID"` on any element - aligning our API with HTML standards and becoming very similar when going framework independent in the future - Older iOS versions does not support popover API, so maybe we should ship with [the polyfill](https://github.com/oddbird/popover-polyfill) in our project for now so we can provide 100% support and just remove the polyfill when iOS 16.7 is "deaderer"? :D ## Arrow is drawn with pseudo-element - This makes `:first-child` and `:last-child` CSS selectors work as expected - This makes it easier to make popover elements in the future when making web components - floating-ui does not support this out of the box, but is easy to fix with a simple custom middleware --------- Co-authored-by: Tobias Barsnes <[email protected]>
digdir · Sep 11, 2024 · 48f5713 · 48f5713
1 parent 6ae7cc4
commit 48f5713
Show file tree

Hide file tree

Showing 18 changed files with 552 additions and 650 deletions.
diff --git a/.changeset/loud-tips-return.md b/.changeset/loud-tips-return.md
@@ -0,0 +1,9 @@
+---
+"@digdir/designsystemet-css": patch
+"@digdir/designsystemet-react": patch
+---
+
+Popover: 
+- Rename `<Popover.Root>` to `<Popover.Context>`
+- use Popover API, allowing `<Popover>` to be used without `Popover.Context`
+- Remove `portal` prop
diff --git a/apps/theme/components/ColorPicker/ColorPicker.tsx b/apps/theme/components/ColorPicker/ColorPicker.tsx
@@ -3,7 +3,7 @@ import { Popover } from '@digdir/designsystemet-react';
 import { CheckmarkIcon, ExclamationmarkIcon } from '@navikt/aksel-icons';
 import { useClickOutside } from '@react-awesome/use-click-outside';
 import cl from 'clsx/lite';
-import { useEffect, useRef, useState } from 'react';
+import { useEffect, useId, useRef, useState } from 'react';
 import { ChromePicker } from 'react-color';
 
 import classes from './ColorPicker.module.css';
@@ -41,56 +41,55 @@ export const ColorPicker = ({
   }, [defaultColor]);
 
   const getStatus = () => {
+    const popoverId = useId();
     return (
       <div>
-        <Popover.Root
-          onOpenChange={function Ya() {}}
+        <button
+          popovertarget={popoverId}
+          className={cl(
+            classes.status,
+            'ds-focus',
+            colorError === 'decorative' && classes.statusYellow,
+            colorError === 'interaction' && classes.statusOrange,
+          )}
+        >
+          {colorError === 'none' && (
+            <CheckmarkIcon title='Alt er OK med fargen' />
+          )}
+          {colorError === 'decorative' && (
+            <ExclamationmarkIcon title='Viktig informasjon om fargen' />
+          )}
+          {colorError === 'interaction' && (
+            <ExclamationmarkIcon title='Viktig informasjon om fargen' />
+          )}
+        </button>
+        <Popover
+          style={{ width: '800px' }}
+          id={popoverId}
           placement='top'
           size='sm'
           variant={colorError === 'none' ? 'default' : 'warning'}
         >
-          <Popover.Trigger asChild>
-            <button
-              className={cl(
-                classes.status,
-                'ds-focus',
-                colorError === 'decorative' && classes.statusYellow,
-                colorError === 'interaction' && classes.statusOrange,
-              )}
-            >
-              {colorError === 'none' && (
-                <CheckmarkIcon title='Alt er OK med fargen' />
-              )}
-              {colorError === 'decorative' && (
-                <ExclamationmarkIcon title='Viktig informasjon om fargen' />
-              )}
-              {colorError === 'interaction' && (
-                <ExclamationmarkIcon title='Viktig informasjon om fargen' />
-              )}
-            </button>
-          </Popover.Trigger>
-          <Popover.Content style={{ width: '800px' }}>
-            <div>
-              {colorError === 'none' &&
-                'Denne fargen har god nok kontrast og kan brukes normalt i systemet.'}
-            </div>
-            <div>
-              {colorError === 'decorative' && (
-                <div>
-                  Vær oppmerksom på at Base Default fargen har mindre enn 3:1
-                  kontrast mot bakgrunnsfargene. Se alle kontrastgrensene inne
-                  på hver farge.
-                </div>
-              )}
-              {colorError === 'interaction' && (
-                <div>
-                  Base Default fargen har ikke god nok kontrast mot hvit eller
-                  svart tekst på tvers av Base fargene.
-                </div>
-              )}
-            </div>
-          </Popover.Content>
-        </Popover.Root>
+          <div>
+            {colorError === 'none' &&
+              'Denne fargen har god nok kontrast og kan brukes normalt i systemet.'}
+          </div>
+          <div>
+            {colorError === 'decorative' && (
+              <div>
+                Vær oppmerksom på at Base Default fargen har mindre enn 3:1
+                kontrast mot bakgrunnsfargene. Se alle kontrastgrensene inne på
+                hver farge.
+              </div>
+            )}
+            {colorError === 'interaction' && (
+              <div>
+                Base Default fargen har ikke god nok kontrast mot hvit eller
+                svart tekst på tvers av Base fargene.
+              </div>
+            )}
+          </div>
+        </Popover>
       </div>
     );
   };

diff --git a/packages/css/popover.css b/packages/css/popover.css
@@ -1,84 +1,76 @@
 .ds-popover {
-  --dsc-popover-border: 1px solid;
+  --dsc-popover-arrow-size: var(--ds-spacing-2);
   --dsc-popover-background: var(--ds-color-neutral-background-default);
+  --dsc-popover-border-radius: min(1rem, var(--ds-border-radius-md));
+  --dsc-popover-border: 1px solid var(--ds-color-neutral-border-default);
   --dsc-popover-color: var(--ds-color-neutral-text-default);
-  --dsc-popover-border-color: var(--ds-color-neutral-border-default);
-  --dsc-popover-padding: var(--ds-spacing-3);
   --dsc-popover-max-width: 300px;
-  --dsc-popover-border-radius: min(1rem, var(--ds-border-radius-md));
+  --dsc-popover-padding: var(--ds-spacing-3) var(--ds-spacing-4);
 
-  z-index: 1500;
   background: var(--dsc-popover-background);
-  color: var(--dsc-popover-color);
-  padding: var(--dsc-popover-padding);
-  border: var(--dsc-popover-border);
   border-radius: var(--dsc-popover-border-radius);
-  border-color: var(--dsc-popover-border-color);
+  border: var(--dsc-popover-border);
+  box-sizing: border-box;
+  color: var(--dsc-popover-color);
+  inset: 0 auto auto 0;
   max-width: var(--dsc-popover-max-width);
-}
-
-.ds-popover--sm {
-  --dsc-popover-padding: var(--ds-spacing-2) var(--ds-spacing-3);
-}
-
-.ds-popover--md {
-  --dsc-popover-padding: var(--ds-spacing-3) var(--ds-spacing-4);
-}
+  overflow: visible;
+  padding: var(--dsc-popover-padding);
+  position: fixed;
 
-.ds-popover--lg {
-  --dsc-popover-padding: var(--ds-spacing-3) var(--ds-spacing-5);
-}
+  &::before {
+    background: var(--dsc-popover-background);
+    border: inherit;
+    border-left-color: transparent;
+    border-top-color: transparent;
+    box-sizing: border-box;
+    content: '';
+    height: var(--dsc-popover-arrow-size);
+    left: var(--ds-popover-arrow-x);
+    position: absolute;
+    top: var(--ds-popover-arrow-y);
+    translate: -50% -50%;
+    width: var(--dsc-popover-arrow-size);
+  }
 
-.ds-popover--default {
-  --dsc-popover-background: var(--ds-color-neutral-background-default);
-  --dsc-popover-border-color: var(--ds-color-neutral-border-default);
-  --dsc-popover-color: var(--ds-color-neutral-text-default);
-}
-
-.ds-popover--info {
-  --dsc-popover-background: var(--ds-color-info-surface-default);
-  --dsc-popover-border-color: var(--ds-color-info-border-default);
-  --dsc-popover-color: var(--ds-color-info-text-default);
-}
+  &[data-size="sm"] { --dsc-popover-padding: var(--ds-spacing-2) var(--ds-spacing-3) }
+  &[data-size="lg"] { --dsc-popover-padding: var(--ds-spacing-3) var(--ds-spacing-5) }
 
-.ds-popover--warning {
-  --dsc-popover-background: var(--ds-color-warning-surface-default);
-  --dsc-popover-border-color: var(--ds-color-warning-border-default);
-  --dsc-popover-color: var(--ds-color-warning-text-default);
-}
+  &[data-placement="top"]::before {
+    top: 100%;
+    rotate: 45deg;
+  }
 
-.ds-popover--danger {
-  --dsc-popover-background: var(--ds-color-danger-surface-default);
-  --dsc-popover-border-color: var(--ds-color-danger-border-default);
-  --dsc-popover-color: var(--ds-color-danger-text-default);
-}
+  &[data-placement="left"]::before {
+    left: 100%;
+    rotate: -45deg;
+  }
 
-.ds-popover__arrow {
-  position: absolute;
-  background: var(--dsc-popover-background);
-  transform: rotate(45deg);
-}
+  &[data-placement="right"]::before {
+    left: 0;
+    rotate: 135deg;
+  }
 
-.ds-popover__arrow.ds-popover__arrow--top {
-  border-top: var(--dsc-popover-border);
-  border-left: var(--dsc-popover-border);
-  border-color: inherit;
-}
+  &[data-placement="bottom"]::before {
+    top: 0;
+    rotate: -135deg;
+  }
 
-.ds-popover__arrow.ds-popover__arrow--bottom {
-  border-bottom: var(--dsc-popover-border);
-  border-right: var(--dsc-popover-border);
-  border-color: inherit;
-}
+  &[data-variant="info"] {
+    --dsc-popover-background: var(--ds-color-info-surface-default);
+    --dsc-popover-border: 1px solid var(--ds-color-info-border-default);
+    --dsc-popover-color: var(--ds-color-info-text-default);
+  }
 
-.ds-popover__arrow.ds-popover__arrow--right {
-  border-top: var(--dsc-popover-border);
-  border-right: var(--dsc-popover-border);
-  border-color: inherit;
-}
+  &[data-variant="warning"] {
+    --dsc-popover-background: var(--ds-color-warning-surface-default);
+    --dsc-popover-border: 1px solid var(--ds-color-warning-border-default);
+    --dsc-popover-color: var(--ds-color-warning-text-default);
+  }
 
-.ds-popover__arrow.ds-popover__arrow--left {
-  border-bottom: var(--dsc-popover-border);
-  border-left: var(--dsc-popover-border);
-  border-color: inherit;
+  &[data-variant="danger"] {
+    --dsc-popover-background: var(--ds-color-danger-surface-default);
+    --dsc-popover-border: 1px solid var(--ds-color-danger-border-default);
+    --dsc-popover-color: var(--ds-color-danger-text-default);
+  }
 }
diff --git a/packages/react/package.json b/packages/react/package.json
@@ -35,6 +35,7 @@
     "access": "public"
   },
   "dependencies": {
+    "@floating-ui/dom": "^1.6.10",
     "@floating-ui/react": "0.26.23",
     "@navikt/aksel-icons": "^6.14.0",
     "@radix-ui/react-slot": "^1.1.0",

diff --git a/packages/react/src/components/HelpText/HelpText.stories.tsx b/packages/react/src/components/HelpText/HelpText.stories.tsx
@@ -31,7 +31,6 @@ export const Portal: Story = {
     title: 'Help text title',
     children: 'Help text content',
     size: 'md',
-    portal: true,
     placement: 'top',
   },
 };
diff --git a/packages/react/src/components/HelpText/HelpText.test.tsx b/packages/react/src/components/HelpText/HelpText.test.tsx
@@ -23,18 +23,18 @@ describe('HelpText', () => {
     render();
     const helpTextTrigger = screen.getByRole('button');
 
-    expect(helpTextTrigger).toBeInTheDocument();
+    expect(helpTextTrigger).toBeVisible();
   });
 
   it('should open HelpText on trigger-click when closed', async () => {
     render();
     const helpTextTrigger = screen.getByRole('button');
 
-    expect(screen.queryByText('Help')).not.toBeInTheDocument();
+    expect(screen.queryByText('Help')).not.toBeVisible();
     await act(async () => {
       await user.click(helpTextTrigger);
     });
-    expect(screen.queryByText('Help')).toBeInTheDocument();
+    expect(screen.queryByText('Help')).toBeVisible();
   });
 
   it('should close HelpText on trigger-click when open', async () => {
@@ -44,23 +44,23 @@ describe('HelpText', () => {
     await act(async () => {
       await user.click(helpTextTrigger);
     });
-    expect(screen.queryByText('Help')).toBeInTheDocument();
+    expect(screen.queryByText('Help')).toBeVisible();
     await act(async () => {
       await user.click(helpTextTrigger);
     });
-    expect(screen.queryByText('Help')).not.toBeInTheDocument();
+    expect(screen.queryByText('Help')).not.toBeVisible();
   });
 
   it('should open HelpText on SPACE pressed when closed', async () => {
     render();
     const helpTextTrigger = screen.getByRole('button');
 
-    expect(screen.queryByText('Help')).not.toBeInTheDocument();
+    expect(screen.queryByText('Help')).not.toBeVisible();
     helpTextTrigger.focus();
     await act(async () => {
       await user.keyboard('[Space]');
     });
-    expect(screen.queryByText('Help')).toBeInTheDocument();
+    expect(screen.queryByText('Help')).toBeVisible();
   });
 
   it('should close HelpText on ESC pressed when open', async () => {
@@ -71,27 +71,10 @@ describe('HelpText', () => {
     await act(async () => {
       await user.click(helpTextTrigger);
     });
-    expect(screen.queryByText('Help')).toBeInTheDocument();
+    expect(screen.queryByText('Help')).toBeVisible();
     await act(async () => {
       await user.keyboard('[Escape]');
     });
-    expect(screen.queryByText('Help')).not.toBeInTheDocument();
-  });
-
-  it('should have `aria-expanded` set to `false` when closed', () => {
-    render();
-    const helpTextTrigger = screen.getByRole('button');
-
-    expect(helpTextTrigger).toHaveAttribute('aria-expanded', 'false');
-  });
-
-  it('should have `aria-expanded` set to `true` when open', async () => {
-    render();
-    const helpTextTrigger = screen.getByRole('button');
-
-    await act(async () => {
-      await user.click(helpTextTrigger);
-    });
-    expect(helpTextTrigger).toHaveAttribute('aria-expanded', 'true');
+    expect(screen.queryByText('Help')).not.toBeVisible();
   });
 });