From cfde4419f52e9024fabf79463b1b8be68c44b1a8 Mon Sep 17 00:00:00 2001 From: Joe <33972521+hnb-ku@users.noreply.github.com> Date: Tue, 5 Jul 2022 00:23:09 +0800 Subject: [PATCH] DEV: Preload CSS in the `` (#17322) This commit adds preload links for core/plugin/theme CSS stylesheets in the head. Preload links are non-blocking and run in parallel. This means that they should have already been downloaded by the time we use the actual stylesheets (in the tag). Google is currently complaining about this here and this PR will address that warning. This commit will also fix an issue in the splash screen where it sometimes doesn't respect the theme colors - causing a slightly jarring experience on dark themes. Note that I opted not to add new specs because the underlying work required already has a lot of coverage. The new methods only change the output HTML so we can chuck that in the document This change also means that we can make all the stylesheets non-render blocking, but that will follow in a separate commit. --- .../javascripts/discourse/app/index.html | 3 +++ .../discourse/lib/bootstrap-json/index.js | 8 +++++++ app/helpers/application_helper.rb | 24 +++++++++++++++++++ .../_discourse_preload_stylesheet.html.erb | 23 ++++++++++++++++++ app/views/layouts/application.html.erb | 2 ++ lib/stylesheet/manager.rb | 18 ++++++++++++++ 6 files changed, 78 insertions(+) create mode 100644 app/views/common/_discourse_preload_stylesheet.html.erb diff --git a/app/assets/javascripts/discourse/app/index.html b/app/assets/javascripts/discourse/app/index.html index ad51651ba85..d0b347c7d97 100644 --- a/app/assets/javascripts/discourse/app/index.html +++ b/app/assets/javascripts/discourse/app/index.html @@ -11,6 +11,9 @@ {{content-for "before-script-load"}} + + {{content-for "discourse-preload-stylesheets"}} + diff --git a/app/assets/javascripts/discourse/lib/bootstrap-json/index.js b/app/assets/javascripts/discourse/lib/bootstrap-json/index.js index d6fe3c93c28..adb61b6d0b6 100644 --- a/app/assets/javascripts/discourse/lib/bootstrap-json/index.js +++ b/app/assets/javascripts/discourse/lib/bootstrap-json/index.js @@ -101,6 +101,13 @@ function beforeScriptLoad(buffer, bootstrap) { ); } +function discoursePreloadStylesheets(buffer, bootstrap) { + (bootstrap.stylesheets || []).forEach((s) => { + let link = ``; + buffer.push(link); + }); +} + function discourseStylesheets(buffer, bootstrap) { (bootstrap.stylesheets || []).forEach((s) => { let attrs = []; @@ -164,6 +171,7 @@ function preloaded(buffer, bootstrap) { const BUILDERS = { "html-tag": htmlTag, "before-script-load": beforeScriptLoad, + "discourse-preload-stylesheets": discoursePreloadStylesheets, head, body, "discourse-stylesheets": discourseStylesheets, diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index eeee4f410e0..3a20ea4ba9a 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -570,6 +570,19 @@ module ApplicationHelper ) end + def discourse_stylesheet_preload_tag(name, opts = {}) + manager = + if opts.key?(:theme_id) + Stylesheet::Manager.new( + theme_id: customization_disabled? ? nil : opts[:theme_id] + ) + else + stylesheet_manager + end + + manager.stylesheet_preload_tag(name, 'all') + end + def discourse_stylesheet_link_tag(name, opts = {}) manager = if opts.key?(:theme_id) @@ -583,6 +596,17 @@ module ApplicationHelper manager.stylesheet_link_tag(name, 'all') end + def discourse_preload_color_scheme_stylesheets + result = +"" + result << stylesheet_manager.color_scheme_stylesheet_preload_tag(scheme_id, 'all') + + if dark_scheme_id != -1 + result << stylesheet_manager.color_scheme_stylesheet_preload_tag(dark_scheme_id, '(prefers-color-scheme: dark)') + end + + result.html_safe + end + def discourse_color_scheme_stylesheets result = +"" result << stylesheet_manager.color_scheme_stylesheet_link_tag(scheme_id, 'all') diff --git a/app/views/common/_discourse_preload_stylesheet.html.erb b/app/views/common/_discourse_preload_stylesheet.html.erb new file mode 100644 index 00000000000..06402715769 --- /dev/null +++ b/app/views/common/_discourse_preload_stylesheet.html.erb @@ -0,0 +1,23 @@ +<%= discourse_preload_color_scheme_stylesheets %> + +<%- if rtl? %> + <%= discourse_stylesheet_preload_tag(mobile_view? ? :mobile_rtl : :desktop_rtl) %> +<%- else %> + <%= discourse_stylesheet_preload_tag(mobile_view? ? :mobile : :desktop) %> +<%- end %> + +<%- if staff? %> + <%= discourse_stylesheet_preload_tag(:admin) %> +<%- end %> + +<%- if admin? %> + <%= discourse_stylesheet_preload_tag(:wizard) %> +<%- end %> + +<%- Discourse.find_plugin_css_assets(include_official: allow_plugins?, include_unofficial: allow_third_party_plugins?, mobile_view: mobile_view?, desktop_view: !mobile_view?, request: request).each do |file| %> + <%= discourse_stylesheet_preload_tag(file) %> +<%- end %> + +<%- if theme_id.present? %> + <%= discourse_stylesheet_preload_tag(mobile_view? ? :mobile_theme : :desktop_theme) %> +<%- end %> diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 969c68a5428..7db635e80d1 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -6,6 +6,8 @@ + + <%= render partial: "common/discourse_preload_stylesheet" %> <%= render partial: "layouts/head" %> <%= discourse_csrf_tags %> diff --git a/lib/stylesheet/manager.rb b/lib/stylesheet/manager.rb index 328762f06c9..72d4424d0c9 100644 --- a/lib/stylesheet/manager.rb +++ b/lib/stylesheet/manager.rb @@ -188,6 +188,14 @@ class Stylesheet::Manager stylesheet_details(target, "all") end + def stylesheet_preload_tag(target = :desktop, media = 'all') + stylesheets = stylesheet_details(target, media) + stylesheets.map do |stylesheet| + href = stylesheet[:new_href] + %[] + end.join("\n").html_safe + end + def stylesheet_link_tag(target = :desktop, media = 'all') stylesheets = stylesheet_details(target, media) stylesheets.map do |stylesheet| @@ -293,6 +301,16 @@ class Stylesheet::Manager stylesheet end + def color_scheme_stylesheet_preload_tag(color_scheme_id = nil, media = 'all') + stylesheet = color_scheme_stylesheet_details(color_scheme_id, media) + + return '' if !stylesheet + + href = stylesheet[:new_href] + + %[].html_safe + end + def color_scheme_stylesheet_link_tag(color_scheme_id = nil, media = 'all') stylesheet = color_scheme_stylesheet_details(color_scheme_id, media)