From 4071af1d099a1c6a0c7bf664ceafa8de264b9744 Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Wed, 10 Mar 2021 11:05:56 -0500 Subject: [PATCH] DEV: Refactor font and category background importers (#12312) --- app/assets/stylesheets/common.scss | 1 - app/assets/stylesheets/desktop.scss | 4 - app/assets/stylesheets/embed.scss | 1 - app/assets/stylesheets/mobile.scss | 4 - app/assets/stylesheets/wizard.scss | 1 - lib/stylesheet/compiler.rb | 10 ++ lib/stylesheet/importer.rb | 132 ++++++++++---------- spec/components/stylesheet/importer_spec.rb | 94 ++++++++------ 8 files changed, 129 insertions(+), 118 deletions(-) diff --git a/app/assets/stylesheets/common.scss b/app/assets/stylesheets/common.scss index d37bc380712..964b2c1529e 100644 --- a/app/assets/stylesheets/common.scss +++ b/app/assets/stylesheets/common.scss @@ -1,4 +1,3 @@ -@import "font"; @import "vendor/normalize"; @import "vendor/normalize-ext"; @import "vendor/pikaday"; diff --git a/app/assets/stylesheets/desktop.scss b/app/assets/stylesheets/desktop.scss index d220dc862c0..dfae3eca774 100644 --- a/app/assets/stylesheets/desktop.scss +++ b/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"; diff --git a/app/assets/stylesheets/embed.scss b/app/assets/stylesheets/embed.scss index fe8d44ea116..169856775d9 100644 --- a/app/assets/stylesheets/embed.scss +++ b/app/assets/stylesheets/embed.scss @@ -1,4 +1,3 @@ -@import "font"; @import "./vendor/normalize"; @import "./vendor/normalize-ext"; @import "./common/foundation/base"; diff --git a/app/assets/stylesheets/mobile.scss b/app/assets/stylesheets/mobile.scss index 402f77169fd..5b73a33dd16 100644 --- a/app/assets/stylesheets/mobile.scss +++ b/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"; diff --git a/app/assets/stylesheets/wizard.scss b/app/assets/stylesheets/wizard.scss index 0ec2ef2a7be..648a4d736f2 100644 --- a/app/assets/stylesheets/wizard.scss +++ b/app/assets/stylesheets/wizard.scss @@ -1,4 +1,3 @@ -@import "wizard_fonts"; @import "color_definitions"; @import "vendor/normalize"; @import "vendor/normalize-ext"; diff --git a/lib/stylesheet/compiler.rb b/lib/stylesheet/compiler.rb index 5be3db27b77..8f9f0255183 100644 --- a/lib/stylesheet/compiler.rb +++ b/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 diff --git a/lib/stylesheet/importer.rb b/lib/stylesheet/importer.rb index d339fe8e654..750957e2e29 100644 --- a/lib/stylesheet/importer.rb +++ b/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 diff --git a/spec/components/stylesheet/importer_spec.rb b/spec/components/stylesheet/importer_spec.rb index 8f8375b9f89..562140f37f9 100644 --- a/spec/components/stylesheet/importer_spec.rb +++ b/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 - expect(compile_css("category_backgrounds")).to_not include("background-image") + context "#category_backgrounds" do + 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) - parent_category = Fabricate(:category) - category = Fabricate(:category, parent_category_id: parent_category.id, uploaded_background: background) + background = Fabricate(:upload) + parent_category = Fabricate(:category) + 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 - 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" + context "#font" do + it "includes font variable" do + default_font = ":root{--font-family: Arial, sans-serif}" + expect(compile_css("desktop")).to include(default_font) + expect(compile_css("mobile")).to include(default_font) + expect(compile_css("embed")).to include(default_font) + expect(compile_css("publish")).to include(default_font) + end - background = Fabricate(:upload_s3) - category = Fabricate(:category, uploaded_background: background) + it "includes separate body and heading font declarations" do + 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") - end + SiteSetting.base_font = base_font[:key] + SiteSetting.heading_font = heading_font[:key] - it "includes font variable" do - expect(compile_css("desktop")) - .to include(":root{--font-family: Arial, sans-serif}") - end + expect(compile_css("desktop")) + .to include(":root{--font-family: #{base_font[:stack]}}") + .and include(":root{--heading-font-family: #{heading_font[:stack]}}") + end - it "includes separate body and heading font declarations" do - base_font = DiscourseFonts.fonts[2] - heading_font = DiscourseFonts.fonts[3] + it "includes all fonts in wizard" do + expect(compile_css("wizard").scan(/\.body-font-/).count) + .to eq(DiscourseFonts.fonts.count) - SiteSetting.base_font = base_font[:key] - SiteSetting.heading_font = heading_font[:key] + expect(compile_css("wizard").scan(/\.heading-font-/).count) + .to eq(DiscourseFonts.fonts.count) - expect(compile_css("desktop")) - .to include(":root{--font-family: #{base_font[:stack]}}") - .and include(":root{--heading-font-family: #{heading_font[:stack]}}") - 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) + expect(compile_css("wizard").scan(/@font-face/).count) + .to eq(DiscourseFonts.fonts.map { |f| f[:variants]&.count || 0 }.sum) + end end context "#import_color_definitions" do