FEATURE: Multiple SCSS file support for themes (#7351)

FEATURE: Multiple SCSS file support for themes (#7351)

Theme developers can include any number of scss files within the /scss/ directory of a theme. These can then be imported from the main common/desktop/mobile scss.

diff --git a/app/assets/javascripts/admin/components/admin-theme-editor.js.es6 b/app/assets/javascripts/admin/components/admin-theme-editor.js.es6
index c9d4c80..19e5cc3 100644
--- a/app/assets/javascripts/admin/components/admin-theme-editor.js.es6
+++ b/app/assets/javascripts/admin/components/admin-theme-editor.js.es6
@@ -27,6 +27,7 @@ export default Ember.Component.extend({
   @computed("currentTargetName", "fieldName")
   activeSectionMode(targetName, fieldName) {
     if (["settings", "translations"].includes(targetName)) return "yaml";
+    if (["extra_scss"].includes(targetName)) return "scss";
     return fieldName && fieldName.indexOf("scss") > -1 ? "scss" : "html";
   },
 
@@ -73,7 +74,7 @@ export default Ember.Component.extend({
 
     addField(name) {
       if (!name) return;
-      name = name.replace(/\W/g, "");
+      name = name.replace(/[^a-zA-Z0-9-_/]/g, "");
       this.get("theme").setField(this.get("currentTargetName"), name, "");
       this.setProperties({ newFieldName: "", addingField: false });
       this.fieldAdded(this.get("currentTargetName"), name);
diff --git a/app/assets/javascripts/admin/models/theme.js.es6 b/app/assets/javascripts/admin/models/theme.js.es6
index d72ac47..0e2ff37 100644
--- a/app/assets/javascripts/admin/models/theme.js.es6
+++ b/app/assets/javascripts/admin/models/theme.js.es6
@@ -27,6 +27,13 @@ const Theme = RestModel.extend({
         icon: "globe",
         advanced: true,
         customNames: true
+      },
+      {
+        id: 5,
+        name: "extra_scss",
+        icon: "paint-brush",
+        advanced: true,
+        customNames: true
       }
     ].map(target => {
       target["edited"] = this.hasEdited(target.name);
@@ -46,6 +53,14 @@ const Theme = RestModel.extend({
       "footer"
     ];
 
+    const scss_fields = (this.get("theme_fields") || [])
+      .filter(f => f.target === "extra_scss" && f.name !== "")
+      .map(f => f.name);
+
+    if (scss_fields.length < 1) {
+      scss_fields.push("importable_scss");
+    }
+
     return {
       common: [...common, "embedded_scss"],
       desktop: common,
@@ -56,7 +71,8 @@ const Theme = RestModel.extend({
         ...(this.get("theme_fields") || [])
           .filter(f => f.target === "translations" && f.name !== "en")
           .map(f => f.name)
-      ]
+      ],
+      extra_scss: scss_fields
     };
   },
 
@@ -71,7 +87,7 @@ const Theme = RestModel.extend({
           error: this.hasError(target, fieldName)
         };
 
-        if (target === "translations") {
+        if (target === "translations" || target === "extra_scss") {
           field.translatedName = fieldName;
         } else {
           field.translatedName = I18n.t(
diff --git a/app/jobs/onceoff/rebake_all_html_theme_fields.rb b/app/jobs/onceoff/rebake_all_html_theme_fields.rb
deleted file mode 100644
index afdce8a..0000000
--- a/app/jobs/onceoff/rebake_all_html_theme_fields.rb
+++ /dev/null
@@ -1,11 +0,0 @@
-module Jobs
-  class RebakeAllHtmlThemeFields < Jobs::Onceoff
-    def execute_onceoff(args)
-      ThemeField.where(type_id: ThemeField.types[:html]).find_each do |theme_field|
-        theme_field.update(value_baked: nil)
-      end
-
-      Theme.clear_cache!
-    end
-  end
-end
diff --git a/app/models/remote_theme.rb b/app/models/remote_theme.rb
index 363656c..895cc17 100644
--- a/app/models/remote_theme.rb
+++ b/app/models/remote_theme.rb
@@ -124,13 +124,14 @@ class RemoteTheme < ActiveRecord::Base
     end
 
     theme_info = RemoteTheme.extract_theme_info(importer)
+    updated_fields = []
 
     theme_info["assets"]&.each do |name, relative_path|
       if path = importer.real_path(relative_path)
         new_path = "#{File.dirname(path)}/#{SecureRandom.hex}#{File.extname(path)}"
         File.rename(path, new_path) # OptimizedImage has strict file name restrictions, so rename temporarily
         upload = UploadCreator.new(File.open(new_path), File.basename(relative_path), for_theme: true).create_for(theme.user_id)
-        theme.set_field(target: :common, name: name, type: :theme_upload_var, upload_id: upload.id)
+        updated_fields << theme.set_field(target: :common, name: name, type: :theme_upload_var, upload_id: upload.id)
       end
     end
 
@@ -144,9 +145,13 @@ class RemoteTheme < ActiveRecord::Base
     importer.all_files.each do |filename|
       next unless opts = ThemeField.opts_from_file_path(filename)
       value = importer[filename]
-      theme.set_field(opts.merge(value: value))
+      updated_fields << theme.set_field(opts.merge(value: value))
     end
 
+    # Destroy fields that no longer exist in the remote theme
+    field_ids_to_destroy = theme.theme_fields.pluck(:id) - updated_fields.map(&:id)
+    ThemeField.where(id: field_ids_to_destroy).destroy_all
+
     if !skip_update
       self.remote_updated_at = Time.zone.now
       self.remote_version = importer.version
diff --git a/app/models/theme.rb b/app/models/theme.rb
index 7a2c3c1..f8c486a 100644
--- a/app/models/theme.rb
+++ b/app/models/theme.rb
@@ -51,6 +51,10 @@ class Theme < ActiveRecord::Base
 
     Theme.expire_site_cache! if saved_change_to_user_selectable? || saved_change_to_name?
 
+    reload
+    settings_field&.ensure_baked! # Other fields require setting to be **baked**
+    theme_fields.each(&:ensure_baked!)
+
     remove_from_cache!
     clear_cached_settings!
     ColorScheme.hex_cache.clear
@@ -76,6 +80,8 @@ class Theme < ActiveRecord::Base
 
     Theme.expire_site_cache!
     ColorScheme.hex_cache.clear
+    CSP::Extension.clear_theme_extensions_cache!
+    SvgSprite.expire_cache
   end
 
   after_commit ->(theme) do
@@ -224,7 +230,7 @@ class Theme < ActiveRecord::Base
   end
 
   def self.targets
-    @targets ||= Enum.new(common: 0, desktop: 1, mobile: 2, settings: 3, translations: 4)
+    @targets ||= Enum.new(common: 0, desktop: 1, mobile: 2, settings: 3, translations: 4, extra_scss: 5)
   end
 
   def self.lookup_target(target_id)
@@ -267,15 +273,15 @@ class Theme < ActiveRecord::Base
 
   def self.list_baked_fields(theme_ids, target, name)
     target = target.to_sym
-    name = name.to_sym
+    name = name&.to_sym
 
     if target == :translations
       fields = ThemeField.find_first_locale_fields(theme_ids, I18n.fallbacks[name])
     else
       fields = ThemeField.find_by_theme_ids(theme_ids)
         .where(target_id: [Theme.targets[target], Theme.targets[:common]])
-        .where(name: name.to_s)
-        .order(:target_id)
+      fields = fields.where(name: name.to_s) unless name.nil?
+      fields = fields.order(:target_id)
     end
 
     fields.each(&:ensure_baked!)
@@ -325,6 +331,7 @@ class Theme < ActiveRecord::Base
           changed_fields << field
         end
       end
+      field
     else
       theme_fields.build(target_id: target_id, value: value, name: name, type_id: type_id, upload_id: upload_id) if value.present? || upload_id.present?
     end
diff --git a/app/models/theme_field.rb b/app/models/theme_field.rb
index 5a1e36c..c1fe2b7 100644
--- a/app/models/theme_field.rb
+++ b/app/models/theme_field.rb
@@ -7,11 +7,6 @@ class ThemeField < ActiveRecord::Base
   belongs_to :upload
   has_one :javascript_cache, dependent: :destroy
 
-  after_commit do |field|
-    SvgSprite.expire_cache if field.target_id == Theme.targets[:settings]
-    SvgSprite.expire_cache if field.name == SvgSprite.theme_sprite_variable_name
-  end
-
   scope :find_by_theme_ids, ->(theme_ids) {
     return none unless theme_ids.present?
 
@@ -221,18 +216,16 @@ class ThemeField < ActiveRecord::Base
     end
 
     self.error = errors.join("\n").presence
-    if !self.error && self.target_id == Theme.targets[:settings]
-      # when settings YAML changes, we need to re-transpile theme JS and CSS
-      theme.theme_fields.where.not(id: self.id).update_all(value_baked: nil)
-    end
   end
 
   def self.guess_type(name:, target:)
-    if html_fields.include?(name.to_s)
+    if basic_targets.include?(target.to_s) && html_fields.include?(name.to_s)
       types[:html]

[... diff too long, it was truncated ...]

GitHub sha: 268d4d4c

2 Likes

This commit has been mentioned on Discourse Meta. There might be relevant details there:

Minor but I think we can do this in a single loop with Array.prototype.reduce(). Just something to note for the future.