FIX: Add order to outputted stylesheet link tags (#13735)

See PR for details. (Disabled by default in this commit.)
This commit is contained in:
Penar Musaraj 2021-07-15 12:51:46 -04:00 committed by GitHub
parent d6fc39c886
commit a23153fdca
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 92 additions and 36 deletions

View File

@ -353,6 +353,9 @@ basic:
default: "arial"
enum: "BaseFontSetting"
refresh: true
order_stylesheets:
default: false
hidden: true
login:
invite_only:

View File

@ -188,12 +188,13 @@ class Stylesheet::Manager
def stylesheet_link_tag(target = :desktop, media = 'all')
stylesheets = stylesheet_details(target, media)
stylesheets.map do |stylesheet|
href = stylesheet[:new_href]
theme_id = stylesheet[:theme_id]
data_theme_id = theme_id ? "data-theme-id=\"#{theme_id}\"" : ""
%[<link href="#{href}" media="#{media}" rel="stylesheet" data-target="#{target}" #{data_theme_id}/>]
theme_name = stylesheet[:theme_name]
data_theme_name = theme_name ? "data-theme-name=\"#{theme_name}\"" : ""
%[<link href="#{href}" media="#{media}" rel="stylesheet" data-target="#{target}" #{data_theme_id} #{data_theme_name}/>]
end.join("\n").html_safe
end
@ -211,54 +212,37 @@ class Stylesheet::Manager
@@lock.synchronize do
stylesheets = []
stale_theme_ids = []
theme_ids = is_theme_target ? @theme_ids : [nil]
theme_ids.each do |theme_id|
cache_key = "path_#{target}_#{theme_id}_#{current_hostname}"
if href = cache[cache_key]
stylesheets << {
target: target,
theme_id: theme_id,
new_href: href
}
else
stale_theme_ids << theme_id
end
end
scss_checker = ScssChecker.new(target, stale_theme_ids)
if is_theme_target
themes = load_themes(stale_theme_ids)
scss_checker = ScssChecker.new(target, @theme_ids)
themes = load_themes(@theme_ids)
themes.each do |theme|
theme_id = theme&.id
data = { target: target, theme_id: theme_id }
data = { target: target, theme_id: theme_id, theme_name: theme&.name.downcase, remote: theme.remote_theme_id? }
builder = Builder.new(target: target, theme: theme, manager: self)
is_theme = builder.is_theme?
has_theme = builder.theme.present?
if is_theme && !has_theme
next
else
next if is_theme && builder.theme&.component && !scss_checker.has_scss(theme_id)
builder.compile unless File.exists?(builder.stylesheet_fullpath)
href = builder.stylesheet_path(current_hostname)
cache.defer_set("path_#{target}_#{theme_id}_#{current_hostname}", href)
end
next if builder.theme&.component && !scss_checker.has_scss(theme_id)
builder.compile unless File.exists?(builder.stylesheet_fullpath)
href = builder.stylesheet_path(current_hostname)
data[:new_href] = href
stylesheets << data
end
if SiteSetting.order_stylesheets && stylesheets.size > 1
stylesheets = stylesheets.sort_by do |s|
[
s[:remote] ? 0 : 1,
s[:theme_id] == @theme_id ? 1 : 0,
s[:theme_name]
]
end
end
else
builder = Builder.new(target: target, manager: self)
builder.compile unless File.exists?(builder.stylesheet_fullpath)
href = builder.stylesheet_path(current_hostname)
cache.defer_set("path_#{target}__#{current_hostname}", href)
data = { target: target, new_href: href }
stylesheets << data
end

View File

@ -20,7 +20,7 @@ describe Stylesheet::Manager do
end
context "themes with components" do
let(:child_theme) { Fabricate(:theme, component: true).tap { |c|
let(:child_theme) { Fabricate(:theme, component: true, name: "a component").tap { |c|
c.set_field(target: :common, name: "scss", value: ".child_common{.scss{color: red;}}")
c.set_field(target: :desktop, name: "scss", value: ".child_desktop{.scss{color: red;}}")
c.set_field(target: :mobile, name: "scss", value: ".child_mobile{.scss{color: red;}}")
@ -135,6 +135,75 @@ describe Stylesheet::Manager do
)
end
context "stylesheet order" do
let(:z_child_theme) do
Fabricate(:theme, component: true, name: "ze component").tap do |z|
z.set_field(target: :desktop, name: "scss", value: ".child_desktop{.scss{color: red;}}")
z.save!
end
end
let(:remote) { RemoteTheme.create!(remote_url: "https://github.com/org/remote-theme1") }
let(:child_remote) do
Fabricate(:theme, remote_theme: remote, component: true).tap do |t|
t.set_field(target: :desktop, name: "scss", value: ".child_desktop{.scss{color: red;}}")
t.save!
end
end
before do
SiteSetting.order_stylesheets = true
end
it 'output remote child, then sort children alphabetically, then local parent' do
theme.add_relative_theme!(:child, z_child_theme)
theme.add_relative_theme!(:child, child_remote)
manager = manager(theme.id)
hrefs = manager.stylesheet_details(:desktop_theme, 'all')
parent = hrefs.select { |href| href[:theme_id] == theme.id }.first
child_a = hrefs.select { |href| href[:theme_id] == child_theme.id }.first
child_z = hrefs.select { |href| href[:theme_id] == z_child_theme.id }.first
child_r = hrefs.select { |href| href[:theme_id] == child_remote.id }.first
child_local_A = "<link href=\"#{child_a[:new_href]}\" data-theme-id=\"#{child_a[:theme_id]}\" data-theme-name=\"#{child_a[:theme_name]}\"/>"
child_local_Z = "<link href=\"#{child_z[:new_href]}\" data-theme-id=\"#{child_z[:theme_id]}\" data-theme-name=\"#{child_z[:theme_name]}\"/>"
child_remote_R = "<link href=\"#{child_r[:new_href]}\" data-theme-id=\"#{child_r[:theme_id]}\" data-theme-name=\"#{child_r[:theme_name]}\"/>"
parent_local = "<link href=\"#{parent[:new_href]}\" data-theme-id=\"#{parent[:theme_id]}\" data-theme-name=\"#{parent[:theme_name]}\"/>"
link_hrefs = manager.stylesheet_link_tag(:desktop_theme).gsub('media="all" rel="stylesheet" data-target="desktop_theme" ', '')
expect(link_hrefs).to eq([child_remote_R, child_local_A, child_local_Z, parent_local].join("\n").html_safe)
end
it "output remote child, remote parent, local child" do
remote2 = RemoteTheme.create!(remote_url: "https://github.com/org/remote-theme2")
remote_main_theme = Fabricate(:theme, remote_theme: remote2, name: "remote main").tap do |t|
t.set_field(target: :desktop, name: "scss", value: ".el{color: red;}")
t.save!
end
remote_main_theme.add_relative_theme!(:child, z_child_theme)
remote_main_theme.add_relative_theme!(:child, child_remote)
manager = manager(remote_main_theme.id)
hrefs = manager.stylesheet_details(:desktop_theme, 'all')
parent_r = hrefs.select { |href| href[:theme_id] == remote_main_theme.id }.first
child_z = hrefs.select { |href| href[:theme_id] == z_child_theme.id }.first
child_r = hrefs.select { |href| href[:theme_id] == child_remote.id }.first
parent_remote = "<link href=\"#{parent_r[:new_href]}\" data-theme-id=\"#{parent_r[:theme_id]}\" data-theme-name=\"#{parent_r[:theme_name]}\"/>"
child_local = "<link href=\"#{child_z[:new_href]}\" data-theme-id=\"#{child_z[:theme_id]}\" data-theme-name=\"#{child_z[:theme_name]}\"/>"
child_remote = "<link href=\"#{child_r[:new_href]}\" data-theme-id=\"#{child_r[:theme_id]}\" data-theme-name=\"#{child_r[:theme_name]}\"/>"
link_hrefs = manager.stylesheet_link_tag(:desktop_theme).gsub('media="all" rel="stylesheet" data-target="desktop_theme" ', '')
expect(link_hrefs).to eq([child_remote, parent_remote, child_local].join("\n").html_safe)
end
end
it 'outputs tags for non-theme targets for theme component' do
child_theme = Fabricate(:theme, component: true)