From 882b0aac19cf604e0a89b64d00df7ec116e5f4ea Mon Sep 17 00:00:00 2001
From: Penar Musaraj <pmusaraj@gmail.com>
Date: Tue, 18 Aug 2020 13:02:13 -0400
Subject: [PATCH] DEV: Let themes extend color definitions (#10429)
Themes can now declare custom colors that get compiled in core's color definitions stylesheet, thus allowing themes to better support dark/light color schemes.
For example, if you need your theme to use tertiary for an element in a light color scheme and quaternary in a dark scheme, you can add the following SCSS to your theme's `color_definitions.scss` file:
```
:root {
--mytheme-tertiary-or-quaternary: #{dark-light-choose($tertiary, $quaternary)};
}
```
And then use the `--mytheme-tertiary-or-quaternary` variable as the color property of that element. You can also use this file to add color variables that use SCSS color transformation functions (lighten, darken, saturate, etc.) without compromising your theme's compatibility with different color schemes.
---
.../admin/components/ace-editor.js | 12 +++++-
.../admin/components/admin-theme-editor.js | 9 ++++
app/assets/javascripts/admin/models/theme.js | 2 +-
.../components/admin-theme-editor.hbs | 2 +-
.../stylesheets/common/admin/customize.scss | 6 +++
app/helpers/application_helper.rb | 2 +-
app/models/color_scheme.rb | 2 +-
app/models/theme_field.rb | 5 ++-
config/locales/client.en.yml | 4 ++
lib/stylesheet/compiler.rb | 2 +-
lib/stylesheet/importer.rb | 10 ++++-
lib/stylesheet/manager.rb | 43 +++++++++----------
spec/components/stylesheet/manager_spec.rb | 29 ++++++++++++-
spec/models/color_scheme_spec.rb | 3 ++
spec/models/remote_theme_spec.rb | 7 ++-
15 files changed, 101 insertions(+), 37 deletions(-)
diff --git a/app/assets/javascripts/admin/components/ace-editor.js b/app/assets/javascripts/admin/components/ace-editor.js
index 407b0fed603..f4868546757 100644
--- a/app/assets/javascripts/admin/components/ace-editor.js
+++ b/app/assets/javascripts/admin/components/ace-editor.js
@@ -33,6 +33,15 @@ export default Component.extend({
}
},
+ @observes("placeholder")
+ placeholderChanged() {
+ if (this._editor) {
+ this._editor.setOptions({
+ placeholder: this.placeholder
+ });
+ }
+ },
+
@observes("disabled")
disabledStateChanged() {
this.changeDisabledState();
@@ -72,7 +81,6 @@ export default Component.extend({
didInsertElement() {
this._super(...arguments);
-
loadScript("/javascripts/ace/ace.js").then(() => {
window.ace.require(["ace/ace"], loadedAce => {
loadedAce.config.set("loadWorkerFromBlob", false);
@@ -85,7 +93,7 @@ export default Component.extend({
editor.setTheme("ace/theme/chrome");
editor.setShowPrintMargin(false);
- editor.setOptions({ fontSize: "14px" });
+ editor.setOptions({ fontSize: "14px", placeholder: this.placeholder });
editor.getSession().setMode("ace/mode/" + this.mode);
editor.on("change", () => {
this._skipContentChangeEvent = true;
diff --git a/app/assets/javascripts/admin/components/admin-theme-editor.js b/app/assets/javascripts/admin/components/admin-theme-editor.js
index 0299c047158..0ae1d1491b0 100644
--- a/app/assets/javascripts/admin/components/admin-theme-editor.js
+++ b/app/assets/javascripts/admin/components/admin-theme-editor.js
@@ -1,3 +1,4 @@
+import I18n from "I18n";
import { next } from "@ember/runloop";
import Component from "@ember/component";
import discourseComputed from "discourse-common/utils/decorators";
@@ -30,9 +31,17 @@ export default Component.extend({
activeSectionMode(targetName, fieldName) {
if (["settings", "translations"].includes(targetName)) return "yaml";
if (["extra_scss"].includes(targetName)) return "scss";
+ if (["color_definitions"].includes(fieldName)) return "scss";
return fieldName && fieldName.indexOf("scss") > -1 ? "scss" : "html";
},
+ @discourseComputed("currentTargetName", "fieldName")
+ placeholder(targetName, fieldName) {
+ return fieldName && fieldName === "color_definitions"
+ ? I18n.t("admin.customize.theme.color_definitions.placeholder")
+ : "";
+ },
+
@discourseComputed("fieldName", "currentTargetName", "theme")
activeSection: {
get(fieldName, target, model) {
diff --git a/app/assets/javascripts/admin/models/theme.js b/app/assets/javascripts/admin/models/theme.js
index eb2aecfd602..06d5faeefc2 100644
--- a/app/assets/javascripts/admin/models/theme.js
+++ b/app/assets/javascripts/admin/models/theme.js
@@ -72,7 +72,7 @@ const Theme = RestModel.extend({
}
return {
- common: [...common, "embedded_scss"],
+ common: [...common, "embedded_scss", "color_definitions"],
desktop: common,
mobile: common,
settings: ["yaml"],
diff --git a/app/assets/javascripts/admin/templates/components/admin-theme-editor.hbs b/app/assets/javascripts/admin/templates/components/admin-theme-editor.hbs
index 3ebd8f2c866..ce928108a21 100644
--- a/app/assets/javascripts/admin/templates/components/admin-theme-editor.hbs
+++ b/app/assets/javascripts/admin/templates/components/admin-theme-editor.hbs
@@ -87,4 +87,4 @@
<pre class="field-error">{{error}}</pre>
{{/if}}
-{{ace-editor content=activeSection editorId=editorId mode=activeSectionMode autofocus="true"}}
+{{ace-editor content=activeSection editorId=editorId mode=activeSectionMode autofocus="true" placeholder=placeholder}}
diff --git a/app/assets/stylesheets/common/admin/customize.scss b/app/assets/stylesheets/common/admin/customize.scss
index a9a0e742954..970a72047e0 100644
--- a/app/assets/stylesheets/common/admin/customize.scss
+++ b/app/assets/stylesheets/common/admin/customize.scss
@@ -447,6 +447,12 @@
bottom: 0;
}
+ .ace_placeholder {
+ font-family: inherit;
+ font-size: $font-up-1;
+ color: $primary-high;
+ }
+
.status-actions {
float: right;
margin-top: 7px;
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index e8c260f7a5a..3d9523b063e 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -458,7 +458,7 @@ module ApplicationHelper
dark_scheme_id = user_dark_scheme_id || SiteSetting.default_dark_mode_color_scheme_id
if dark_scheme_id != -1
- result << Stylesheet::Manager.color_scheme_stylesheet_link_tag(dark_scheme_id, '(prefers-color-scheme: dark)')
+ result << Stylesheet::Manager.color_scheme_stylesheet_link_tag(dark_scheme_id, '(prefers-color-scheme: dark)', theme_ids)
end
result.html_safe
end
diff --git a/app/models/color_scheme.rb b/app/models/color_scheme.rb
index eb09f1cddbf..29788581781 100644
--- a/app/models/color_scheme.rb
+++ b/app/models/color_scheme.rb
@@ -271,7 +271,7 @@ class ColorScheme < ActiveRecord::Base
def publish_discourse_stylesheet
if self.id
- Stylesheet::Manager.color_scheme_cache_clear(self)
+ Stylesheet::Manager.clear_color_scheme_cache!
theme_ids = Theme.where(color_scheme_id: self.id).pluck(:id)
if theme_ids.present?
diff --git a/app/models/theme_field.rb b/app/models/theme_field.rb
index a15d42ec9e5..4e525b0c1db 100644
--- a/app/models/theme_field.rb
+++ b/app/models/theme_field.rb
@@ -265,7 +265,7 @@ class ThemeField < ActiveRecord::Base
end
def self.scss_fields
- @scss_fields ||= %w(scss embedded_scss)
+ @scss_fields ||= %w(scss embedded_scss color_definitions)
end
def self.basic_targets
@@ -424,6 +424,9 @@ class ThemeField < ActiveRecord::Base
ThemeFileMatcher.new(regex: /^common\/embedded\.scss$/,
targets: :common, names: "embedded_scss", types: :scss,
canonical: -> (h) { "common/embedded.scss" }),
+ ThemeFileMatcher.new(regex: /^common\/color_definitions\.scss$/,
+ targets: :common, names: "color_definitions", types: :scss,
+ canonical: -> (h) { "common/color_definitions.scss" }),
ThemeFileMatcher.new(regex: /^(?:scss|stylesheets)\/(?<name>.+)\.scss$/,
targets: :extra_scss, names: nil, types: :scss,
canonical: -> (h) { "stylesheets/#{h[:name]}.scss" }),
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index 0168e342682..333cfd090d5 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -3994,6 +3994,10 @@ en:
embedded_scss:
text: "Embedded CSS"
title: "Enter custom CSS to deliver with embedded version of comments"
+ color_definitions:
+ text: "Color Definitions"
+ title: "Enter custom color definitions (advanced users only)"
+ placeholder: "\r\nUse this stylesheet to add custom colors to the list of CSS custom properties.\r\n\r\nExample: \r\n\r\n:root {\r\n --mytheme-tertiary-or-quaternary: #{dark-light-choose($tertiary, $quaternary)};\r\n}\r\n\r\nPrefixing the property names is highly recommended to avoid conflicts with plugins and/or core."
head_tag:
text: "</head>"
title: "HTML that will be inserted before the </head> tag"
diff --git a/lib/stylesheet/compiler.rb b/lib/stylesheet/compiler.rb
index b5575cda494..8596c5d8b86 100644
--- a/lib/stylesheet/compiler.rb
+++ b/lib/stylesheet/compiler.rb
@@ -21,7 +21,7 @@ module Stylesheet
file = File.read path
if asset.to_s == Stylesheet::Manager::COLOR_SCHEME_STYLESHEET
- file += Stylesheet::Importer.import_color_definitions
+ file += Stylesheet::Importer.import_color_definitions(options[:theme_id])
end
end
diff --git a/lib/stylesheet/importer.rb b/lib/stylesheet/importer.rb
index f1901969ae7..e08d766ebf5 100644
--- a/lib/stylesheet/importer.rb
+++ b/lib/stylesheet/importer.rb
@@ -115,14 +115,20 @@ module Stylesheet
register_imports!
- def self.import_color_definitions
- return "" unless DiscoursePluginRegistry.color_definition_stylesheets.length
+ def self.import_color_definitions(theme_id)
contents = +""
DiscoursePluginRegistry.color_definition_stylesheets.each do |name, path|
contents << "// Color definitions from #{name}\n\n"
contents << File.read(path.to_s)
contents << "\n\n"
end
+
+ if theme_id
+ Theme.list_baked_fields([theme_id], :common, :color_definitions).each do |row|
+ contents << "// Color definitions from #{Theme.find_by_id(theme_id)&.name}\n\n"
+ contents << row.value
+ end
+ end
contents
end
diff --git a/lib/stylesheet/manager.rb b/lib/stylesheet/manager.rb
index 0101847578b..209d25ad52c 100644
--- a/lib/stylesheet/manager.rb
+++ b/lib/stylesheet/manager.rb
@@ -23,6 +23,10 @@ class Stylesheet::Manager
cache.hash.keys.select { |k| k =~ /theme/ }.each { |k| cache.delete(k) }
end
+ def self.clear_color_scheme_cache!
+ cache.hash.keys.select { |k| k =~ /color_definitions/ }.each { |k| cache.delete(k) }
+ end
+
def self.clear_core_cache!(targets)
cache.hash.keys.select { |k| k =~ /#{targets.join('|')}/ }.each { |k| cache.delete(k) }
end
@@ -94,13 +98,14 @@ class Stylesheet::Manager
end
def self.color_scheme_stylesheet_details(color_scheme_id = nil, media, theme_id)
+ theme_id = theme_id || SiteSetting.default_theme_id
+
color_scheme = begin
ColorScheme.find(color_scheme_id)
rescue
# don't load fallback when requesting dark color scheme
return false if media != "all"
- theme_id = theme_id || SiteSetting.default_theme_id
Theme.find_by_id(theme_id)&.color_scheme || ColorScheme.base
end
@@ -108,13 +113,14 @@ class Stylesheet::Manager
target = COLOR_SCHEME_STYLESHEET.to_sym
current_hostname = Discourse.current_hostname
- cache_key = color_scheme_cache_key(color_scheme)
+ cache_key = color_scheme_cache_key(color_scheme, theme_id)
stylesheets = cache[cache_key]
return stylesheets if stylesheets.present?
stylesheet = { color_scheme_id: color_scheme&.id }
- builder = self.new(target, nil, color_scheme)
+ builder = self.new(target, theme_id, color_scheme)
+
builder.compile unless File.exists?(builder.stylesheet_fullpath)
href = builder.stylesheet_path(current_hostname)
@@ -132,20 +138,16 @@ class Stylesheet::Manager
%[<link href="#{href}" media="#{media}" rel="stylesheet"/>].html_safe
end
- def self.color_scheme_cache_key(color_scheme)
+ def self.color_scheme_cache_key(color_scheme, theme_id = nil)
color_scheme_name = Slug.for(color_scheme.name) + color_scheme&.id.to_s
- "#{COLOR_SCHEME_STYLESHEET}_#{color_scheme_name}_#{Discourse.current_hostname}"
- end
-
- def self.color_scheme_cache_clear(color_scheme)
- cache_key = color_scheme_cache_key(color_scheme)
- cache[cache_key] = nil
+ theme_string = theme_id ? "_theme#{theme_id}" : ""
+ "#{COLOR_SCHEME_STYLESHEET}_#{color_scheme_name}#{theme_string}_#{Discourse.current_hostname}"
end
def self.precompile_css
- themes = Theme.where('user_selectable OR id = ?', SiteSetting.default_theme_id).pluck(:id, :name)
+ themes = Theme.where('user_selectable OR id = ?', SiteSetting.default_theme_id).pluck(:id, :name, :color_scheme_id)
themes << nil
- themes.each do |id, name|
+ themes.each do |id, name, color_scheme_id|
[:desktop, :mobile, :desktop_rtl, :mobile_rtl, :desktop_theme, :mobile_theme, :admin].each do |target|
theme_id = id || SiteSetting.default_theme_id
next if target =~ THEME_REGEX && theme_id == -1
@@ -156,17 +158,13 @@ class Stylesheet::Manager
builder.compile(force: true)
cache[cache_key] = nil
end
- end
- cs_ids = Theme.where('user_selectable OR id = ?', SiteSetting.default_theme_id).pluck(:color_scheme_id)
- ColorScheme.where(id: cs_ids).each do |cs|
- target = COLOR_SCHEME_STYLESHEET
- STDERR.puts "precompile target: #{target} #{cs.name}"
+ scheme = ColorScheme.find_by_id(color_scheme_id) || ColorScheme.base
+ STDERR.puts "precompile target: #{COLOR_SCHEME_STYLESHEET} #{name} (#{scheme.name})"
- builder = self.new(target, nil, cs)
+ builder = self.new(COLOR_SCHEME_STYLESHEET, id, scheme)
builder.compile(force: true)
- cache_key = color_scheme_cache_key(cs)
- cache[cache_key] = nil
+ clear_color_scheme_cache!
end
nil
@@ -349,8 +347,6 @@ class Stylesheet::Manager
end
def theme_digest
- scss = ""
-
if [:mobile_theme, :desktop_theme].include?(@target)
scss_digest = theme.resolve_baked_field(:common, :scss)
scss_digest += theme.resolve_baked_field(@target.to_s.sub("_theme", ""), :scss)
@@ -401,7 +397,8 @@ class Stylesheet::Manager
category_updated = Category.where("uploaded_background_id IS NOT NULL").pluck(:updated_at).map(&:to_i).sum
if cs || category_updated > 0
- Digest::SHA1.hexdigest "#{RailsMultisite::ConnectionManagement.current_db}-#{cs&.id}-#{cs&.version}-#{Stylesheet::Manager.last_file_updated}-#{category_updated}"
+ theme_color_defs = theme&.resolve_baked_field(:common, :color_definitions)
+ Digest::SHA1.hexdigest "#{RailsMultisite::ConnectionManagement.current_db}-#{cs&.id}-#{cs&.version}-#{theme_color_defs}-#{Stylesheet::Manager.last_file_updated}-#{category_updated}"
else
digest_string = "defaults-#{Stylesheet::Manager.last_file_updated}"
diff --git a/spec/components/stylesheet/manager_spec.rb b/spec/components/stylesheet/manager_spec.rb
index a544d9ee64e..7ca60a24349 100644
--- a/spec/components/stylesheet/manager_spec.rb
+++ b/spec/components/stylesheet/manager_spec.rb
@@ -175,6 +175,21 @@ describe Stylesheet::Manager do
expect(digest1).to_not eq(digest2)
end
+
+ it "updates digest when updating a theme's color definitions" do
+ scheme = ColorScheme.base
+ theme = Fabricate(:theme)
+ manager = Stylesheet::Manager.new(:color_definitions, theme.id, scheme)
+ digest1 = manager.color_scheme_digest
+
+ theme.set_field(target: :common, name: :color_definitions, value: 'body {color: brown}')
+ theme.save!
+
+ digest2 = manager.color_scheme_digest
+
+ expect(digest1).to_not eq(digest2)
+ end
+
end
describe 'color_scheme_stylesheets' do
@@ -248,6 +263,16 @@ 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!
+
+ scheme = ColorScheme.base
+ stylesheet = Stylesheet::Manager.new(:color_definitions, theme.id, scheme).compile
+
+ expect(stylesheet).to include("--special: rebeccapurple")
+ end
end
# this test takes too long, we don't run it by default
@@ -286,7 +311,7 @@ describe Stylesheet::Manager do
Stylesheet::Manager.precompile_css
results = StylesheetCache.pluck(:target)
- expect(results.size).to eq(16) # (2 themes x 7 targets) + (2 themes x 1 color scheme)
+ expect(results.size).to eq(17) # (2 themes x 7 targets) + 3 color schemes (2 themes, 1 base)
core_targets.each do |tar|
expect(results.count { |target| target =~ /^#{tar}_(#{scheme1.id}|#{scheme2.id})$/ }).to eq(2)
end
@@ -301,7 +326,7 @@ describe Stylesheet::Manager do
Stylesheet::Manager.precompile_css
results = StylesheetCache.pluck(:target)
- expect(results.size).to eq(21) # (2 themes x 7 targets) + (1 no/default/core theme x 5 core targets) + (2 themes x 1 color scheme)
+ expect(results.size).to eq(22) # (2 themes x 7 targets) + (1 no/default/core theme x 5 core targets) + 3 color schemes (2 themes, 1 base)
core_targets.each do |tar|
expect(results.count { |target| target =~ /^(#{tar}_(#{scheme1.id}|#{scheme2.id})|#{tar})$/ }).to eq(3)
diff --git a/spec/models/color_scheme_spec.rb b/spec/models/color_scheme_spec.rb
index 4a9f34018b4..fe2dd608feb 100644
--- a/spec/models/color_scheme_spec.rb
+++ b/spec/models/color_scheme_spec.rb
@@ -21,12 +21,15 @@ describe ColorScheme do
theme.save!
href = Stylesheet::Manager.stylesheet_data(:desktop_theme, theme.id)[0][:new_href]
+ colors_href = Stylesheet::Manager.color_scheme_stylesheet_details(scheme.id, "all", nil)
ColorSchemeRevisor.revise(scheme, colors: [{ name: 'primary', hex: 'bbb' }])
href2 = Stylesheet::Manager.stylesheet_data(:desktop_theme, theme.id)[0][:new_href]
+ colors_href2 = Stylesheet::Manager.color_scheme_stylesheet_details(scheme.id, "all", nil)
expect(href).not_to eq(href2)
+ expect(colors_href).not_to eq(colors_href2)
end
describe "new" do
diff --git a/spec/models/remote_theme_spec.rb b/spec/models/remote_theme_spec.rb
index 29cc4b939d5..8b0e44ea416 100644
--- a/spec/models/remote_theme_spec.rb
+++ b/spec/models/remote_theme_spec.rb
@@ -60,6 +60,7 @@ describe RemoteTheme do
"common/header.html" => "I AM HEADER",
"common/random.html" => "I AM SILLY",
"common/embedded.scss" => "EMBED",
+ "common/color_definitions.scss" => ":root{--color-var: red}",
"assets/font.woff2" => "FAKE FONT",
"settings.yaml" => "boolean_setting: true",
"locales/en.yml" => "sometranslations"
@@ -90,12 +91,14 @@ describe RemoteTheme do
expect(@theme.theme_modifier_set.serialize_topic_excerpts).to eq(true)
- expect(@theme.theme_fields.length).to eq(9)
+ expect(@theme.theme_fields.length).to eq(10)
mapped = Hash[*@theme.theme_fields.map { |f| ["#{f.target_id}-#{f.name}", f.value] }.flatten]
+
expect(mapped["0-header"]).to eq("I AM HEADER")
expect(mapped["1-scss"]).to eq(scss_data)
expect(mapped["0-embedded_scss"]).to eq("EMBED")
+ expect(mapped["0-color_definitions"]).to eq(":root{--color-var: red}")
expect(mapped["0-font"]).to eq("")
@@ -103,7 +106,7 @@ describe RemoteTheme do
expect(mapped["4-en"]).to eq("sometranslations")
- expect(mapped.length).to eq(9)
+ expect(mapped.length).to eq(10)
expect(@theme.settings.length).to eq(1)
expect(@theme.settings.first.value).to eq(true)