From b8ee52c4cb7804edce6eb3d808fc06e55fc4dbe9 Mon Sep 17 00:00:00 2001
From: Penar Musaraj <pmusaraj@gmail.com>
Date: Thu, 26 Oct 2023 08:24:47 -0400
Subject: [PATCH] UX: Wait for render before invoking A11YDialog (#24100)

Previously, focus wasn't being applied correctly on dialogs using named
components. This was because the A11YDialog was being invoked before
the component was completely rendered.

The long-term plan is to move away from A11YDialog doing the rendering
here, but for now this should do.
---
 .../controllers/admin-web-hooks-index.js      |  2 +-
 .../addon/controllers/admin-web-hooks-show.js |  2 +-
 .../admin/addon/templates/web-hooks-index.hbs |  2 +-
 .../admin/addon/templates/web-hooks-show.hbs  |  2 +-
 .../dialog-holder/addon/services/dialog.js    | 29 +++++++++----------
 .../tests/acceptance/admin-plugins-test.js    |  2 +-
 6 files changed, 18 insertions(+), 21 deletions(-)

diff --git a/app/assets/javascripts/admin/addon/controllers/admin-web-hooks-index.js b/app/assets/javascripts/admin/addon/controllers/admin-web-hooks-index.js
index c7c9420d654..87b53e2ca3c 100644
--- a/app/assets/javascripts/admin/addon/controllers/admin-web-hooks-index.js
+++ b/app/assets/javascripts/admin/addon/controllers/admin-web-hooks-index.js
@@ -16,7 +16,7 @@ export default class AdminWebHooksIndexController extends Controller {
   @alias("adminWebHooks.model") model;
 
   @action
-  destroy(webhook) {
+  destroyWebhook(webhook) {
     return this.dialog.deleteConfirm({
       message: I18n.t("admin.web_hooks.delete_confirm"),
       didConfirm: async () => {
diff --git a/app/assets/javascripts/admin/addon/controllers/admin-web-hooks-show.js b/app/assets/javascripts/admin/addon/controllers/admin-web-hooks-show.js
index 411f592d0b1..115543d3bab 100644
--- a/app/assets/javascripts/admin/addon/controllers/admin-web-hooks-show.js
+++ b/app/assets/javascripts/admin/addon/controllers/admin-web-hooks-show.js
@@ -15,7 +15,7 @@ export default class AdminWebHooksShowController extends Controller {
   }
 
   @action
-  destroy() {
+  destroyWebhook() {
     return this.dialog.deleteConfirm({
       message: I18n.t("admin.web_hooks.delete_confirm"),
       didConfirm: async () => {
diff --git a/app/assets/javascripts/admin/addon/templates/web-hooks-index.hbs b/app/assets/javascripts/admin/addon/templates/web-hooks-index.hbs
index 9db001efcbe..2af64ed2a1d 100644
--- a/app/assets/javascripts/admin/addon/templates/web-hooks-index.hbs
+++ b/app/assets/javascripts/admin/addon/templates/web-hooks-index.hbs
@@ -51,7 +51,7 @@
                 </LinkTo>
 
                 <DButton
-                  @action={{fn this.destroy webhook}}
+                  @action={{fn this.destroyWebhook webhook}}
                   @icon="times"
                   @title="delete"
                   class="destroy btn-danger"
diff --git a/app/assets/javascripts/admin/addon/templates/web-hooks-show.hbs b/app/assets/javascripts/admin/addon/templates/web-hooks-show.hbs
index 3e73d087244..a4f35ad4521 100644
--- a/app/assets/javascripts/admin/addon/templates/web-hooks-show.hbs
+++ b/app/assets/javascripts/admin/addon/templates/web-hooks-show.hbs
@@ -15,7 +15,7 @@
     />
 
     <DButton
-      @action={{this.destroy}}
+      @action={{this.destroyWebhook}}
       @icon="times"
       @title="delete"
       class="destroy btn-danger admin-webhooks__delete-button"
diff --git a/app/assets/javascripts/dialog-holder/addon/services/dialog.js b/app/assets/javascripts/dialog-holder/addon/services/dialog.js
index e7592f47789..4e8f7e4a170 100644
--- a/app/assets/javascripts/dialog-holder/addon/services/dialog.js
+++ b/app/assets/javascripts/dialog-holder/addon/services/dialog.js
@@ -1,4 +1,4 @@
-import { next } from "@ember/runloop";
+import { schedule } from "@ember/runloop";
 import Service from "@ember/service";
 import A11yDialog from "a11y-dialog";
 import { bind } from "discourse-common/utils/decorators";
@@ -49,26 +49,11 @@ export default class DialogService extends Service {
       buttons,
     } = params;
 
-    let element = document.getElementById("dialog-holder");
-    if (!element) {
-      await new Promise((resolve) => next(resolve));
-      element = document.getElementById("dialog-holder");
-    }
-
-    if (!element) {
-      const msg =
-        "dialog-holder wrapper element not found. Unable to render dialog";
-      // eslint-disable-next-line no-console
-      console.error(msg, params);
-      throw new Error(msg);
-    }
-
     this.setProperties({
       message,
       bodyComponent,
       bodyComponentModel,
       type,
-      dialogInstance: new A11yDialog(element),
 
       title,
       titleElementId: title !== null ? "dialog-title" : null,
@@ -88,6 +73,18 @@ export default class DialogService extends Service {
       class: params.class,
     });
 
+    await new Promise((resolve) => schedule("afterRender", resolve));
+    const element = document.getElementById("dialog-holder");
+
+    if (!element) {
+      const msg =
+        "dialog-holder wrapper element not found. Unable to render dialog";
+      // eslint-disable-next-line no-console
+      console.error(msg, params);
+      throw new Error(msg);
+    }
+
+    this.dialogInstance = new A11yDialog(element);
     this.dialogInstance.show();
 
     this.dialogInstance.on("hide", () => {
diff --git a/app/assets/javascripts/discourse/tests/acceptance/admin-plugins-test.js b/app/assets/javascripts/discourse/tests/acceptance/admin-plugins-test.js
index 357f42b8da4..52155780656 100644
--- a/app/assets/javascripts/discourse/tests/acceptance/admin-plugins-test.js
+++ b/app/assets/javascripts/discourse/tests/acceptance/admin-plugins-test.js
@@ -33,7 +33,7 @@ acceptance("Admin - Plugins", function (needs) {
     );
 
     server.put("/admin/site_settings/testplugin_enabled", () =>
-      helper.response()
+      helper.response(200, {})
     );
   });