DEV: Refactor font and category background importers (#12312)

app/assets/stylesheets/common.scss

@@ -1,4 +1,3 @@
-@import "font";
 @import "vendor/normalize";
 @import "vendor/normalize-ext";
 @import "vendor/pikaday";

app/assets/stylesheets/desktop.scss

@@ -4,7 +4,3 @@
 
 // Import all component-specific files
 @import "desktop/components/_index";
-
-/* These files doesn't actually exist, they are injected by Stylesheet::Compiler. */
-
-@import "category_backgrounds";

app/assets/stylesheets/embed.scss

@@ -1,4 +1,3 @@
-@import "font";
 @import "./vendor/normalize";
 @import "./vendor/normalize-ext";
 @import "./common/foundation/base";

app/assets/stylesheets/mobile.scss

@@ -6,7 +6,3 @@
 @import "mobile/components/_index";
 
 @import "mobile/select-kit/_index";
-
-/* These files doesn't actually exist, they are injected by Stylesheet::Compiler. */
-
-@import "category_backgrounds";

app/assets/stylesheets/wizard.scss

@@ -1,4 +1,3 @@
-@import "wizard_fonts";
 @import "color_definitions";
 @import "vendor/normalize";
 @import "vendor/normalize-ext";

lib/stylesheet/compiler.rb

@@ -24,6 +24,16 @@ module Stylesheet
         path = "#{Stylesheet::Common::ASSET_ROOT}/#{filename}"
         file += File.read path
 
+        case asset.to_s
+        when "desktop", "mobile"
+          file += importer.category_backgrounds
+          file += importer.font
+        when "embed", "publish"
+          file += importer.font
+        when "wizard"
+          file += importer.wizard_fonts
+        end
+
         if asset.to_s == Stylesheet::Manager::COLOR_SCHEME_STYLESHEET
           file += importer.import_color_definitions
           file += importer.import_wcag_overrides

lib/stylesheet/importer.rb

@@ -36,78 +36,78 @@ module Stylesheet
         end
       end
 
-      register_import "font" do
-        body_font = DiscourseFonts.fonts.find { |f| f[:key] == SiteSetting.base_font }
-        heading_font = DiscourseFonts.fonts.find { |f| f[:key] == SiteSetting.heading_font }
-        contents = +""
-
-        if body_font.present?
-          contents << <<~EOF
-            #{font_css(body_font)}
-
-            :root {
-              --font-family: #{body_font[:stack]};
-            }
-          EOF
-        end
-
-        if heading_font.present?
-          contents << <<~EOF
-            #{font_css(heading_font)}
-
-            :root {
-              --heading-font-family: #{heading_font[:stack]};
-            }
-          EOF
-        end
-
-        Import.new("font.scss", source: contents)
-      end
-
-      register_import "wizard_fonts" do
-        contents = +""
-
-        DiscourseFonts.fonts.each do |font|
-          if font[:key] == "system"
-            # Overwrite font definition because the preview canvases in the wizard require explicit @font-face definitions.
-            # uses same technique as https://github.com/jonathantneal/system-font-css
-            font[:variants] = [
-              { src: 'local(".SFNS-Regular"), local(".SFNSText-Regular"), local(".HelveticaNeueDeskInterface-Regular"), local(".LucidaGrandeUI"), local("Segoe UI"), local("Ubuntu"), local("Roboto-Regular"), local("DroidSans"), local("Tahoma")', weight: 400 },
-              { src: 'local(".SFNS-Bold"), local(".SFNSText-Bold"), local(".HelveticaNeueDeskInterface-Bold"), local(".LucidaGrandeUI"), local("Segoe UI Bold"), local("Ubuntu Bold"), local("Roboto-Bold"), local("DroidSans-Bold"), local("Tahoma Bold")', weight: 700 }
-            ]
-          end
-
-          contents << font_css(font)
-          contents << <<~EOF
-            .body-font-#{font[:key].tr("_", "-")} {
-              font-family: #{font[:stack]};
-            }
-            .heading-font-#{font[:key].tr("_", "-")} h2 {
-              font-family: #{font[:stack]};
-            }
-          EOF
-        end
-
-        Import.new("wizard_fonts.scss", source: contents)
-      end
-
-      register_import "category_backgrounds" do
-        contents = +""
-        Category.where('uploaded_background_id IS NOT NULL').each do |c|
-          contents << category_css(c) if c.uploaded_background&.url.present?
-        end
-
-        Import.new("category_background.scss", source: contents)
-      end
-
     end
 
     register_imports!
 
