UX: Show loading spinner while loading dependencies for ace-editor (#26099)

Why this change?

On a slow network, using the `AceEditor` component will result in a blob
of text being shown first before being swapped out with the `ace.js`
editor after it has completed loading.

There is also a problem when setting the theme for the editor which
would result in a "flash" as reported in
https://github.com/ajaxorg/ace/issues/3286. To avoid this, we need to
load the theme js file before displaying the editor.

What does this change do?

1. Adds a loading spinner and set the `div.ace` with a `.hidden` class.
2. Once all the relevant scripts and initialization is done, we will
   then remove the loading spinner and remove `div.ace`.

app/assets/javascripts/admin/addon/components/ace-editor.hbs

@@ -1 +1,5 @@
-<div class="ace">{{this.content}}</div>
+{{#if this.isLoading}}
+  {{loading-spinner size="small"}}
+{{else}}
+  <div class="ace">{{this.content}}</div>
+{{/if}}

app/assets/javascripts/admin/addon/components/ace-editor.js

@@ -1,5 +1,6 @@
 import Component from "@ember/component";
 import { action } from "@ember/object";
+import { next } from "@ember/runloop";
 import { classNames } from "@ember-decorators/component";
 import { observes, on } from "@ember-decorators/object";
 import $ from "jquery";
@@ -13,6 +14,7 @@ const COLOR_VARS_REGEX =
 
 @classNames("ace-wrapper")
 export default class AceEditor extends Component {
+  isLoading = true;
   mode = "css";
   disabled = false;
   htmlPlaceholder = false;
@@ -95,67 +97,78 @@ export default class AceEditor extends Component {
 
   didInsertElement() {
     super.didInsertElement(...arguments);
+
     loadScript("/javascripts/ace/ace.js").then(() => {
-      window.ace.require(["ace/ace"], (loadedAce) => {
-        loadedAce.config.set("loadWorkerFromBlob", false);
-        loadedAce.config.set("workerPath", getURL("/javascripts/ace")); // Do not use CDN for workers
+      loadScript(`/javascripts/ace/theme-${this.aceTheme}.js`).then(() => {
+        this.set("isLoading", false);
 
-        if (this.htmlPlaceholder) {
-          this._overridePlaceholder(loadedAce);
-        }
+        next(() => {
+          window.ace.require(["ace/ace"], (loadedAce) => {
+            loadedAce.config.set("loadWorkerFromBlob", false);
+            loadedAce.config.set("workerPath", getURL("/javascripts/ace")); // Do not use CDN for workers
 
-        if (!this.element || this.isDestroying || this.isDestroyed) {
-          return;
-        }
-        const editor = loadedAce.edit(this.element.querySelector(".ace"));
+            if (this.htmlPlaceholder) {
+              this._overridePlaceholder(loadedAce);
+            }
 
-        editor.setShowPrintMargin(false);
-        editor.setOptions({ fontSize: "14px", placeholder: this.placeholder });
-        editor.getSession().setMode("ace/mode/" + this.mode);
-        editor.on("change", () => {
-          this._skipContentChangeEvent = true;
-          this.set("content", editor.getSession().getValue());
-        });
-        if (this.save) {
-          editor.commands.addCommand({
-            name: "save",
-            exec: () => {
-              this.save();
-            },
-            bindKey: { mac: "cmd-s", win: "ctrl-s" },
+            if (!this.element || this.isDestroying || this.isDestroyed) {
+              return;
+            }
+            const aceElement = this.element.querySelector(".ace");
+            const editor = loadedAce.edit(aceElement);
+            editor.setShowPrintMargin(false);
+            editor.setOptions({
+              fontSize: "14px",
+              placeholder: this.placeholder,
+            });
+            editor.getSession().setMode("ace/mode/" + this.mode);
+            editor.on("change", () => {
+              this._skipContentChangeEvent = true;
+              this.set("content", editor.getSession().getValue());
+            });
+            if (this.save) {
+              editor.commands.addCommand({
+                name: "save",
+                exec: () => {
+                  this.save();
+                },
+                bindKey: { mac: "cmd-s", win: "ctrl-s" },
+              });
+            }
+
+            editor.on("blur", () => {
+              this.warnSCSSDeprecations();
+            });
+
+            editor.$blockScrolling = Infinity;
+            editor.renderer.setScrollMargin(10, 10);
+
+            this.element.setAttribute("data-editor", editor);
+            this._editor = editor;
+            this.changeDisabledState();
+            this.warnSCSSDeprecations();
+
+            $(window)
+              .off("ace:resize")
+              .on("ace:resize", () => this.appEvents.trigger("ace:resize"));
+
+            if (this.appEvents) {
+              // xxx: don't run during qunit tests
+              this.appEvents.on("ace:resize", this, "resize");
+            }
+
+            if (this.autofocus) {
+              this.send("focus");
+            }
+
+            this.setAceTheme();
+
+            this._darkModeListener = window.matchMedia(
+              "(prefers-color-scheme: dark)"
+            );
+            this._darkModeListener.addListener(this.setAceTheme);
           });
-        }
-
-        editor.on("blur", () => {
-          this.warnSCSSDeprecations();
         });
-
-        editor.$blockScrolling = Infinity;
-        editor.renderer.setScrollMargin(10, 10);
-
-        this.element.setAttribute("data-editor", editor);
-        this._editor = editor;
-        this.changeDisabledState();
-        this.warnSCSSDeprecations();
-
-        $(window)
-          .off("ace:resize")
-          .on("ace:resize", () => this.appEvents.trigger("ace:resize"));
-
-        if (this.appEvents) {
-          // xxx: don't run during qunit tests
-          this.appEvents.on("ace:resize", this, "resize");
-        }
-
-        if (this.autofocus) {
-          this.send("focus");
-        }
-
-        this.setAceTheme();
-        this._darkModeListener = window.matchMedia(
-          "(prefers-color-scheme: dark)"
-        );
-        this._darkModeListener.addListener(this.setAceTheme);
       });
     });
   }
@@ -165,15 +178,17 @@ export default class AceEditor extends Component {
     this._darkModeListener?.removeListener(this.setAceTheme);
   }
 
-  @bind
-  setAceTheme() {
+  get aceTheme() {
     const schemeType = getComputedStyle(document.body)
       .getPropertyValue("--scheme-type")
       .trim();
 
-    this._editor.setTheme(
-      `ace/theme/${schemeType === "dark" ? "chaos" : "chrome"}`
-    );
+    return schemeType === "dark" ? "chaos" : "chrome";
+  }
+
+  @bind
+  setAceTheme() {
+    this._editor.setTheme(`ace/theme/${this.aceTheme}`);
   }
 
   warnSCSSDeprecations() {

app/assets/javascripts/discourse/app/lib/public-js-versions.js

@@ -3,6 +3,8 @@
 
 export const PUBLIC_JS_VERSIONS = {
   "ace/ace.js": "ace.js/1.4.13/ace.js",
+  "ace/theme-chrome.js": "ace.js/1.4.13/theme-chrome.js",
+  "ace/theme-chaos.js": "ace.js/1.4.13/theme-chaos.js",
   "jsoneditor.js": "@json-editor/json-editor/2.10.0/jsoneditor.js",
   "chart.min.js": "chart.js/3.5.1/chart.min.js",
   "chartjs-plugin-datalabels.min.js":

app/assets/javascripts/discourse/tests/integration/components/admin-theme-settings-editor-test.js

@@ -1,4 +1,4 @@
-import { render } from "@ember/test-helpers";
+import { render, waitUntil } from "@ember/test-helpers";
 import { hbs } from "ember-cli-htmlbars";
 import { module, test } from "qunit";
 import { setupRenderingTest } from "discourse/tests/helpers/component-test";
@@ -56,9 +56,10 @@ module(
           )
         )
     }} />`);
-      const lines = document.querySelectorAll(".ace_line");
-      const indexOf = lines[0].innerHTML.indexOf("[");
-      assert.ok(indexOf >= 0);
+
+      await waitUntil(() => document.querySelectorAll(".ace_line")[0]);
+
+      assert.dom(".ace_line").hasText("[");
     });
 
     test("input is valid json", async function (assert) {

lib/tasks/javascript.rake

@@ -232,28 +232,30 @@ task "javascript:update" => "clean_up" do
         dest = "#{path}/#{filename}"
 
         FileUtils.mkdir_p(path) unless File.exist?(path)
+
+        if src.include? "ace.js"
+          versions["ace/ace.js"] = versions.delete("ace.js")
+
+          themes = %w[theme-chrome theme-chaos]
+
+          themes.each do |file|
+            versions["ace/#{file}.js"] = "#{package_dir_name}/#{package_version}/#{file}.js"
+          end
+
+          ace_root = "#{library_src}/ace-builds/src-min-noconflict/"
+
+          addtl_files = %w[ext-searchbox mode-html mode-scss mode-sql mode-yaml worker-html].concat(
+            themes,
+          )
+
+          dest_path = dest.split("/")[0..-2].join("/")
+          addtl_files.each { |file| FileUtils.cp_r("#{ace_root}#{file}.js", dest_path) }
+        end
       end
     else
       dest = "#{vendor_js}/#{filename}"
     end
 
-    if src.include? "ace.js"
-      versions["ace/ace.js"] = versions.delete("ace.js")
-      ace_root = "#{library_src}/ace-builds/src-min-noconflict/"
-      addtl_files = %w[
-        ext-searchbox
-        mode-html
-        mode-scss
-        mode-sql
-        mode-yaml
-        theme-chrome
-        theme-chaos
-        worker-html
-      ]
-      dest_path = dest.split("/")[0..-2].join("/")
-      addtl_files.each { |file| FileUtils.cp_r("#{ace_root}#{file}.js", dest_path) }
-    end
-
     STDERR.puts "New dependency added: #{dest}" unless File.exist?(dest)
 
     FileUtils.cp_r(src, dest)