FIX: color scheme selection with non-default theme

This fixes an issue where a non-default theme set to use the base color
scheme (i.e. the theme had an empty `color_scheme_id`) was loading the
default theme's color scheme instead.
This commit is contained in:
Penar Musaraj 2020-08-12 08:49:13 -04:00
parent eb7320f52c
commit 6dd9f2eca2
No known key found for this signature in database
GPG Key ID: E390435D881FF0F7
3 changed files with 20 additions and 6 deletions

View File

@ -452,7 +452,7 @@ module ApplicationHelper
def discourse_color_scheme_stylesheets def discourse_color_scheme_stylesheets
result = +"" result = +""
result << Stylesheet::Manager.color_scheme_stylesheet_link_tag(scheme_id) result << Stylesheet::Manager.color_scheme_stylesheet_link_tag(scheme_id, 'all', theme_ids)
user_dark_scheme_id = current_user&.user_option&.dark_scheme_id user_dark_scheme_id = current_user&.user_option&.dark_scheme_id
dark_scheme_id = user_dark_scheme_id || SiteSetting.default_dark_mode_color_scheme_id dark_scheme_id = user_dark_scheme_id || SiteSetting.default_dark_mode_color_scheme_id

View File

@ -93,13 +93,15 @@ class Stylesheet::Manager
end end
end end
def self.color_scheme_stylesheet_details(color_scheme_id = nil, media) def self.color_scheme_stylesheet_details(color_scheme_id = nil, media, theme_id)
color_scheme = begin color_scheme = begin
ColorScheme.find(color_scheme_id) ColorScheme.find(color_scheme_id)
rescue rescue
# don't load fallback when requesting dark color scheme # don't load fallback when requesting dark color scheme
return false if media != "all" return false if media != "all"
Theme.find_by_id(SiteSetting.default_theme_id)&.color_scheme || ColorScheme.base
theme_id = theme_id || SiteSetting.default_theme_id
Theme.find_by_id(theme_id)&.color_scheme || ColorScheme.base
end end
return false if !color_scheme return false if !color_scheme
@ -121,8 +123,9 @@ class Stylesheet::Manager
stylesheet stylesheet
end end
def self.color_scheme_stylesheet_link_tag(color_scheme_id = nil, media = 'all') def self.color_scheme_stylesheet_link_tag(color_scheme_id = nil, media = 'all', theme_ids = nil)
stylesheet = color_scheme_stylesheet_details(color_scheme_id, media) theme_id = theme_ids&.first
stylesheet = color_scheme_stylesheet_details(color_scheme_id, media, theme_id)
return '' if !stylesheet return '' if !stylesheet
href = stylesheet[:new_href] href = stylesheet[:new_href]

View File

@ -208,7 +208,18 @@ describe Stylesheet::Manager do
expect(link).to include("/stylesheets/color_definitions_funky_#{cs.id}_") expect(link).to include("/stylesheets/color_definitions_funky_#{cs.id}_")
end end
it "uses the correct scheme when colors are passed" do it "uses the correct color scheme when a non-default theme is selected and it uses the base 'Light' scheme" do
cs = Fabricate(:color_scheme, name: 'Not This')
default_theme = Fabricate(:theme, color_scheme_id: cs.id)
SiteSetting.default_theme_id = default_theme.id
user_theme = Fabricate(:theme, color_scheme_id: nil)
link = Stylesheet::Manager.color_scheme_stylesheet_link_tag(nil, "all", [user_theme.id])
expect(link).to include("/stylesheets/color_definitions_base_")
end
it "uses the correct scheme when a valid scheme id is used" do
link = Stylesheet::Manager.color_scheme_stylesheet_link_tag(ColorScheme.first.id) link = Stylesheet::Manager.color_scheme_stylesheet_link_tag(ColorScheme.first.id)
slug = Slug.for(ColorScheme.first.name) + "_" + ColorScheme.first.id.to_s slug = Slug.for(ColorScheme.first.name) + "_" + ColorScheme.first.id.to_s
expect(link).to include("/stylesheets/color_definitions_#{slug}_") expect(link).to include("/stylesheets/color_definitions_#{slug}_")