From 4bc769cac083dd90c2c312e691dca91c65725ee7 Mon Sep 17 00:00:00 2001
From: David Taylor <david@taylorhq.com>
Date: Mon, 15 May 2023 20:48:00 +0100
Subject: [PATCH] DEV: Control modal 'hidden' with Ember (#21562)

Moving all control of 'hidden' into Ember will resolve issues we're seeing with Ember fighting against manual DOM manipulation (both vanilla JS and JQuery).

Looking up `controller:modal` from components is not ideal. However, the next step in the refactoring is to create a modal 'service' which will be able to injected into components cleanly.
---
 .../discourse/app/components/d-modal-body.js        |  2 ++
 .../discourse/app/components/d-modal.hbs            |  1 +
 .../javascripts/discourse/app/components/d-modal.js | 13 ++++++++-----
 .../discourse/app/components/hide-modal-trigger.js  |  2 +-
 .../javascripts/discourse/app/controllers/modal.js  |  6 +++++-
 .../javascripts/discourse/app/routes/application.js |  1 +
 .../javascripts/discourse/app/templates/modal.hbs   |  2 +-
 7 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/app/assets/javascripts/discourse/app/components/d-modal-body.js b/app/assets/javascripts/discourse/app/components/d-modal-body.js
index ffcac5146b7..bb6f9c857f4 100644
--- a/app/assets/javascripts/discourse/app/components/d-modal-body.js
+++ b/app/assets/javascripts/discourse/app/components/d-modal-body.js
@@ -3,6 +3,7 @@ import { disableImplicitInjections } from "discourse/lib/implicit-injections";
 import { action } from "@ember/object";
 import { tracked } from "@glimmer/tracking";
 import { inject as service } from "@ember/service";
+import { getOwner } from "@ember/application";
 
 function pick(object, keys) {
   const result = {};
@@ -28,6 +29,7 @@ export default class DModalBody extends Component {
     if (fixedParent) {
       this.fixed = true;
       $(fixedParent).modal("show");
+      getOwner(this).lookup("controller:modal").hidden = false;
     }
 
     this.appEvents.trigger(
diff --git a/app/assets/javascripts/discourse/app/components/d-modal.hbs b/app/assets/javascripts/discourse/app/components/d-modal.hbs
index 8d341df8e4f..1f4f436df40 100644
--- a/app/assets/javascripts/discourse/app/components/d-modal.hbs
+++ b/app/assets/javascripts/discourse/app/components/d-modal.hbs
@@ -8,6 +8,7 @@
     this.modalClass
     this.modalStyle
     (if this.hasPanels "has-panels")
+    (if @hidden "hidden")
   }}
   id={{if (not-eq this.modalStyle "inline-modal") "discourse-modal"}}
   data-keyboard="false"
diff --git a/app/assets/javascripts/discourse/app/components/d-modal.js b/app/assets/javascripts/discourse/app/components/d-modal.js
index 59e498c0266..c7ace74afe2 100644
--- a/app/assets/javascripts/discourse/app/components/d-modal.js
+++ b/app/assets/javascripts/discourse/app/components/d-modal.js
@@ -6,6 +6,7 @@ import { disableImplicitInjections } from "discourse/lib/implicit-injections";
 import { inject as service } from "@ember/service";
 import { action } from "@ember/object";
 import { tracked } from "@glimmer/tracking";
+import { getOwner } from "@ember/application";
 
 @disableImplicitInjections
 export default class DModal extends Component {
@@ -146,19 +147,21 @@ export default class DModal extends Component {
     }
 
     if (data.fixed) {
-      this.wrapperElement.classList.remove("hidden");
+      getOwner(this).lookup("controller:modal").hidden = false;
     }
 
     this.modalBodyData = data;
 
-    schedule("afterRender", () => {
-      this._trapTab();
+    next(() => {
+      schedule("afterRender", () => {
+        this._trapTab();
+      });
     });
   }
 
   @bind
   _handleModalEvents(event) {
-    if (this.wrapperElement.classList.contains("hidden")) {
+    if (this.args.hidden) {
       return;
     }
 
@@ -177,7 +180,7 @@ export default class DModal extends Component {
   }
 
   _trapTab(event) {
-    if (this.wrapperElement.classList.contains("hidden")) {
+    if (this.args.hidden) {
       return true;
     }
 
diff --git a/app/assets/javascripts/discourse/app/components/hide-modal-trigger.js b/app/assets/javascripts/discourse/app/components/hide-modal-trigger.js
index 26b127301d9..9bafc1b39c6 100644
--- a/app/assets/javascripts/discourse/app/components/hide-modal-trigger.js
+++ b/app/assets/javascripts/discourse/app/components/hide-modal-trigger.js
@@ -2,6 +2,6 @@ import Component from "@ember/component";
 export default Component.extend({
   didInsertElement() {
     this._super(...arguments);
-    $(".d-modal.fixed-modal").modal("hide").addClass("hidden");
+    $(".d-modal.fixed-modal").modal("hide");
   },
 });
diff --git a/app/assets/javascripts/discourse/app/controllers/modal.js b/app/assets/javascripts/discourse/app/controllers/modal.js
index cf6c4e3aa2e..f2530f868bf 100644
--- a/app/assets/javascripts/discourse/app/controllers/modal.js
+++ b/app/assets/javascripts/discourse/app/controllers/modal.js
@@ -1,2 +1,6 @@
 import Controller from "@ember/controller";
-export default Controller.extend();
+import { tracked } from "@glimmer/tracking";
+
+export default class ModalController extends Controller {
+  @tracked hidden;
+}
diff --git a/app/assets/javascripts/discourse/app/routes/application.js b/app/assets/javascripts/discourse/app/routes/application.js
index cef44d7c4e5..4328e0bbc1b 100644
--- a/app/assets/javascripts/discourse/app/routes/application.js
+++ b/app/assets/javascripts/discourse/app/routes/application.js
@@ -190,6 +190,7 @@ const ApplicationRoute = DiscourseRoute.extend(OpenComposer, {
         }
         modalController.set("name", null);
       }
+      modalController.hidden = true;
     },
 
     /**
diff --git a/app/assets/javascripts/discourse/app/templates/modal.hbs b/app/assets/javascripts/discourse/app/templates/modal.hbs
index 8825264bd4d..782924904f7 100644
--- a/app/assets/javascripts/discourse/app/templates/modal.hbs
+++ b/app/assets/javascripts/discourse/app/templates/modal.hbs
@@ -6,7 +6,7 @@
   @panels={{this.panels}}
   @selectedPanel={{this.selectedPanel}}
   @onSelectPanel={{this.onSelectPanel}}
-  @class="hidden"
+  @hidden={{this.hidden}}
   @errors={{this.errors}}
   @closeModal={{route-action "closeModal"}}
 >