From 1a5bcf2a648caf3d277ae2b0856a566789c55e3f Mon Sep 17 00:00:00 2001 From: Mark VanLandingham Date: Thu, 21 May 2020 08:32:50 -0500 Subject: [PATCH] UX: Remove live theme previewing in favor of refresh (#9798) --- .../app/controllers/preferences/interface.js | 30 +++++---- .../discourse/app/helpers/page-reloader.js | 10 +++ .../discourse/app/lib/theme-selector.js | 61 ------------------- .../app/templates/preferences/interface.hbs | 4 +- app/assets/stylesheets/common/base/user.scss | 3 + app/controllers/themes_controller.rb | 31 ---------- config/locales/client.en.yml | 1 + config/routes.rb | 2 - .../acceptance/preferences-test.js | 1 - 9 files changed, 35 insertions(+), 108 deletions(-) create mode 100644 app/assets/javascripts/discourse/app/helpers/page-reloader.js delete mode 100644 app/controllers/themes_controller.rb diff --git a/app/assets/javascripts/discourse/app/controllers/preferences/interface.js b/app/assets/javascripts/discourse/app/controllers/preferences/interface.js index 1ad85b7d7b..2fcc5781c3 100644 --- a/app/assets/javascripts/discourse/app/controllers/preferences/interface.js +++ b/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 { - listThemes, - previewTheme, - setLocalTheme -} from "discourse/lib/theme-selector"; +import discourseComputed from "discourse-common/utils/decorators"; +import { listThemes, 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") - themeIdChanged() { - const id = this.themeId; - previewTheme([id]); + @discourseComputed("themeId") + themeIdChanged(themeId) { + if (this.currentThemeId === -1) { + 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); }, diff --git a/app/assets/javascripts/discourse/app/helpers/page-reloader.js b/app/assets/javascripts/discourse/app/helpers/page-reloader.js new file mode 100644 index 0000000000..6429be3a3c --- /dev/null +++ b/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(); + } + } +}); diff --git a/app/assets/javascripts/discourse/app/lib/theme-selector.js b/app/assets/javascripts/discourse/app/lib/theme-selector.js index 9d293fe312..2eb4912c9c 100644 --- a/app/assets/javascripts/discourse/app/lib/theme-selector.js +++ b/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"); diff --git a/app/assets/javascripts/discourse/app/templates/preferences/interface.hbs b/app/assets/javascripts/discourse/app/templates/preferences/interface.hbs index 45607c8cb7..4f1f13c614 100644 --- a/app/assets/javascripts/discourse/app/templates/preferences/interface.hbs +++ b/app/assets/javascripts/discourse/app/templates/preferences/interface.hbs @@ -5,9 +5,11 @@ {{combo-box content=userSelectableThemes value=themeId - onChange=(action (mut themeId)) }} + {{#if themeIdChanged}} +

{{i18n "user.save_to_change_theme" save_text=(i18n "save") }}

+ {{/if}} {{#if showThemeSetDefault}}
{{preference-checkbox labelKey="user.theme_default_on_all_devices" checked=makeThemeDefault}} diff --git a/app/assets/stylesheets/common/base/user.scss b/app/assets/stylesheets/common/base/user.scss index 587af3c353..4d5e9f4288 100644 --- a/app/assets/stylesheets/common/base/user.scss +++ b/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 { diff --git a/app/controllers/themes_controller.rb b/app/controllers/themes_controller.rb deleted file mode 100644 index 6520ec7097..0000000000 --- a/app/controllers/themes_controller.rb +++ /dev/null @@ -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 diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 0da7d67342..3c3618ef96 100644 --- a/config/locales/client.en.yml +++ b/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" diff --git a/config/routes.rb b/config/routes.rb index e3e7731bb2..69ade71938 100644 --- a/config/routes.rb +++ b/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" diff --git a/test/javascripts/acceptance/preferences-test.js b/test/javascripts/acceptance/preferences-test.js index a9db1f08e5..20afdcd167 100644 --- a/test/javascripts/acceptance/preferences-test.js +++ b/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", {