Zamoore/hds 5575/modal and flyout teardown logic changes (#3339)

Author: zamoore

packages/components/src/components/hds/flyout/index.ts

@@ -92,11 +92,7 @@ export default class HdsFlyout extends Component<HdsFlyoutSignature> {
     return classes.join(' ');
   }
 
-  @action registerOnCloseCallback(event: Event) {
-    if (this.args.onClose && typeof this.args.onClose === 'function') {
-      this.args.onClose(event);
-    }
-
+  private _performCloseCleanup() {
     this._isOpen = false;
 
     // Reset page `overflow` property
@@ -146,7 +142,7 @@ export default class HdsFlyout extends Component<HdsFlyoutSignature> {
     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._performCloseCleanup();
       }
 
       this._element?.removeEventListener(
@@ -158,6 +154,14 @@ export default class HdsFlyout extends Component<HdsFlyoutSignature> {
     };
   });
 
+  @action registerOnCloseCallback(event: Event) {
+    if (this.args.onClose && typeof this.args.onClose === 'function') {
+      this.args.onClose(event);
+    }
+
+    this._performCloseCleanup();
+  }
+
   @action
   open(): void {
     // Make flyout dialog visible using the native `showModal` method

packages/components/src/components/hds/modal/index.ts

@@ -110,6 +110,33 @@ export default class HdsModal extends Component<HdsModalSignature> {
     return classes.join(' ');
   }
 
+  private _performCloseCleanup(): void {
+    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();
+      }
+    }
+  }
+
   @action registerOnCloseCallback(event: Event): void {
     if (
       !this.isDismissDisabled &&
@@ -129,30 +156,7 @@ export default class HdsModal extends Component<HdsModalSignature> {
         this._element.showModal();
       }
     } 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();
-        }
-      }
+      this._performCloseCleanup();
     }
   }
 
@@ -195,7 +199,7 @@ export default class HdsModal extends Component<HdsModalSignature> {
     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._performCloseCleanup();
       }
 
       this._element?.removeEventListener(

showcase/tests/integration/components/hds/flyout/index-test.js

@@ -407,6 +407,31 @@ module('Integration | Component | hds/flyout/index', function (hooks) {
     assert.ok(closed);
   });
 
+  test('it should not call `onClose` when the flyout is removed from the DOM directly', async function (assert) {
+    let closed = false;
+
+    this.set('onClose', () => (closed = true));
+    this.set('isFlyoutRendered', true);
+
+    await render(
+      hbs`
+        {{#if this.isFlyoutRendered}}
+          <Hds::Flyout id="test-modal-onclose-no-callback" @onClose={{this.onClose}} as |F|>
+            <F.Header>Title</F.Header>
+          </Hds::Flyout>
+        {{/if}}
+      `,
+    );
+
+    assert.dom('#test-modal-onclose-no-callback').isVisible();
+
+    this.set('isFlyoutRendered', false);
+    assert.dom('#test-modal-onclose-no-callback').doesNotExist();
+
+    await settled();
+    assert.notOk(closed);
+  });
+
   // ASSERTIONS
 
   test('it should throw an assertion if an incorrect value for @size is provided', async function (assert) {

showcase/tests/integration/components/hds/modal/index-test.js

@@ -430,6 +430,31 @@ module('Integration | Component | hds/modal/index', function (hooks) {
     assert.ok(closed);
   });
 
+  test('it should not call `onClose` when the modal is removed from the DOM directly', async function (assert) {
+    let closed = false;
+
+    this.set('onClose', () => (closed = true));
+    this.set('isModalRendered', true);
+
+    await render(
+      hbs`
+        {{#if this.isModalRendered}}
+          <Hds::Modal id="test-modal-onclose-no-callback" @onClose={{this.onClose}} as |M|>
+            <M.Header>Title</M.Header>
+          </Hds::Modal>
+        {{/if}}
+      `,
+    );
+
+    assert.dom('#test-modal-onclose-no-callback').isVisible();
+
+    this.set('isModalRendered', false);
+
+    await settled();
+    assert.dom('#test-modal-onclose-no-callback').doesNotExist();
+    assert.notOk(closed);
+  });
+
   // ASSERTIONS
 
   test('it should throw an assertion if an incorrect value for @size is provided', async function (assert) {