PopoverPrimitive - Allow dynamic toggle element

.changeset/brave-owls-jump.md

@@ -0,0 +1,7 @@
+---
+"@hashicorp/design-system-components": minor
+---
+
+<!-- START utilities/popover-primitive -->
+`PopoverPrimitive` - Added support for dynamic swap/injection of the toggle element.
+<!-- END -->

packages/components/src/components/hds/popover-primitive/index.ts

@@ -62,6 +62,7 @@ export interface SetupPrimitivePopoverModifier {
 export default class HdsPopoverPrimitive extends Component<HdsPopoverPrimitiveSignature> {
   @tracked private _isOpen;
   @tracked private _isClosing = false;
+  @tracked private _anchoredPositionOptions?: HdsAnchoredPositionOptions;
   private _containerElement?: HTMLElement;
   private _toggleElement?: HTMLButtonElement;
   private _popoverElement?: HTMLElement;
@@ -106,13 +107,23 @@ export default class HdsPopoverPrimitive extends Component<HdsPopoverPrimitiveSi
   );
 
   setupPrimitiveToggle = modifier<SetupPrimitiveToggleModifier>(
-    (element: HTMLButtonElement): void => {
+    (element: HTMLButtonElement) => {
       this._toggleElement = element;
 
       assert(
         `The toggle element of "Hds::PopoverPrimitive" must be a <button>; element received: <${element.tagName.toLowerCase()}>`,
         element instanceof HTMLButtonElement
       );
+
+      this._linkToggleAndPopover();
+
+      // Return a teardown function to clean up the modifier's side effects.
+      // This is a safeguard against bugs where this element might be
+      // cached and re-parented in the DOM, rather than being fully destroyed.
+      return () => {
+        element.removeAttribute('aria-controls');
+        element.removeAttribute('popovertarget');
+      };
     }
   );
 
@@ -126,23 +137,8 @@ export default class HdsPopoverPrimitive extends Component<HdsPopoverPrimitiveSi
 
       // We need to create a popoverId in order to connect the popover and the toggle with aria-controls
       // and an id is needed to implement `onclick` event listeners
-      if (this._toggleElement) {
-        let popoverId;
-        if (this._popoverElement.id) {
-          popoverId = this._popoverElement.id;
-        } else {
-          // we need a DOM id for the `aria-controls` and `popovertarget` attributes
-          popoverId = guidFor(this);
-          this._popoverElement.id = popoverId;
-        }
-        this._toggleElement.setAttribute('aria-controls', popoverId);
-
-        // for the click events we don't use `onclick` event listeners, but we rely on the `popovertarget` attribute
-        // provided by the Popover API which does all the magic for us without needing JS code
-        // (important: to work it needs to be applied to a button)
-        if (this.enableClickEvents) {
-          this._toggleElement.setAttribute('popovertarget', popoverId);
-        }
+      if (!this._popoverElement.id) {
+        this._popoverElement.id = guidFor(this);
       }
 
       // this should be an extremely edge case, but in the case the popover needs to be initially forced to be open
@@ -167,27 +163,58 @@ export default class HdsPopoverPrimitive extends Component<HdsPopoverPrimitiveSi
       registerEvent(this._popoverElement, ['toggle', this.onTogglePopover]);
 
       // we need to spread the argument because if it's set via `{{ hash … }}` Ember complains when we overwrite one of its values
-      const anchoredPositionOptions: HdsAnchoredPositionOptions = {
+      this._anchoredPositionOptions = {
         ...named.anchoredPositionOptions,
       };
 
-      // Apply the `hds-anchored-position` modifier to the "popover" element
-      // (notice: this function runs the first time when the element the modifier was applied to is inserted into the DOM, and it autotracks while running.
-      // Any tracked values that it accesses will be tracked, including the arguments it receives, and if any of them changes, the function will run again)
-      // This modifiers uses the Floating UI library to provide:
-      // - positioning of the "popover" in relation to the "toggle"
-      // - collision detection (optional)
+      this._linkToggleAndPopover();
+    }
+  );
+
+  // Apply the `hds-anchored-position` modifier to the "popover" element
+  // (notice: this function runs the first time when the element the modifier was applied to is inserted into the DOM, and it autotracks while running.
+  // Any tracked values that it accesses will be tracked, including the arguments it receives, and if any of them changes, the function will run again)
+  // This modifiers uses the Floating UI library to provide:
+  // - positioning of the "popover" in relation to the "toggle"
+  // - collision detection (optional)
+  private _applyAnchoredPositionModifier(): void {
+    if (
+      this._toggleElement !== undefined &&
+      this._popoverElement !== undefined &&
+      this._anchoredPositionOptions !== undefined
+    ) {
       // eslint-disable-next-line ember/no-runloop
       next((): void => {
         // @ts-expect-error: known issue with type of invocation
         anchoredPositionModifier(
           this._popoverElement, // element the modifier is attached to
           [this._toggleElement], // positional arguments
-          anchoredPositionOptions // named arguments
+          this._anchoredPositionOptions // named arguments
         );
       });
     }
-  );
+  }
+
+  private _linkToggleAndPopover(): void {
+    if (
+      this._toggleElement === undefined ||
+      this._popoverElement === undefined
+    ) {
+      return;
+    }
+
+    const popoverId = this._popoverElement.id;
+
+    this._toggleElement.setAttribute('aria-controls', popoverId);
+
+    if (this.enableClickEvents) {
+      this._toggleElement.setAttribute('popovertarget', popoverId);
+    } else {
+      this._toggleElement.removeAttribute('popovertarget');
+    }
+
+    this._applyAnchoredPositionModifier();
+  }
 
   @action
   showPopover(): void {

showcase/tests/integration/components/hds/popover-primitive/index-test.js

@@ -21,7 +21,7 @@ module(
       await render(hbs`
         <Hds::PopoverPrimitive @enableClickEvents={{true}} as |PP|>
           <div {{PP.setupPrimitiveContainer}}>
-            <button {{PP.setupPrimitiveToggle}} />
+            <button {{PP.setupPrimitiveToggle}} type="button" />
             <main {{PP.setupPrimitivePopover}} />
           </div>
         </Hds::PopoverPrimitive>
@@ -38,7 +38,7 @@ module(
       await render(hbs`
         <Hds::PopoverPrimitive @enableClickEvents={{true}} as |PP|>
           <div {{PP.setupPrimitiveContainer}}>
-            <button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" />
+            <button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" type="button" />
             <div {{PP.setupPrimitivePopover}} id="test-popover-primitive-content" />
           </div>
         </Hds::PopoverPrimitive>
@@ -57,7 +57,7 @@ module(
       await render(hbs`
         <Hds::PopoverPrimitive @enableSoftEvents={{true}} as |PP|>
           <div {{PP.setupPrimitiveContainer}}>
-            <button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" />
+            <button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" type="button" />
             <div {{PP.setupPrimitivePopover}} id="test-popover-primitive-content" />
           </div>
         </Hds::PopoverPrimitive>
@@ -82,7 +82,7 @@ module(
       await render(hbs`
         <Hds::PopoverPrimitive @enableClickEvents={{true}} as |PP|>
           <div {{PP.setupPrimitiveContainer}}>
-            <button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" />
+            <button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" type="button" />
             <div {{PP.setupPrimitivePopover}} id="test-popover-primitive-content" />
           </div>
         </Hds::PopoverPrimitive>
@@ -103,6 +103,72 @@ module(
       // should go back to hidden
       assert.dom('#test-popover-primitive-content').isNotVisible();
     });
+    test('it should continue to work when the toggle element is dynamically swapped', async function (assert) {
+      this.set('isSwapped', false);
+
+      await render(hbs`
+        <Hds::PopoverPrimitive @enableClickEvents={{true}} as |PP|>
+          <div {{PP.setupPrimitiveContainer}}>
+            {{#if this.isSwapped}}
+              <button data-test-id="replacement-toggle" type="button" {{PP.setupPrimitiveToggle}}>
+                Replacement
+              </button>
+            {{else}}
+              <button data-test-id="original-toggle" type="button" {{PP.setupPrimitiveToggle}}>
+                Original
+              </button>
+            {{/if}}
+            <div data-test-id="popover-content" {{PP.setupPrimitivePopover}}>
+              Content
+            </div>
+          </div>
+        </Hds::PopoverPrimitive>
+      `);
+
+      // verify the initial toggle works as expected
+      assert
+        .dom('[data-test-id="original-toggle"]')
+        .exists('The original toggle is rendered');
+      assert
+        .dom('[data-test-id="popover-content"]')
+        .isNotVisible('The popover is initially hidden');
+
+      await click('[data-test-id="original-toggle"]');
+      assert
+        .dom('[data-test-id="popover-content"]')
+        .isVisible('The popover becomes visible after the first click');
+
+      await click('[data-test-id="original-toggle"]');
+      assert
+        .dom('[data-test-id="popover-content"]')
+        .isNotVisible('The popover is hidden again');
+
+      // swap the toggle element
+      this.set('isSwapped', true);
+      assert
+        .dom('[data-test-id="original-toggle"]')
+        .doesNotExist('The original toggle is removed');
+      assert
+        .dom('[data-test-id="replacement-toggle"]')
+        .exists('The replacement toggle is rendered');
+
+      // verify the *new* toggle now controls the popover
+      assert
+        .dom('[data-test-id="popover-content"]')
+        .isNotVisible('The popover remains hidden after the swap');
+
+      await click('[data-test-id="replacement-toggle"]');
+      assert
+        .dom('[data-test-id="popover-content"]')
+        .isVisible(
+          'The popover becomes visible when the new toggle is clicked',
+        );
+
+      await click('[data-test-id="replacement-toggle"]');
+      assert
+        .dom('[data-test-id="popover-content"]')
+        .isNotVisible('The popover is hidden again by the new toggle');
+    });
     skip('it should toggle the popover visibility on click', async function (assert) {
       await render(hbs`
         <Hds::PopoverPrimitive @enableClickEvents={{true}}>
@@ -141,7 +207,7 @@ module(
           as |PP|
         >
           <div {{PP.setupPrimitiveContainer}}>
-            <button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" />
+            <button {{PP.setupPrimitiveToggle}}  id="test-popover-primitive-toggle" />
             <div {{PP.setupPrimitivePopover}} />
           </div>
         </Hds::PopoverPrimitive>
@@ -165,7 +231,7 @@ module(
       await render(hbs`
         <Hds::PopoverPrimitive as |PP|>
           <div {{PP.setupPrimitiveContainer}}>
-            <button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" />
+            <button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" type="button" />
             <div {{PP.setupPrimitivePopover}} />
           </div>
         </Hds::PopoverPrimitive>
@@ -178,7 +244,7 @@ module(
       await render(hbs`
         <Hds::PopoverPrimitive @enableClickEvents={{true}} as |PP|>
           <div {{PP.setupPrimitiveContainer}}>
-            <button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" />
+            <button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" type="button" />
             <div {{PP.setupPrimitivePopover}} id="test-popover-primitive-popover" />
           </div>
         </Hds::PopoverPrimitive>
@@ -191,7 +257,7 @@ module(
       await render(hbs`
         <Hds::PopoverPrimitive as |PP|>
           <div {{PP.setupPrimitiveContainer}}>
-            <button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" />
+            <button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" type="button" />
             <div {{PP.setupPrimitivePopover}} id="test-popover-primitive-content" />
           </div>
         </Hds::PopoverPrimitive>
@@ -204,7 +270,7 @@ module(
       await render(hbs`
         <Hds::PopoverPrimitive @enableClickEvents={{true}} @isOpen={{true}} as |PP|>
           <div {{PP.setupPrimitiveContainer}}>
-            <button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" />
+            <button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" type="button" />
             <div {{PP.setupPrimitivePopover}} id="test-popover-primitive-content" />
           </div>
         </Hds::PopoverPrimitive>
@@ -226,7 +292,7 @@ module(
       await render(hbs`
         <Hds::PopoverPrimitive @enableClickEvents={{true}} @isOpen={{true}} as |PP|>
           <div {{PP.setupPrimitiveContainer}}>
-            <button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" />
+            <button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" type="button" />
             <div {{PP.setupPrimitivePopover}} id="test-popover-primitive-content" />
           </div>
         </Hds::PopoverPrimitive>
@@ -244,7 +310,7 @@ module(
       await render(hbs`
         <Hds::PopoverPrimitive @enableClickEvents={{true}} @isOpen={{true}} as |PP|>
           <div {{PP.setupPrimitiveContainer}}>
-            <button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" />
+            <button {{PP.setupPrimitiveToggle}} id="test-popover-primitive-toggle" type="button" />
             <div {{PP.setupPrimitivePopover}} id="test-popover-primitive-content" />
           </div>
         </Hds::PopoverPrimitive>