FIX: Modal `onClose` was being called repeatedly

FIX: Modal onClose was being called repeatedly

This happened because the modal controller was not clearing the name attribute, which is used for looking up the controller to call onClose on.

Every page navigation would call the method over and over, breaking state in odd ways.

diff --git a/app/assets/javascripts/discourse/routes/application.js.es6 b/app/assets/javascripts/discourse/routes/application.js.es6
index 4dbc79f..f9cdcab 100644
--- a/app/assets/javascripts/discourse/routes/application.js.es6
+++ b/app/assets/javascripts/discourse/routes/application.js.es6
@@ -156,10 +156,17 @@ const ApplicationRoute = Discourse.Route.extend(OpenComposer, {
       this.render("hide-modal", { into: "modal", outlet: "modalBody" });
 
       const route = getOwner(this).lookup("route:application");
-      const name = route.controllerFor("modal").get("name");
-      const controller = getOwner(this).lookup(`controller:${name}`);
-      if (controller && controller.onClose) {
-        controller.onClose();
+      let modalController = route.controllerFor("modal");
+      const controllerName = modalController.get("name");
+
+      if (controllerName) {
+        const controller = getOwner(this).lookup(
+          `controller:${controllerName}`
+        );
+        if (controller && controller.onClose) {
+          controller.onClose();
+        }
+        modalController.set("name", null);
       }
     },
 
diff --git a/test/javascripts/acceptance/modal-test.js.es6 b/test/javascripts/acceptance/modal-test.js.es6
index 11754b8..c15d23b 100644
--- a/test/javascripts/acceptance/modal-test.js.es6
+++ b/test/javascripts/acceptance/modal-test.js.es6
@@ -1,9 +1,9 @@
-import { acceptance } from "helpers/qunit-helpers";
+import { acceptance, controllerFor } from "helpers/qunit-helpers";
 import showModal from "discourse/lib/show-modal";
 
 acceptance("Modal");
 
-QUnit.test("modal", async assert => {
+QUnit.test("modal", async function(assert) {
   await visit("/");
 
   assert.ok(
@@ -14,11 +14,15 @@ QUnit.test("modal", async assert => {
   await click(".login-button");
   assert.ok(find(".d-modal:visible").length === 1, "modal should appear");
 
+  let controller = controllerFor("modal");
+  assert.equal(controller.name, "login");
+
   await click(".modal-outer-container");
   assert.ok(
     find(".d-modal:visible").length === 0,
     "modal should disappear when you click outside"
   );
+  assert.equal(controller.name, null);
 
   await click(".login-button");
   assert.ok(find(".d-modal:visible").length === 1, "modal should reappear");

GitHub sha: f57fdee2

2 Likes