+    def font
+      body_font = DiscourseFonts.fonts.find { |f| f[:key] == SiteSetting.base_font }
+      heading_font = DiscourseFonts.fonts.find { |f| f[:key] == SiteSetting.heading_font }
+      contents = +""
+
+      if body_font.present?
+        contents << <<~EOF
+          #{font_css(body_font)}
+
+          :root {
+            --font-family: #{body_font[:stack]};
+          }
+        EOF
+      end
+
+      if heading_font.present?
+        contents << <<~EOF
+          #{font_css(heading_font)}
+
+          :root {
+            --heading-font-family: #{heading_font[:stack]};
+          }
+        EOF
+      end
+
+      contents
+    end
+
+    def wizard_fonts
+      contents = +""
+
+      DiscourseFonts.fonts.each do |font|
+        if font[:key] == "system"
+          # Overwrite font definition because the preview canvases in the wizard require explicit @font-face definitions.
+          # uses same technique as https://github.com/jonathantneal/system-font-css
+          font[:variants] = [
+            { src: 'local(".SFNS-Regular"), local(".SFNSText-Regular"), local(".HelveticaNeueDeskInterface-Regular"), local(".LucidaGrandeUI"), local("Segoe UI"), local("Ubuntu"), local("Roboto-Regular"), local("DroidSans"), local("Tahoma")', weight: 400 },
+            { src: 'local(".SFNS-Bold"), local(".SFNSText-Bold"), local(".HelveticaNeueDeskInterface-Bold"), local(".LucidaGrandeUI"), local("Segoe UI Bold"), local("Ubuntu Bold"), local("Roboto-Bold"), local("DroidSans-Bold"), local("Tahoma Bold")', weight: 700 }
+          ]
+        end
+
+        contents << font_css(font)
+        contents << <<~EOF
+          .body-font-#{font[:key].tr("_", "-")} {
+            font-family: #{font[:stack]};
+          }
+          .heading-font-#{font[:key].tr("_", "-")} h2 {
+            font-family: #{font[:stack]};
+          }
+        EOF
+      end
+
+      contents
+    end
+
+    def category_backgrounds
+      contents = +""
+      Category.where('uploaded_background_id IS NOT NULL').each do |c|
+        contents << category_css(c) if c.uploaded_background&.url.present?
+      end
+
+      contents
+    end
+
     def import_color_definitions
       contents = +""
       DiscoursePluginRegistry.color_definition_stylesheets.each do |name, path|
-        contents << "// Color definitions from #{name}\n\n"
+        contents << "\n\n// Color definitions from #{name}\n\n"
         contents << File.read(path.to_s)
         contents << "\n\n"
       end
@@ -119,7 +119,7 @@ module Stylesheet
         theme = Theme.find_by_id(theme_id)
         contents << theme&.scss_variables.to_s
         Theme.list_baked_fields(resolved_ids, :common, :color_definitions).each do |field|
-          contents << "// Color definitions from #{field.theme.name}\n\n"
+          contents << "\n\n// Color definitions from #{field.theme.name}\n\n"
 
           if field.theme_id == theme.id
             contents << field.value

spec/components/stylesheet/importer_spec.rb

@@ -9,58 +9,70 @@ describe Stylesheet::Importer do
     Stylesheet::Compiler.compile_asset(name)[0]
   end
 
-  it "applies CDN to background category images" do
+  context "#category_backgrounds" do
-    expect(compile_css("category_backgrounds")).to_not include("background-image")
+    it "applies CDN to background category images" do
+      expect(compile_css("mobile")).to_not include("body.category-")
+      expect(compile_css("desktop")).to_not include("body.category-")
 
-    background = Fabricate(:upload)
+      background = Fabricate(:upload)
-    parent_category = Fabricate(:category)
+      parent_category = Fabricate(:category)
-    category = Fabricate(:category, parent_category_id: parent_category.id, uploaded_background: background)
+      category = Fabricate(:category, parent_category_id: parent_category.id, uploaded_background: background)
 
