From a084c80e3dfe8f04658443ec5f01dfc5d386879a Mon Sep 17 00:00:00 2001
From: David Taylor <david@taylorhq.com>
Date: Mon, 11 Sep 2023 17:11:44 +0100
Subject: [PATCH] FIX: Ensure declarative DModals do not interfere with service
 (#23510)

While it's generally not recommended from a UX perspective, the DModal system is intended to allow multiple modals to be rendered simultaneously when using the declarative API. This wasn't working because `{{#in-element` was configured to replace the content of the container rather than append a new modal.

This commit fixes that and adds a test for the functionality.
---
 .../discourse/app/components/d-modal.hbs      |  1 +
 .../tests/acceptance/modal-service-test.gjs   | 52 +++++++++++++++++++
 2 files changed, 53 insertions(+)

diff --git a/app/assets/javascripts/discourse/app/components/d-modal.hbs b/app/assets/javascripts/discourse/app/components/d-modal.hbs
index 9a16560ca68..d91888fe2f7 100644
--- a/app/assets/javascripts/discourse/app/components/d-modal.hbs
+++ b/app/assets/javascripts/discourse/app/components/d-modal.hbs
@@ -4,6 +4,7 @@
 <ConditionalInElement
   @element={{this.modal.containerElement}}
   @inline={{@inline}}
+  @append={{true}}
 >
   <this.dynamicElement
     class={{concat-class
diff --git a/app/assets/javascripts/discourse/tests/acceptance/modal-service-test.gjs b/app/assets/javascripts/discourse/tests/acceptance/modal-service-test.gjs
index b2b4c263ed1..e296b930487 100644
--- a/app/assets/javascripts/discourse/tests/acceptance/modal-service-test.gjs
+++ b/app/assets/javascripts/discourse/tests/acceptance/modal-service-test.gjs
@@ -11,6 +11,8 @@ import DModal, {
 } from "discourse/components/d-modal";
 import { action } from "@ember/object";
 import { on } from "@ember/modifier";
+import { tracked } from "@glimmer/tracking";
+import { registerTemporaryModule } from "discourse/tests/helpers/temporary-module-helper";
 
 class MyModalClass extends Component {
   <template>
@@ -151,5 +153,55 @@ acceptance("Modal service: component-based API", function () {
     );
   });
 
+  test("alongside declarative modals", async function (assert) {
+    class State {
+      @tracked showDeclarativeModal;
+    }
+
+    const testState = new State();
+    const closeModal = () => (testState.showDeclarativeModal = false);
+
+    const MyConnector = <template>
+      {{#if testState.showDeclarativeModal}}
+        <DModal
+          class="declarative-modal"
+          @title="Declarative modal"
+          @closeModal={{closeModal}}
+        >
+          <span class="declarative-modal-content">Declarative modal content</span>
+        </DModal>
+      {{/if}}
+    </template>;
+
+    registerTemporaryModule(
+      "discourse/plugins/my-plugin/connectors/below-footer/connector-name",
+      MyConnector
+    );
+
+    await visit("/");
+
+    const modalService = getOwner(this).lookup("service:modal");
+
+    modalService.show(MyModalClass);
+    await settled();
+    assert.dom(".d-modal.service-modal").exists("modal should appear");
+
+    testState.showDeclarativeModal = true;
+    await settled();
+    assert
+      .dom(".d-modal.declarative-modal")
+      .exists("declarative modal should appear");
+    assert.dom(".d-modal.service-modal").exists("service modal should remain");
+
+    await click(".d-modal.declarative-modal .modal-close");
+    assert
+      .dom(".d-modal.declarative-modal")
+      .doesNotExist("declarative modal should close");
+    assert.dom(".d-modal.service-modal").exists("service modal should remain");
+
+    await click(".d-modal.service-modal .modal-close");
+    assert.dom(".d-modal").doesNotExist("all modals closed");
+  });
+
   // (See also, `tests/integration/component/d-modal-test.js`)
 });