UX: Remove live theme previewing in favor of refresh (#9798)

app/assets/javascripts/discourse/app/controllers/preferences/interface.js

@@ -2,13 +2,10 @@ import I18n from "I18n";
 import { inject } from "@ember/controller";
 import Controller from "@ember/controller";
 import { setDefaultHomepage } from "discourse/lib/utilities";
-import discourseComputed, { observes } from "discourse-common/utils/decorators";
+import discourseComputed from "discourse-common/utils/decorators";
-import {
+import { listThemes, setLocalTheme } from "discourse/lib/theme-selector";
-  listThemes,
-  previewTheme,
-  setLocalTheme
-} from "discourse/lib/theme-selector";
 import { popupAjaxError } from "discourse/lib/ajax-error";
+import pageReloader from "discourse/helpers/page-reloader";
 import {
   safariHacksDisabled,
   isiPad,
@@ -28,6 +25,9 @@ const TEXT_SIZES = ["smaller", "normal", "larger", "largest"];
 const TITLE_COUNT_MODES = ["notifications", "contextual"];
 
 export default Controller.extend({
+  currentThemeId: -1,
+  preferencesController: inject("preferences"),
+
   @discourseComputed("makeThemeDefault")
   saveAttrNames(makeDefault) {
     let attrs = [
@@ -51,8 +51,6 @@ export default Controller.extend({
     return attrs;
   },
 
-  preferencesController: inject("preferences"),
-
   @discourseComputed()
   isiPad() {
     // TODO: remove this preference checkbox when iOS adoption > 90%
@@ -105,10 +103,14 @@ export default Controller.extend({
     return themes && themes.length > 1;
   },
 
-  @observes("themeId")
+  @discourseComputed("themeId")
-  themeIdChanged() {
+  themeIdChanged(themeId) {
-    const id = this.themeId;
+    if (this.currentThemeId === -1) {
-    previewTheme([id]);
+      this.set("currentThemeId", themeId);
+      return false;
+    } else {
+      return this.currentThemeId !== themeId;
+    }
   },
 
   @discourseComputed("model.user_option.theme_ids", "themeId")
@@ -189,6 +191,10 @@ export default Controller.extend({
               this.disableSafariHacks.toString()
             );
           }
+
+          if (this.themeId !== this.currentThemeId) {
+            pageReloader.reload();
+          }
         })
         .catch(popupAjaxError);
     },

app/assets/javascripts/discourse/app/helpers/page-reloader.js

@@ -0,0 +1,10 @@
+import EmberObject from "@ember/object";
+import Ember from "ember";
+
+export default EmberObject.create({
+  reload: function() {
+    if (!Ember.testing) {
+      location.reload();
+    }
+  }
+});

app/assets/javascripts/discourse/app/lib/theme-selector.js

@@ -1,5 +1,4 @@
 import I18n from "I18n";
-import { ajax } from "discourse/lib/ajax";
 import deprecated from "discourse-common/lib/deprecated";
 
 const keySelector = "meta[name=discourse_theme_ids]";
@@ -44,66 +43,6 @@ export function setLocalTheme(ids, themeSeq) {
   }
 }
 
-export function refreshCSS(node, hash, newHref) {
-  let $orig = $(node);
-
-  if ($orig.data("reloading")) {
-    clearTimeout($orig.data("timeout"));
-    $orig.data("copy").remove();
-  }
-
-  if (!$orig.data("orig")) {
-    $orig.data("orig", node.href);
-  }
-
-  $orig.data("reloading", true);
-
-  const orig = $(node).data("orig");
-
-  let reloaded = $orig.clone(true);
-  if (hash) {
-    reloaded[0].href =
-      orig + (orig.indexOf("?") >= 0 ? "&hash=" : "?hash=") + hash;
-  } else {
-    reloaded[0].href = newHref;
-  }
-
-  $orig.after(reloaded);
-
-  let timeout = setTimeout(() => {
-    $orig.remove();
-    reloaded.data("reloading", false);
-  }, 2000);
-
-  $orig.data("timeout", timeout);
-  $orig.data("copy", reloaded);
-}
-
-export function previewTheme(ids = []) {
-  ids = ids.reject(id => !id);
-  if (!ids.includes(currentThemeId())) {
-    Discourse.set("assetVersion", "forceRefresh");
-
-    ajax(`/themes/assets/${ids.length > 0 ? ids.join("-") : "default"}`).then(
-      results => {
-        const elem = _.first($(keySelector));
-        if (elem) {
-          elem.content = ids.join(",");
-        }
-
-        results.themes.forEach(theme => {
-          const node = $(
-            `link[rel=stylesheet][data-target=${theme.target}]`
-          )[0];
-          if (node) {
-            refreshCSS(node, null, theme.new_href);
-          }
-        });
-      }
-    );
-  }
-}
-
 export function listThemes(site) {
   let themes = site.get("user_themes");
 

app/assets/javascripts/discourse/app/templates/preferences/interface.hbs

@@ -5,9 +5,11 @@
       {{combo-box
         content=userSelectableThemes
         value=themeId
-        onChange=(action (mut themeId))
       }}
     </div>
+    {{#if themeIdChanged}}
+      <p class="alert alert-success save-theme-alert">{{i18n "user.save_to_change_theme" save_text=(i18n "save") }}</p>
+    {{/if}}
     {{#if showThemeSetDefault}}
       <div class="controls">
         {{preference-checkbox labelKey="user.theme_default_on_all_devices" checked=makeThemeDefault}}

app/assets/stylesheets/common/base/user.scss

@@ -653,6 +653,9 @@
     padding: 5px 0;
     font-weight: bold;
   }
+  .save-theme-alert {
+    font-size: $font-down-1;
+  }
 }
 
 .paginated-topics-list {

app/controllers/themes_controller.rb

@@ -1,31 +0,0 @@
-# frozen_string_literal: true
-
-class ThemesController < ::ApplicationController
-  def assets
-    theme_ids = params[:ids].to_s.split("-").map(&:to_i)
-
-    if params[:ids] == "default"
-      theme_ids = nil
-    else
-      raise Discourse::NotFound unless guardian.allow_themes?(theme_ids)
-    end
-
-    targets = view_context.mobile_view? ? [:mobile, :mobile_theme] : [:desktop, :desktop_theme]
-    targets << :admin if guardian.is_staff?
-    targets.append(*Discourse.find_plugin_css_assets(mobile_view: true, desktop_view: true))
-
-    object = targets.map do |target|
-      Stylesheet::Manager.stylesheet_data(target, theme_ids).map do |hash|
-        next hash unless Rails.env.development?
-
-        dup_hash = hash.dup
-        dup_hash[:new_href] = dup_hash[:new_href].dup
-        dup_hash[:new_href] << (dup_hash[:new_href].include?("?") ? "&" : "?")
-        dup_hash[:new_href] << SecureRandom.hex
-        dup_hash
-      end
-    end.flatten
-
-    render json: object.as_json
-  end
-end

config/locales/client.en.yml

@@ -963,6 +963,7 @@ en:
       api_approved: "Approved:"
       api_last_used_at: "Last used at:"
       theme: "Theme"
+      save_to_change_theme: 'Theme will be updated after you click "%{save_text}"'
       home: "Default Home Page"
       staged: "Staged"
 

config/routes.rb

@@ -950,8 +950,6 @@ Discourse::Application.routes.draw do
   get "/safe-mode" => "safe_mode#index"
   post "/safe-mode" => "safe_mode#enter", as: "safe_mode_enter"
 
-  get "/themes/assets/:ids" => "themes#assets"
-
   unless Rails.env.production?
     get "/qunit" => "qunit#index"
     get "/wizard/qunit" => "wizard#qunit"

test/javascripts/acceptance/preferences-test.js

@@ -1,7 +1,6 @@
 import I18n from "I18n";
 import { acceptance, updateCurrentUser } from "helpers/qunit-helpers";
 import selectKit from "helpers/select-kit-helper";
-
 import User from "discourse/models/user";
 
 acceptance("User Preferences", {