PopoverPrimitive - Allow dynamic toggle element

[zamoore]

📌 Summary

If merged, this PR will fix a bug where the Hds::PopoverPrimitive would stop working if its toggle element was dynamically swapped, ensuring the popover remains correctly associated and positioned.

🛠️ Detailed description

The previous implementation only ran the logic to link the toggle and popover (popovertarget, aria-controls, and positioning) once when the component was first rendered. This caused a bug when a consumer dynamically changed the toggle element, as the new element was never linked to the popover.

Example of Broken Behavior

In the code below, clicking the "Original" button worked, but after a state change, the "New" button would not trigger the popover.

<Hds::PopoverPrimitive @enableClickEvents={{true}} as |PP|>
  {{#if this.isSwapped}}
    <button {{PP.setupPrimitiveToggle}}>New Toggle</button>
  {{else}}
    <button {{PP.setupPrimitiveToggle}}>Original Toggle</button>
  {{/if}}
  <div {{PP.setupPrimitivePopover}}>Content</div>
</Hds::PopoverPrimitive>

🔗 External links

Jira ticket: HDS-4701

---

👀 Component checklist

• Percy was checked for any visual regression
• A changelog entry was added via Changesets if needed (see templates here)

💬 Please consider using conventional comments when reviewing this PR.

📋 PCI review checklist

• If applicable, I've documented a plan to revert these changes if they require more than reverting the pull request.
• If applicable, I've worked with GRC to document the impact of any changes to security controls.
  Examples of changes to controls include access controls, encryption, logging, etc.
• If applicable, I've worked with GRC to ensure compliance due to a significant change to the in-scope PCI environment.
  Examples include changes to operating systems, ports, protocols, services, cryptography-related components, PII processing code, etc.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Updated (UTC)
hds-showcase	Ready	Preview	Sep 16, 2025 8:05pm
hds-website	Ready	Preview	Sep 16, 2025 8:05pm

[Copilot]

Pull Request Overview

This PR fixes a bug in the Hds::PopoverPrimitive component where dynamically swapped toggle elements would stop working, ensuring proper association between toggle and popover elements regardless of when they're rendered.

• Refactored the linking logic to run whenever toggle or popover elements are set up
• Added proper cleanup for toggle elements when they're removed from the DOM
• Improved positioning update handling to work with dynamically changed elements

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
packages/components/src/components/hds/popover-primitive/index.ts	Core fix implementing dynamic linking between toggle and popover elements
showcase/tests/integration/components/hds/popover-primitive/index-test.js	Added comprehensive test coverage for dynamic toggle swapping and improved existing tests
.changeset/brave-owls-jump.md	Changelog entry documenting the new dynamic toggle element support

[didoo]

Great PR! 🙌

I have left only a few suggestions, but nothing is a blocker.

[alex-ju]

I followed the logic and I can see why this implementation is flagged as minor, which is great. Should we plan to test it with a release candidate though?

[shleewhite]

Could you add an example of the toggle switching to the showcase?

[didoo]

Could you add an example of the toggle switching to the showcase?

[premise: no strong opinions] I thought the same at the beginning, but then I thought that maybe in this case it's not necessary, there is nothing UI related that a visual regression tests or a human test could catch, that the integration tests does not already catch

[zamoore]

I followed the logic and I can see why this implementation is flagged as minor, which is great. Should we plan to test it with a release candidate though?

Independently of the next release? We can, but I don't think it's necessary.

[zamoore]

Could you add an example of the toggle switching to the showcase?

[premise: no strong opinions] I thought the same at the beginning, but then I thought that maybe in this case it's not necessary, there is nothing UI related that a visual regression tests or a human test could catch, that the integration tests does not already catch

@shleewhite @didoo

This was my thinking as well. I can add the examples in the showcase if anyone has a strong opinion, but this more covers an edge case than provides additional intentional behavior.

[shleewhite]

@zamoore @didoo ehh im convinced- we can leave it out for now.