From 081c36a4592c2d1285aa52fb671c20808fb929ee Mon Sep 17 00:00:00 2001 From: David Taylor Date: Mon, 16 Sep 2019 17:06:34 +0100 Subject: [PATCH] FIX: Do not include theme variables in plugin SCSS, and fix register_css --- app/models/theme_field.rb | 2 +- lib/plugin/instance.rb | 2 +- lib/stylesheet/compiler.rb | 4 +- lib/stylesheet/importer.rb | 133 +++++++++++--------- lib/stylesheet/manager.rb | 2 +- spec/components/stylesheet/compiler_spec.rb | 35 ++++++ 6 files changed, 112 insertions(+), 66 deletions(-) diff --git a/app/models/theme_field.rb b/app/models/theme_field.rb index e45bae20ac6..8d8a0c6c998 100644 --- a/app/models/theme_field.rb +++ b/app/models/theme_field.rb @@ -343,7 +343,7 @@ class ThemeField < ActiveRecord::Base end def compile_scss - Stylesheet::Compiler.compile("@import \"common/foundation/variables\"; @import \"theme_variables\"; @import \"theme_field\";", + Stylesheet::Compiler.compile("@import \"common/foundation/variables\"; @import \"common/foundation/mixins\"; @import \"theme_variables\"; @import \"theme_field\";", "theme.scss", theme_field: self.value.dup, theme: self.theme diff --git a/lib/plugin/instance.rb b/lib/plugin/instance.rb index 4a3211d028c..28a7c34c36d 100644 --- a/lib/plugin/instance.rb +++ b/lib/plugin/instance.rb @@ -266,7 +266,7 @@ class Plugin::Instance automatic_assets.each do |path, contents| write_asset(path, contents) paths << path - assets << [path] + assets << [path, nil, directory_name] end delete_extra_automatic_assets(paths) diff --git a/lib/stylesheet/compiler.rb b/lib/stylesheet/compiler.rb index 1dea3780832..f1519aca5d1 100644 --- a/lib/stylesheet/compiler.rb +++ b/lib/stylesheet/compiler.rb @@ -12,7 +12,9 @@ module Stylesheet if Importer.special_imports[asset.to_s] filename = "theme_#{options[:theme_id]}.scss" - file = "@import \"common/foundation/variables\"; @import \"common/foundation/mixins\"; @import \"theme_variables\"; @import \"#{asset}\";" + file = "@import \"common/foundation/variables\"; @import \"common/foundation/mixins\";" + file += " @import \"theme_variables\";" if Importer::THEME_TARGETS.include?(asset.to_s) + file += " @import \"#{asset}\";" else filename = "#{asset}.scss" path = "#{ASSET_ROOT}/#{filename}" diff --git a/lib/stylesheet/importer.rb b/lib/stylesheet/importer.rb index d06b4dbf0d7..5cf12e026dc 100644 --- a/lib/stylesheet/importer.rb +++ b/lib/stylesheet/importer.rb @@ -7,6 +7,8 @@ module Stylesheet class Importer < SassC::Importer include GlobalPath + THEME_TARGETS = %w{embedded_theme mobile_theme desktop_theme} + def self.special_imports @special_imports ||= {} end @@ -15,87 +17,94 @@ module Stylesheet special_imports[name] = blk end - register_import "theme_field" do - Import.new("#{theme_dir(@theme_id)}/theme_field.scss", source: @theme_field) - end + # Contained in function so that it can be called repeatedly from test mode + def self.register_imports! + @special_imports = {} - Discourse.plugins.each do |plugin| - plugin_directory_name = plugin.directory_name + register_import "theme_field" do + Import.new("#{theme_dir(@theme_id)}/theme_field.scss", source: @theme_field) + end - ["", "mobile", "desktop"].each do |type| - asset_name = type.present? ? "#{plugin_directory_name}_#{type}" : plugin_directory_name - stylesheets = type.present? ? DiscoursePluginRegistry.send("#{type}_stylesheets") : DiscoursePluginRegistry.stylesheets + Discourse.plugins.each do |plugin| + plugin_directory_name = plugin.directory_name - if stylesheets[plugin_directory_name].present? - register_import asset_name do - import_files(stylesheets[plugin_directory_name]) + ["", "mobile", "desktop"].each do |type| + asset_name = type.present? ? "#{plugin_directory_name}_#{type}" : plugin_directory_name + stylesheets = type.present? ? DiscoursePluginRegistry.send("#{type}_stylesheets") : DiscoursePluginRegistry.stylesheets + + if stylesheets[plugin_directory_name].present? + register_import asset_name do + import_files(stylesheets[plugin_directory_name]) + end end end end - end - register_import "plugins_variables" do - import_files(DiscoursePluginRegistry.sass_variables) - end - - register_import "theme_colors" do - contents = +"" - colors = (@theme_id && theme.color_scheme) ? theme.color_scheme.resolved_colors : ColorScheme.base_colors - colors.each do |n, hex| - contents << "$#{n}: ##{hex} !default;\n" + register_import "plugins_variables" do + import_files(DiscoursePluginRegistry.sass_variables) end - Import.new("theme_colors.scss", source: contents) - end - - register_import "theme_variables" do - contents = +"" - - theme&.all_theme_variables&.each do |field| - if field.type_id == ThemeField.types[:theme_upload_var] - if upload = field.upload - url = upload_cdn_path(upload.url) - contents << "$#{field.name}: unquote(\"#{url}\");\n" - end - else - contents << to_scss_variable(field.name, field.value) + register_import "theme_colors" do + contents = +"" + colors = (@theme_id && theme.color_scheme) ? theme.color_scheme.resolved_colors : ColorScheme.base_colors + colors.each do |n, hex| + contents << "$#{n}: ##{hex} !default;\n" end + + Import.new("theme_colors.scss", source: contents) end - theme&.included_settings&.each do |name, value| - next if name == "theme_uploads" - contents << to_scss_variable(name, value) + register_import "theme_variables" do + contents = +"" + + theme&.all_theme_variables&.each do |field| + if field.type_id == ThemeField.types[:theme_upload_var] + if upload = field.upload + url = upload_cdn_path(upload.url) + contents << "$#{field.name}: unquote(\"#{url}\");\n" + end + else + contents << to_scss_variable(field.name, field.value) + end + end + + theme&.included_settings&.each do |name, value| + next if name == "theme_uploads" + contents << to_scss_variable(name, value) + end + + Import.new("theme_variable.scss", source: contents) end - Import.new("theme_variable.scss", source: contents) - end + register_import "category_backgrounds" do + contents = +"" + Category.where('uploaded_background_id IS NOT NULL').each do |c| + contents << category_css(c) if c.uploaded_background&.url.present? + end - register_import "category_backgrounds" do - contents = +"" - Category.where('uploaded_background_id IS NOT NULL').each do |c| - contents << category_css(c) if c.uploaded_background&.url.present? + Import.new("category_background.scss", source: contents) end - Import.new("category_background.scss", source: contents) + register_import "embedded_theme" do + next unless @theme_id + + theme_import(:common, :embedded_scss) + end + + register_import "mobile_theme" do + next unless @theme_id + + theme_import(:mobile, :scss) + end + + register_import "desktop_theme" do + next unless @theme_id + + theme_import(:desktop, :scss) + end end - register_import "embedded_theme" do - next unless @theme_id - - theme_import(:common, :embedded_scss) - end - - register_import "mobile_theme" do - next unless @theme_id - - theme_import(:mobile, :scss) - end - - register_import "desktop_theme" do - next unless @theme_id - - theme_import(:desktop, :scss) - end + register_imports! def initialize(options) @theme = options[:theme] diff --git a/lib/stylesheet/manager.rb b/lib/stylesheet/manager.rb index 2d9a2754ebe..266c7f2717f 100644 --- a/lib/stylesheet/manager.rb +++ b/lib/stylesheet/manager.rb @@ -168,7 +168,7 @@ class Stylesheet::Manager source_map_file: source_map_filename ) rescue SassC::SyntaxError => e - if %w{embedded_theme mobile_theme desktop_theme}.include?(@target.to_s) + if Importer::THEME_TARGETS.include?(@target.to_s) # no special errors for theme, handled in theme editor ["", nil] else diff --git a/spec/components/stylesheet/compiler_spec.rb b/spec/components/stylesheet/compiler_spec.rb index a5f2a95f0d5..7443fbf6263 100644 --- a/spec/components/stylesheet/compiler_spec.rb +++ b/spec/components/stylesheet/compiler_spec.rb @@ -15,6 +15,41 @@ describe Stylesheet::Compiler do end end + context "with a theme" do + let!(:theme) { Fabricate(:theme) } + let!(:upload) { Fabricate(:upload) } + let!(:upload_theme_field) { ThemeField.create!(theme: theme, target_id: 0, name: "primary", upload: upload, value: "", type_id: ThemeField.types[:theme_upload_var]) } + let!(:stylesheet_theme_field) { ThemeField.create!(theme: theme, target_id: 0, name: "scss", value: "body { background: $primary }", type_id: ThemeField.types[:scss]) } + before { stylesheet_theme_field.save! } + + it "theme stylesheet should be able to access theme asset variables" do + css, _map = Stylesheet::Compiler.compile_asset("desktop_theme", theme_id: theme.id) + expect(css).to include(upload.url) + end + + context "with a plugin" do + before do + plugin = Plugin::Instance.new + plugin.path = "#{Rails.root}/spec/fixtures/plugins/my_plugin/plugin.rb" + plugin.register_css "body { background: $primary }" + Discourse.plugins << plugin + plugin.activate! + Stylesheet::Importer.register_imports! + end + + after do + Discourse.plugins.pop + Stylesheet::Importer.register_imports! + end + + it "does not include theme variables in plugins" do + css, _map = Stylesheet::Compiler.compile_asset("my_plugin", theme_id: theme.id) + expect(css).not_to include(upload.url) + expect(css).to include("background:") + end + end + end + it "supports asset-url" do css, _map = Stylesheet::Compiler.compile(".body{background-image: asset-url('/images/favicons/github.png');}", "test.scss")