-    expect(compile_css("category_backgrounds")).to include("body.category-#{parent_category.slug}-#{category.slug}{background-image:url(#{background.url})}")
+      expect(compile_css("mobile")).to include("body.category-#{parent_category.slug}-#{category.slug}{background-image:url(#{background.url})}")
+      expect(compile_css("desktop")).to include("body.category-#{parent_category.slug}-#{category.slug}{background-image:url(#{background.url})}")
+
+      GlobalSetting.stubs(:cdn_url).returns("//awesome.cdn")
+      expect(compile_css("mobile")).to include("body.category-#{parent_category.slug}-#{category.slug}{background-image:url(//awesome.cdn#{background.url})}")
+      expect(compile_css("desktop")).to include("body.category-#{parent_category.slug}-#{category.slug}{background-image:url(//awesome.cdn#{background.url})}")
+    end
+
+    it "applies S3 CDN to background category images" do
+      setup_s3
+      SiteSetting.s3_use_iam_profile = true
+      SiteSetting.s3_upload_bucket = 'test'
+      SiteSetting.s3_region = 'ap-southeast-2'
+      SiteSetting.s3_cdn_url = "https://s3.cdn"
+
+      background = Fabricate(:upload_s3)
+      category = Fabricate(:category, uploaded_background: background)
+
+      expect(compile_css("mobile")).to include("body.category-#{category.slug}{background-image:url(https://s3.cdn/original")
+      expect(compile_css("desktop")).to include("body.category-#{category.slug}{background-image:url(https://s3.cdn/original")
+    end
 
-    GlobalSetting.stubs(:cdn_url).returns("//awesome.cdn")
-    expect(compile_css("category_backgrounds")).to include("body.category-#{parent_category.slug}-#{category.slug}{background-image:url(//awesome.cdn#{background.url})}")
   end
 
-  it "applies S3 CDN to background category images" do
+  context "#font" do
-    setup_s3
+    it "includes font variable" do
-    SiteSetting.s3_use_iam_profile = true
+      default_font = ":root{--font-family: Arial, sans-serif}"
-    SiteSetting.s3_upload_bucket = 'test'
+      expect(compile_css("desktop")).to include(default_font)
-    SiteSetting.s3_region = 'ap-southeast-2'
+      expect(compile_css("mobile")).to include(default_font)
-    SiteSetting.s3_cdn_url = "https://s3.cdn"
+      expect(compile_css("embed")).to include(default_font)
+      expect(compile_css("publish")).to include(default_font)
+    end
 
-    background = Fabricate(:upload_s3)
+    it "includes separate body and heading font declarations" do
-    category = Fabricate(:category, uploaded_background: background)
+      base_font = DiscourseFonts.fonts[2]
+      heading_font = DiscourseFonts.fonts[3]
 
-    expect(compile_css("category_backgrounds")).to include("body.category-#{category.slug}{background-image:url(https://s3.cdn/original")
+      SiteSetting.base_font = base_font[:key]
-  end
+      SiteSetting.heading_font = heading_font[:key]
 
-  it "includes font variable" do
+      expect(compile_css("desktop"))
-    expect(compile_css("desktop"))
+        .to include(":root{--font-family: #{base_font[:stack]}}")
-      .to include(":root{--font-family: Arial, sans-serif}")
+        .and include(":root{--heading-font-family: #{heading_font[:stack]}}")
-  end
+    end
 
-  it "includes separate body and heading font declarations" do
+    it "includes all fonts in wizard" do
-    base_font = DiscourseFonts.fonts[2]
+      expect(compile_css("wizard").scan(/\.body-font-/).count)
-    heading_font = DiscourseFonts.fonts[3]
+        .to eq(DiscourseFonts.fonts.count)
 
-    SiteSetting.base_font = base_font[:key]
+      expect(compile_css("wizard").scan(/\.heading-font-/).count)
-    SiteSetting.heading_font = heading_font[:key]
+        .to eq(DiscourseFonts.fonts.count)
 
-    expect(compile_css("desktop"))
+      expect(compile_css("wizard").scan(/@font-face/).count)
-      .to include(":root{--font-family: #{base_font[:stack]}}")
+        .to eq(DiscourseFonts.fonts.map { |f| f[:variants]&.count || 0 }.sum)
-      .and include(":root{--heading-font-family: #{heading_font[:stack]}}")
+    end
-  end
-
-  it "includes all fonts in wizard" do
-    expect(compile_css("wizard").scan(/\.body-font-/).count)
-      .to eq(DiscourseFonts.fonts.count)
-
-    expect(compile_css("wizard").scan(/\.heading-font-/).count)
-      .to eq(DiscourseFonts.fonts.count)
-
-    expect(compile_css("wizard").scan(/@font-face/).count)
-      .to eq(DiscourseFonts.fonts.map { |f| f[:variants]&.count || 0 }.sum)
   end
 
   context "#import_color_definitions" do