diff --git a/lib/stylesheet/importer.rb b/lib/stylesheet/importer.rb index 1e741168c2c..b59a09735d9 100644 --- a/lib/stylesheet/importer.rb +++ b/lib/stylesheet/importer.rb @@ -152,8 +152,12 @@ module Stylesheet contents << "\n\n" end - if theme_id - Theme.list_baked_fields([theme_id], :common, :color_definitions).each do |row| + theme_id ||= SiteSetting.default_theme_id + resolved_ids = Theme.transform_ids([theme_id]) + + if resolved_ids + contents << " @import \"theme_variables\";" + Theme.list_baked_fields(resolved_ids, :common, :color_definitions).each do |row| contents << "// Color definitions from #{Theme.find_by_id(theme_id)&.name}\n\n" contents << row.value end diff --git a/lib/stylesheet/manager.rb b/lib/stylesheet/manager.rb index 3332b7e98fc..847de64c8e2 100644 --- a/lib/stylesheet/manager.rb +++ b/lib/stylesheet/manager.rb @@ -244,6 +244,10 @@ class Stylesheet::Manager if Stylesheet::Importer::THEME_TARGETS.include?(@target.to_s) # no special errors for theme, handled in theme editor ["", nil] + elsif @target.to_s == COLOR_SCHEME_STYLESHEET + # log error but do not crash for errors in color definitions SCSS + Rails.logger.error "SCSS compilation error: #{e.message}" + ["", nil] else raise Discourse::ScssError, e.message end diff --git a/spec/components/stylesheet/importer_spec.rb b/spec/components/stylesheet/importer_spec.rb index 9c95d86cb8a..e3a6312c98b 100644 --- a/spec/components/stylesheet/importer_spec.rb +++ b/spec/components/stylesheet/importer_spec.rb @@ -138,4 +138,40 @@ describe Stylesheet::Importer do end + context "#import_color_definitions" do + let(:scss) { ":root { --custom-color: green}" } + let(:scss_child) { ":root { --custom-color: red}" } + + let(:theme) do + Fabricate(:theme).tap do |t| + t.set_field(target: :common, name: "color_definitions", value: scss) + t.save! + end + end + + let(:child) { Fabricate(:theme, component: true).tap { |t| + t.set_field(target: :common, name: "color_definitions", value: scss_child) + t.save! + }} + + it "should include color definitions in the theme" do + styles = Stylesheet::Importer.import_color_definitions(theme.id) + expect(styles).to include(scss) + end + + it "should include color definitions from components" do + theme.add_relative_theme!(:child, child) + theme.save! + + styles = Stylesheet::Importer.import_color_definitions(theme.id) + expect(styles).to include(scss_child) + end + + it "should include default theme color definitions" do + SiteSetting.default_theme_id = theme.id + styles = Stylesheet::Importer.import_color_definitions(nil) + expect(styles).to include(scss) + end + + end end diff --git a/spec/components/stylesheet/manager_spec.rb b/spec/components/stylesheet/manager_spec.rb index 2ae86992b11..1a6f493df00 100644 --- a/spec/components/stylesheet/manager_spec.rb +++ b/spec/components/stylesheet/manager_spec.rb @@ -263,15 +263,28 @@ describe Stylesheet::Manager do expect(stylesheet2).to include("--primary: #c00;") end - it "includes theme color definitions in color scheme" do - theme = Fabricate(:theme) - theme.set_field(target: :common, name: :color_definitions, value: ':root {--special: rebeccapurple;}') - theme.save! + context "theme colors" do + let(:theme) { Fabricate(:theme).tap { |t| + t.set_field(target: :common, name: "color_definitions", value: ':root {--special: rebeccapurple;}') + t.save! + }} - scheme = ColorScheme.base - stylesheet = Stylesheet::Manager.new(:color_definitions, theme.id, scheme).compile + let(:scheme) { ColorScheme.base } - expect(stylesheet).to include("--special: rebeccapurple") + it "includes theme color definitions in color scheme" do + stylesheet = Stylesheet::Manager.new(:color_definitions, theme.id, scheme).compile + expect(stylesheet).to include("--special: rebeccapurple") + end + + it "fails gracefully for broken SCSS" do + scss = "$test: $missing-var;" + theme.set_field(target: :common, name: "color_definitions", value: scss) + theme.save! + + stylesheet = Stylesheet::Manager.new(:color_definitions, theme.id, scheme) + + expect { stylesheet.compile }.not_to raise_error + end end context 'encoded slugs' do