From 0cfc42e0e63d5d1671e5634d76d8977b88e13039 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9rgio=20Saquetim?= <1108771+megothss@users.noreply.github.com> Date: Fri, 20 Oct 2023 09:48:06 -0300 Subject: [PATCH] FEATURE: Add dark mode option for category backgrounds (#24003) Adds a new upload field for a dark mode category background that will be used as an alternative when Discourse is using a dark mode theme. --- .../app/components/edit-category-images.hbs | 11 ++ .../app/components/edit-category-images.js | 13 ++ .../discourse/app/models/category.js | 1 + app/controllers/categories_controller.rb | 1 + app/models/category.rb | 11 +- app/models/category_list.rb | 1 + app/models/site.rb | 1 + app/serializers/basic_category_serializer.rb | 1 + config/locales/client.en.yml | 1 + ..._add_dark_mode_background_to_categories.rb | 7 + lib/stylesheet/compiler.rb | 2 +- lib/stylesheet/importer.rb | 24 +++- lib/tasks/site.rake | 1 + lib/tasks/uploads.rake | 1 + spec/jobs/clean_up_uploads_spec.rb | 10 ++ spec/lib/stylesheet/importer_spec.rb | 122 ++++++++++++++++-- spec/models/upload_reference_spec.rb | 6 +- .../json/category_create_response.json | 9 +- .../schemas/json/category_list_response.json | 9 +- .../json/category_update_response.json | 9 +- .../api/schemas/json/site_response.json | 7 + 21 files changed, 228 insertions(+), 20 deletions(-) create mode 100644 db/migrate/20231018225833_add_dark_mode_background_to_categories.rb diff --git a/app/assets/javascripts/discourse/app/components/edit-category-images.hbs b/app/assets/javascripts/discourse/app/components/edit-category-images.hbs index 460c6f2bdc9..3fd3a6c4bef 100644 --- a/app/assets/javascripts/discourse/app/components/edit-category-images.hbs +++ b/app/assets/javascripts/discourse/app/components/edit-category-images.hbs @@ -31,4 +31,15 @@ @type="category_background" @id="category-background-uploader" /> + + +
+ +
\ No newline at end of file diff --git a/app/assets/javascripts/discourse/app/components/edit-category-images.js b/app/assets/javascripts/discourse/app/components/edit-category-images.js index a11669854f5..ea4adbee45d 100644 --- a/app/assets/javascripts/discourse/app/components/edit-category-images.js +++ b/app/assets/javascripts/discourse/app/components/edit-category-images.js @@ -8,6 +8,11 @@ export default buildCategoryPanel("images").extend({ return uploadedBackgroundUrl || ""; }, + @discourseComputed("category.uploaded_background_dark.url") + backgroundDarkImageUrl(uploadedBackgroundDarkUrl) { + return uploadedBackgroundDarkUrl || ""; + }, + @discourseComputed("category.uploaded_logo.url") logoImageUrl(uploadedLogoUrl) { return uploadedLogoUrl || ""; @@ -42,6 +47,14 @@ export default buildCategoryPanel("images").extend({ backgroundUploadDeleted() { this._deleteUpload("category.uploaded_background"); }, + + backgroundDarkUploadDone(upload) { + this._setFromUpload("category.uploaded_background_dark", upload); + }, + + backgroundDarkUploadDeleted() { + this._deleteUpload("category.uploaded_background_dark"); + }, }, _deleteUpload(path) { diff --git a/app/assets/javascripts/discourse/app/models/category.js b/app/assets/javascripts/discourse/app/models/category.js index cfdc6927f94..ac9af062e85 100644 --- a/app/assets/javascripts/discourse/app/models/category.js +++ b/app/assets/javascripts/discourse/app/models/category.js @@ -232,6 +232,7 @@ const Category = RestModel.extend({ uploaded_logo_id: this.get("uploaded_logo.id"), uploaded_logo_dark_id: this.get("uploaded_logo_dark.id"), uploaded_background_id: this.get("uploaded_background.id"), + uploaded_background_dark_id: this.get("uploaded_background_dark.id"), allow_badges: this.allow_badges, category_setting_attributes: this.category_setting, custom_fields: this.custom_fields, diff --git a/app/controllers/categories_controller.rb b/app/controllers/categories_controller.rb index 5d6086d0e41..6622fb78411 100644 --- a/app/controllers/categories_controller.rb +++ b/app/controllers/categories_controller.rb @@ -459,6 +459,7 @@ class CategoriesController < ApplicationController :uploaded_logo_id, :uploaded_logo_dark_id, :uploaded_background_id, + :uploaded_background_dark_id, :slug, :allow_badges, :topic_template, diff --git a/app/models/category.rb b/app/models/category.rb index 92e888ebd73..687d0ad56e4 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -29,6 +29,7 @@ class Category < ActiveRecord::Base belongs_to :uploaded_logo, class_name: "Upload" belongs_to :uploaded_logo_dark, class_name: "Upload" belongs_to :uploaded_background, class_name: "Upload" + belongs_to :uploaded_background_dark, class_name: "Upload" has_many :topics has_many :category_users @@ -115,8 +116,13 @@ class Category < ActiveRecord::Base after_save do if saved_change_to_uploaded_logo_id? || saved_change_to_uploaded_logo_dark_id? || - saved_change_to_uploaded_background_id? - upload_ids = [self.uploaded_logo_id, self.uploaded_logo_dark_id, self.uploaded_background_id] + saved_change_to_uploaded_background_id? || saved_change_to_uploaded_background_dark_id? + upload_ids = [ + self.uploaded_logo_id, + self.uploaded_logo_dark_id, + self.uploaded_background_id, + self.uploaded_background_dark_id, + ] UploadReference.ensure_exist!(upload_ids: upload_ids, target: self) end end @@ -1185,6 +1191,7 @@ end # allow_unlimited_owner_edits_on_first_post :boolean default(FALSE), not null # default_slow_mode_seconds :integer # uploaded_logo_dark_id :integer +# uploaded_background_dark_id :integer # # Indexes # diff --git a/app/models/category_list.rb b/app/models/category_list.rb index 8eee48792e2..b6e8e7d9e13 100644 --- a/app/models/category_list.rb +++ b/app/models/category_list.rb @@ -16,6 +16,7 @@ class CategoryList def self.included_associations [ :uploaded_background, + :uploaded_background_dark, :uploaded_logo, :uploaded_logo_dark, :topic_only_relative_url, diff --git a/app/models/site.rb b/app/models/site.rb index 07c82ff0914..526a5fc9988 100644 --- a/app/models/site.rb +++ b/app/models/site.rb @@ -81,6 +81,7 @@ class Site :uploaded_logo, :uploaded_logo_dark, :uploaded_background, + :uploaded_background_dark, :tags, :tag_groups, :form_templates, diff --git a/app/serializers/basic_category_serializer.rb b/app/serializers/basic_category_serializer.rb index 06dfa9628eb..2ec27126888 100644 --- a/app/serializers/basic_category_serializer.rb +++ b/app/serializers/basic_category_serializer.rb @@ -35,6 +35,7 @@ class BasicCategorySerializer < ApplicationSerializer has_one :uploaded_logo, embed: :object, serializer: CategoryUploadSerializer has_one :uploaded_logo_dark, embed: :object, serializer: CategoryUploadSerializer has_one :uploaded_background, embed: :object, serializer: CategoryUploadSerializer + has_one :uploaded_background_dark, embed: :object, serializer: CategoryUploadSerializer def include_parent_category_id? parent_category_id diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 2f449543cef..df17fe18e17 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -3734,6 +3734,7 @@ en: logo: "Category Logo Image" logo_dark: "Dark Mode Category Logo Image" background_image: "Category Background Image" + background_image_dark: "Dark Category Background Image" badge_colors: "Badge colors" background_color: "Background color" foreground_color: "Foreground color" diff --git a/db/migrate/20231018225833_add_dark_mode_background_to_categories.rb b/db/migrate/20231018225833_add_dark_mode_background_to_categories.rb new file mode 100644 index 00000000000..6351e08a703 --- /dev/null +++ b/db/migrate/20231018225833_add_dark_mode_background_to_categories.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddDarkModeBackgroundToCategories < ActiveRecord::Migration[7.0] + def change + add_column :categories, :uploaded_background_dark_id, :integer, index: true + end +end diff --git a/lib/stylesheet/compiler.rb b/lib/stylesheet/compiler.rb index d395c388d00..8c67fd2dbb2 100644 --- a/lib/stylesheet/compiler.rb +++ b/lib/stylesheet/compiler.rb @@ -34,7 +34,7 @@ module Stylesheet when Stylesheet::Manager::COLOR_SCHEME_STYLESHEET file += importer.import_color_definitions file += importer.import_wcag_overrides - file += importer.category_backgrounds + file += importer.category_backgrounds(options[:color_scheme_id]) file += importer.font end end diff --git a/lib/stylesheet/importer.rb b/lib/stylesheet/importer.rb index 8024e119391..90fdcdddf1e 100644 --- a/lib/stylesheet/importer.rb +++ b/lib/stylesheet/importer.rb @@ -95,11 +95,16 @@ module Stylesheet contents end - def category_backgrounds + def category_backgrounds(color_scheme_id) + is_dark_color_scheme = + color_scheme_id.present? && ColorScheme.find_by_id(color_scheme_id)&.is_dark? + contents = +"" Category .where("uploaded_background_id IS NOT NULL") - .each { |c| contents << category_css(c) if c.uploaded_background&.url.present? } + .each do |c| + contents << category_css(c, is_dark_color_scheme) if c.uploaded_background&.url.present? + end contents end @@ -215,9 +220,20 @@ module Stylesheet @theme == :nil ? nil : @theme end - def category_css(category) + def category_css(category, is_dark_color_scheme) full_slug = category.full_slug.split("-")[0..-2].join("-") - "body.category-#{full_slug} { background-image: url(#{upload_cdn_path(category.uploaded_background.url)}) }\n" + + # in case we're using a dark color scheme, we define the background using the dark image + # if one is available. Otherwise, we use the light image by default. + if is_dark_color_scheme && category.uploaded_background_dark&.url.present? + return category_background_css(full_slug, category.uploaded_background_dark.url) + end + + category_background_css(full_slug, category.uploaded_background.url) + end + + def category_background_css(full_slug, background_url) + "body.category-#{full_slug} { background-image: url(#{upload_cdn_path(background_url)}) }" end def font_css(font) diff --git a/lib/tasks/site.rake b/lib/tasks/site.rake index 94d78a034e3..6f6ef037063 100644 --- a/lib/tasks/site.rake +++ b/lib/tasks/site.rake @@ -220,6 +220,7 @@ task "site:export_structure", [:zip_path] => :environment do |task, args| uploaded_logo_id: data.set_upload(c.uploaded_logo_id), uploaded_logo_dark_id: data.set_upload(c.uploaded_logo_dark_id), uploaded_background_id: data.set_upload(c.uploaded_background_id), + uploaded_background_dark_id: data.set_upload(c.uploaded_background_dark_id), topic_featured_link_allowed: c.topic_featured_link_allowed, all_topics_wiki: c.all_topics_wiki, show_subcategory_list: c.show_subcategory_list, diff --git a/lib/tasks/uploads.rake b/lib/tasks/uploads.rake index b795bacc4b2..3e869e54cf7 100644 --- a/lib/tasks/uploads.rake +++ b/lib/tasks/uploads.rake @@ -965,6 +965,7 @@ def analyze_missing_s3 %i[categories uploaded_logo_id], %i[categories uploaded_logo_dark_id], %i[categories uploaded_background_id], + %i[categories uploaded_background_dark_id], %i[custom_emojis upload_id], %i[theme_fields upload_id], %i[user_exports upload_id], diff --git a/spec/jobs/clean_up_uploads_spec.rb b/spec/jobs/clean_up_uploads_spec.rb index a0d4befcdc3..837ae807fdd 100644 --- a/spec/jobs/clean_up_uploads_spec.rb +++ b/spec/jobs/clean_up_uploads_spec.rb @@ -229,6 +229,16 @@ RSpec.describe Jobs::CleanUpUploads do expect(Upload.exists?(id: category_background_upload.id)).to eq(true) end + it "does not delete category dark background uploads" do + category_background_dark_upload = fabricate_upload + Fabricate(:category, uploaded_background_dark: category_background_dark_upload) + + Jobs::CleanUpUploads.new.execute(nil) + + expect(Upload.exists?(id: expired_upload.id)).to eq(false) + expect(Upload.exists?(id: category_background_dark_upload.id)).to eq(true) + end + it "does not delete post uploads" do upload = fabricate_upload Fabricate(:post, uploads: [upload]) diff --git a/spec/lib/stylesheet/importer_spec.rb b/spec/lib/stylesheet/importer_spec.rb index 96d422ec3e2..53f1fc3837c 100644 --- a/spec/lib/stylesheet/importer_spec.rb +++ b/spec/lib/stylesheet/importer_spec.rb @@ -3,33 +3,114 @@ require "stylesheet/importer" RSpec.describe Stylesheet::Importer do - def compile_css(name) - Stylesheet::Compiler.compile_asset(name)[0] + def compile_css(name, options = {}) + Stylesheet::Compiler.compile_asset(name, options)[0] end describe "#category_backgrounds" do - it "applies CDN to background category images" do - expect(compile_css("color_definitions")).to_not include("body.category-") - + it "uses the correct background image based in the color scheme" do background = Fabricate(:upload) + background_dark = Fabricate(:upload) + parent_category = Fabricate(:category) category = Fabricate( :category, parent_category_id: parent_category.id, uploaded_background: background, + uploaded_background_dark: background_dark, ) - expect(compile_css("color_definitions")).to include( + # light color schemes + ["Neutral", "Shades of Blue", "WCAG", "Summer", "Solarized Light"].each do |scheme_name| + scheme = ColorScheme.create_from_base(name: "Light Test", base_scheme_id: scheme_name) + + compiled_css = compile_css("color_definitions", { color_scheme_id: scheme.id }) + + expect(compiled_css).to include( + "body.category-#{parent_category.slug}-#{category.slug}{background-image:url(#{background.url})}", + ) + expect(compiled_css).not_to include(background_dark.url) + end + + # dark color schemes + [ + "Dark", + "Grey Amber", + "Latte", + "Dark Rose", + "WCAG Dark", + "Dracula", + "Solarized Dark", + ].each do |scheme_name| + scheme = ColorScheme.create_from_base(name: "Light Test", base_scheme_id: scheme_name) + + compiled_css = compile_css("color_definitions", { color_scheme_id: scheme.id }) + + expect(compiled_css).not_to include(background.url) + expect(compiled_css).to include( + "body.category-#{parent_category.slug}-#{category.slug}{background-image:url(#{background_dark.url})}", + ) + end + end + + it "applies CDN to background category images" do + expect(compile_css("color_definitions")).to_not include("body.category-") + + background = Fabricate(:upload) + background_dark = Fabricate(:upload) + + parent_category = Fabricate(:category) + category = + Fabricate( + :category, + parent_category_id: parent_category.id, + uploaded_background: background, + uploaded_background_dark: background_dark, + ) + + compiled_css = compile_css("color_definitions") + expect(compiled_css).to include( "body.category-#{parent_category.slug}-#{category.slug}{background-image:url(#{background.url})}", ) GlobalSetting.stubs(:cdn_url).returns("//awesome.cdn") - expect(compile_css("color_definitions")).to include( + compiled_css = compile_css("color_definitions") + expect(compiled_css).to include( "body.category-#{parent_category.slug}-#{category.slug}{background-image:url(//awesome.cdn#{background.url})}", ) end + it "applies CDN to dark background category images" do + scheme = ColorScheme.create_from_base(name: "Dark Test", base_scheme_id: "Dark") + expect(compile_css("color_definitions", { color_scheme_id: scheme.id })).to_not include( + "body.category-", + ) + + background = Fabricate(:upload) + background_dark = Fabricate(:upload) + + parent_category = Fabricate(:category) + category = + Fabricate( + :category, + parent_category_id: parent_category.id, + uploaded_background: background, + uploaded_background_dark: background_dark, + ) + + compiled_css = compile_css("color_definitions", { color_scheme_id: scheme.id }) + expect(compiled_css).to include( + "body.category-#{parent_category.slug}-#{category.slug}{background-image:url(#{background_dark.url})}", + ) + + GlobalSetting.stubs(:cdn_url).returns("//awesome.cdn") + compiled_css = compile_css("color_definitions", { color_scheme_id: scheme.id }) + expect(compiled_css).to include( + "body.category-#{parent_category.slug}-#{category.slug}{background-image:url(//awesome.cdn#{background_dark.url})}", + ) + end + it "applies S3 CDN to background category images" do setup_s3 SiteSetting.s3_use_iam_profile = true @@ -40,7 +121,32 @@ RSpec.describe Stylesheet::Importer do background = Fabricate(:upload_s3) category = Fabricate(:category, uploaded_background: background) - expect(compile_css("color_definitions")).to include( + compiled_css = compile_css("color_definitions") + expect(compiled_css).to include( + "body.category-#{category.slug}{background-image:url(https://s3.cdn/original", + ) + end + + it "applies S3 CDN to dark background category images" do + scheme = ColorScheme.create_from_base(name: "Dark Test", base_scheme_id: "WCAG Dark") + + 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) + background_dark = Fabricate(:upload_s3) + category = + Fabricate( + :category, + uploaded_background: background, + uploaded_background_dark: background_dark, + ) + + compiled_css = compile_css("color_definitions", { color_scheme_id: scheme.id }) + expect(compiled_css).to include( "body.category-#{category.slug}{background-image:url(https://s3.cdn/original", ) end diff --git a/spec/models/upload_reference_spec.rb b/spec/models/upload_reference_spec.rb index c7675a4577a..9b761a2953b 100644 --- a/spec/models/upload_reference_spec.rb +++ b/spec/models/upload_reference_spec.rb @@ -22,6 +22,7 @@ RSpec.describe UploadReference do fab!(:upload1) { Fabricate(:upload) } fab!(:upload2) { Fabricate(:upload) } fab!(:upload3) { Fabricate(:upload) } + fab!(:upload4) { Fabricate(:upload) } it "creates upload references" do category = nil @@ -32,13 +33,14 @@ RSpec.describe UploadReference do uploaded_logo_id: upload1.id, uploaded_logo_dark_id: upload2.id, uploaded_background_id: upload3.id, + uploaded_background_dark_id: upload4.id, ) - }.to change { UploadReference.count }.by(3) + }.to change { UploadReference.count }.by(4) upload_reference = UploadReference.last expect(upload_reference.target).to eq(category) - expect { category.destroy! }.to change { UploadReference.count }.by(-3) + expect { category.destroy! }.to change { UploadReference.count }.by(-4) end end diff --git a/spec/requests/api/schemas/json/category_create_response.json b/spec/requests/api/schemas/json/category_create_response.json index bc2da289014..1e91ec3c331 100644 --- a/spec/requests/api/schemas/json/category_create_response.json +++ b/spec/requests/api/schemas/json/category_create_response.json @@ -260,6 +260,12 @@ "string", "null" ] + }, + "uploaded_background_dark": { + "type": [ + "string", + "null" + ] } }, "required": [ @@ -310,7 +316,8 @@ "search_priority", "uploaded_logo", "uploaded_logo_dark", - "uploaded_background" + "uploaded_background", + "uploaded_background_dark" ] } }, diff --git a/spec/requests/api/schemas/json/category_list_response.json b/spec/requests/api/schemas/json/category_list_response.json index ab9da835689..0896cc5d10b 100644 --- a/spec/requests/api/schemas/json/category_list_response.json +++ b/spec/requests/api/schemas/json/category_list_response.json @@ -167,6 +167,12 @@ "string", "null" ] + }, + "uploaded_background_dark": { + "type": [ + "string", + "null" + ] } }, "required": [ @@ -206,7 +212,8 @@ "subcategory_ids", "uploaded_logo", "uploaded_logo_dark", - "uploaded_background" + "uploaded_background", + "uploaded_background_dark" ] } } diff --git a/spec/requests/api/schemas/json/category_update_response.json b/spec/requests/api/schemas/json/category_update_response.json index 9ed80aa85fd..550382ef901 100644 --- a/spec/requests/api/schemas/json/category_update_response.json +++ b/spec/requests/api/schemas/json/category_update_response.json @@ -263,6 +263,12 @@ "string", "null" ] + }, + "uploaded_background_dark": { + "type": [ + "string", + "null" + ] } }, "required": [ @@ -314,7 +320,8 @@ "search_priority", "uploaded_logo", "uploaded_logo_dark", - "uploaded_background" + "uploaded_background", + "uploaded_background_dark" ] } }, diff --git a/spec/requests/api/schemas/json/site_response.json b/spec/requests/api/schemas/json/site_response.json index 29194ce1c97..2004f7e0153 100644 --- a/spec/requests/api/schemas/json/site_response.json +++ b/spec/requests/api/schemas/json/site_response.json @@ -701,6 +701,12 @@ "null" ] }, + "uploaded_background_dark": { + "type": [ + "string", + "null" + ] + }, "can_edit": { "type": "boolean" }, @@ -752,6 +758,7 @@ "uploaded_logo", "uploaded_logo_dark", "uploaded_background", + "uploaded_background_dark", "can_edit" ] }