eric fix(#3540): add manual positioning to Popover by willcodeforcoffee · Pull Request #3655 · GovAlta/ui-components

willcodeforcoffee · 2026-03-24T16:05:49Z

This PR adds manual positioning for a Popover when the browser does not support CSS anchor positioning.

Before (the change)

The popover would work only on browsers that support Anchor Positioning. On browsers that do not support Anchor Positioning all popovers would appear in the top left corner of a page.

After (the change)

The popover works on older browsers that do not support Anchor Positioning.

Make sure that you've checked the boxes below before you submit the PR

I have read and followed the setup steps
I have created necessary unit tests
I have tested the functionality in both React and Angular.

Steps needed to test

See this list for browsers and versions that do and do not support anchor positioning: https://caniuse.com/css-anchor-positioning
Test with as many browsers that do and do not support anchor positioning as you can, especially in Browser Stack
Ensure you test with Safari 18.x which is one of the more commonly used browsers on GoA websites
One of the best places to test is in the Feat. 3478 PR test page:
- http://localhost:4200/features/3478
- http://localhost:4201/features/3478

Copilot

Pull request overview

This PR adds a JavaScript fallback to manually position goa-popover when CSS Anchor Positioning is not supported, so popovers don’t default to the page’s top-left in older browsers.

Changes:

Added a manual positioning loop (RAF) to compute and apply top/left for the popover when anchor positioning isn’t available.
Updated open/close handling to start/stop manual positioning and added extra close coordination logic for non-anchor browsers.
Relaxed a few React browser specs by increasing waitFor timeouts.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 9 comments.

File	Description
libs/web-components/src/components/popover/Popover.svelte	Adds manual positioning logic and adjusts toggle/open/close behavior for non-anchor browsers.
libs/react-components/specs/popover.browser.spec.tsx	Increases `waitFor` timeouts for popover close/focus assertions.
libs/react-components/specs/app-header-menu.browser.spec.tsx	Increases `waitFor` timeout for app-header-menu close assertion.

libs/web-components/src/components/popover/Popover.svelte

libs/react-components/specs/popover.browser.spec.tsx

Copilot · 2026-03-24T17:55:39Z

libs/web-components/src/components/popover/Popover.svelte

+    const isAbove =
+      position === "above" ||
+      (position === "auto" && _autoPosition === "above");
+
+    if (isAbove) {
+      _popoverEl.style.top = `${targetRect.top - yOffset}px`;
+      _popoverEl.style.left = `${targetRect.left + xOffset}px`;
+      _popoverEl.style.transform = "translateY(-100%)";
+    } else {
+      _popoverEl.style.top = `${targetRect.bottom + yOffset}px`;
+      _popoverEl.style.left = `${targetRect.left + xOffset}px`;
+      _popoverEl.style.transform = "";
+    }


updatePopoverPosition only handles above/below/auto. When position === "right" and anchor positioning isn't supported, the popover will still be positioned below the target (and may retain position-right class styling that no longer makes sense). Manual positioning should include a right branch that matches the CSS-anchor behavior for position-right.

Copilot · 2026-03-24T17:55:39Z

libs/web-components/src/components/popover/Popover.svelte

+  function startManualPositioning() {
+    if (!_needsManualPositioning) return;
+
+    const loop = () => {
+      updatePopoverPosition();
+      _positionRafId = requestAnimationFrame(loop);
+    };
+    _positionRafId = requestAnimationFrame(loop);
+  }


startManualPositioning() can be called multiple times without checking/canceling an existing loop. If it gets invoked twice (e.g., duplicate toggle handling), the first RAF loop will keep running but its id is lost, causing a leak and extra per-frame work. Add a guard (return if _positionRafId is already set) or call stopManualPositioning() before starting a new loop.

Copilot · 2026-03-24T17:55:39Z

libs/web-components/src/components/popover/Popover.svelte

    // Dispatch _open/_close events for consumer components
    // (MenuButton, AppHeader, AppHeaderMenu, Dropdown)
    if (_isOpen) {
-      dispatch(_rootEl, "_open", {}, { bubbles: true });
+      dispatch(_rootEl, "_open");
      requestAnimationFrame(updateAutoPosition); // same vs await tick(), make sure popover element is fully rendered before we measure its dimension
+      startManualPositioning();
    } else {
-      _targetEl.focus();
-      dispatch(_rootEl, "_close", {}, { bubbles: true });
+      stopManualPositioning();
+      _targetEl?.focus();
+      dispatch(_rootEl, "_close");
    }


dispatch(_rootEl, "_open") / dispatch(_rootEl, "_close") now omit { bubbles: true }. The shared dispatch helper defaults bubbles to false when not provided, so this is a behavior change vs the previous bubbling events and may break consumers relying on event bubbling. Consider restoring { bubbles: true } (and any other previously set options) to avoid an accidental breaking change.

Copilot · 2026-03-24T17:55:39Z

libs/react-components/specs/app-header-menu.browser.spec.tsx

+    await vi.waitFor(
+      () => {
+        const popoverContent = result.getByTestId("popover-content");
+        expect(popoverContent.element().checkVisibility()).toBeFalsy();
+      },
+      { timeout: 10000 },
+    );


This waitFor was bumped to a 10s timeout, which is notably higher than other tests in this suite (commonly 1–3s). If possible, prefer keeping timeouts small/consistent and fix the underlying close timing issue so the test doesn't need such a large budget.

Suggested change

await vi.waitFor(

() => {

const popoverContent = result.getByTestId("popover-content");

expect(popoverContent.element().checkVisibility()).toBeFalsy();

},

{ timeout: 10000 },

);

await vi.waitFor(() => {

const popoverContent = result.getByTestId("popover-content");

expect(popoverContent.element().checkVisibility()).toBeFalsy();

});

Copilot · 2026-03-24T17:55:40Z

libs/web-components/src/components/popover/Popover.svelte

+    if (isAbove) {
+      _popoverEl.style.top = `${targetRect.top - yOffset}px`;
+      _popoverEl.style.left = `${targetRect.left + xOffset}px`;
+      _popoverEl.style.transform = "translateY(-100%)";
+    } else {
+      _popoverEl.style.top = `${targetRect.bottom + yOffset}px`;
+      _popoverEl.style.left = `${targetRect.left + xOffset}px`;
+      _popoverEl.style.transform = "";
+    }


Manual positioning sets top/left and also applies transform, but .popover-content still has the CSS translate: rules (including the position-above -100% shift). In browsers without anchor positioning support this will compound transforms/offsets (e.g., position-above ends up translating by -200% and offsets are effectively applied twice), producing incorrect placement. When _needsManualPositioning is true, disable/override the CSS translate behavior (or rely on it exclusively) so the JS positioning math isn't compounded by existing transform rules.

libs/web-components/src/components/popover/Popover.svelte

bdfranck

I looked at the "Feature #3478: Popover API Rewrite" test scenarios...

✅ The scenarios work with Safari 18

✅ The scenarios still work with the latest version of Safari

Looks good to me! 👍

ArakTaiRoth · 2026-03-24T22:40:27Z

libs/web-components/src/components/popover/Popover.svelte


  // Functions

+  function updatePopoverPosition() {


Safari 18 will ignore Anchor Positioning, but it won't ignore all the translate calls I put in here. So when you're manually calculating the positioning here, the translate calls are still running. This means when someone is using below, the horizontal and vertical offset will be doubled. And when someone is using above, the position of the popover will be twice its height above the element.

…ot support CSS anchor positioning

willcodeforcoffee self-assigned this Mar 24, 2026

willcodeforcoffee added Popover P1 Priority 1 (must have): Prevents product teams from adopting/using DS 2.0 as intended at launch. labels Mar 24, 2026

willcodeforcoffee linked an issue Mar 24, 2026 that may be closed by this pull request

Popover: Safari 18 fallback for CSS anchor positioning #3540

Open

willcodeforcoffee mentioned this pull request Mar 24, 2026

feat(#3540): Add polyfill for anchor positioning #3598

Closed

3 tasks

willcodeforcoffee force-pushed the eric/3540-manual-anchor-positioning branch 2 times, most recently from c4d4bde to e2e7707 Compare March 24, 2026 17:31

willcodeforcoffee marked this pull request as ready for review March 24, 2026 17:48

willcodeforcoffee requested review from bdfranck and Copilot March 24, 2026 17:48

Copilot started reviewing on behalf of willcodeforcoffee March 24, 2026 17:49 View session

Copilot AI reviewed Mar 24, 2026

View reviewed changes

bdfranck approved these changes Mar 24, 2026

View reviewed changes

willcodeforcoffee requested a review from ArakTaiRoth March 24, 2026 20:30

ArakTaiRoth reviewed Mar 24, 2026

View reviewed changes

fix(#3540): add manual positioning to Popover when the browser does n…

cae35f7

…ot support CSS anchor positioning

willcodeforcoffee force-pushed the eric/3540-manual-anchor-positioning branch from 7084f2e to cae35f7 Compare March 24, 2026 23:42

willcodeforcoffee requested a review from bdfranck March 24, 2026 23:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

eric fix(#3540): add manual positioning to Popover#3655

eric fix(#3540): add manual positioning to Popover#3655
willcodeforcoffee wants to merge 1 commit intodevfrom
eric/3540-manual-anchor-positioning

willcodeforcoffee commented Mar 24, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Uh oh!

Uh oh!

bdfranck left a comment

Uh oh!

ArakTaiRoth Mar 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

willcodeforcoffee commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Before (the change)

After (the change)

Make sure that you've checked the boxes below before you submit the PR

Steps needed to test

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bdfranck left a comment

Choose a reason for hiding this comment

Uh oh!

ArakTaiRoth Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

willcodeforcoffee commented Mar 24, 2026 •

edited

Loading