From 6fdc711b4a27cc5f6b5450c8ab978657e450b890 Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Thu, 6 Aug 2020 09:45:37 -0400 Subject: [PATCH] FEATURE: Allow users to opt out of automatic dark mode (#10377) --- .../app/controllers/preferences/interface.js | 20 ++++++++++ .../javascripts/discourse/app/models/user.js | 1 + .../app/templates/preferences/interface.hbs | 9 +++++ app/helpers/application_helper.rb | 4 +- app/serializers/user_option_serializer.rb | 1 + app/services/user_updater.rb | 1 + config/locales/client.en.yml | 2 + config/site_settings.yml | 1 + ...1752_add_dark_scheme_id_to_user_options.rb | 7 ++++ lib/stylesheet/manager.rb | 10 ++++- spec/components/stylesheet/manager_spec.rb | 9 ++++- spec/helpers/application_helper_spec.rb | 37 +++++++++++++++++++ .../acceptance/preferences-test.js | 13 +++++++ 13 files changed, 110 insertions(+), 5 deletions(-) create mode 100644 db/migrate/20200805151752_add_dark_scheme_id_to_user_options.rb diff --git a/app/assets/javascripts/discourse/app/controllers/preferences/interface.js b/app/assets/javascripts/discourse/app/controllers/preferences/interface.js index 9baf458d696..5d41ab7b128 100644 --- a/app/assets/javascripts/discourse/app/controllers/preferences/interface.js +++ b/app/assets/javascripts/discourse/app/controllers/preferences/interface.js @@ -33,6 +33,7 @@ export default Controller.extend({ let attrs = [ "locale", "external_links_in_new_tab", + "dark_scheme_id", "dynamic_favicon", "enable_quoting", "enable_defer", @@ -149,6 +150,20 @@ export default Controller.extend({ return result; }, + @discourseComputed + showDarkModeToggle() { + return this.siteSettings.default_dark_mode_color_scheme_id > 0; + }, + + enableDarkMode: computed({ + set(key, value) { + return value; + }, + get() { + return this.get("model.user_option.dark_scheme_id") === -1 ? false : true; + } + }), + actions: { save() { this.set("saved", false); @@ -162,6 +177,11 @@ export default Controller.extend({ this.set("model.user_option.text_size", this.textSize); } + this.set( + "model.user_option.dark_scheme_id", + this.enableDarkMode ? null : -1 + ); + return this.model .save(this.saveAttrNames) .then(() => { diff --git a/app/assets/javascripts/discourse/app/models/user.js b/app/assets/javascripts/discourse/app/models/user.js index c4bf6b5f584..58195f7b7a9 100644 --- a/app/assets/javascripts/discourse/app/models/user.js +++ b/app/assets/javascripts/discourse/app/models/user.js @@ -300,6 +300,7 @@ const User = RestModel.extend({ "email_messages_level", "email_level", "email_previous_replies", + "dark_scheme_id", "dynamic_favicon", "enable_quoting", "enable_defer", diff --git a/app/assets/javascripts/discourse/app/templates/preferences/interface.hbs b/app/assets/javascripts/discourse/app/templates/preferences/interface.hbs index 7f198baa4a8..c3d23b12ff2 100644 --- a/app/assets/javascripts/discourse/app/templates/preferences/interface.hbs +++ b/app/assets/javascripts/discourse/app/templates/preferences/interface.hbs @@ -20,6 +20,15 @@ {{/if}} +{{#if showDarkModeToggle}} +
+ +
+ {{preference-checkbox labelKey="user.dark_mode_enable" checked=enableDarkMode}} +
+
+{{/if}} +
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 15513b3fac8..84d7909fa7e 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -454,7 +454,9 @@ module ApplicationHelper result = +"" result << Stylesheet::Manager.color_scheme_stylesheet_link_tag(scheme_id) - dark_scheme_id = SiteSetting.default_dark_mode_color_scheme_id + user_dark_scheme_id = current_user&.user_option&.dark_scheme_id + dark_scheme_id = user_dark_scheme_id || SiteSetting.default_dark_mode_color_scheme_id + if dark_scheme_id != -1 result << Stylesheet::Manager.color_scheme_stylesheet_link_tag(dark_scheme_id, '(prefers-color-scheme: dark)') end diff --git a/app/serializers/user_option_serializer.rb b/app/serializers/user_option_serializer.rb index c95f454d089..0187bcc9e9a 100644 --- a/app/serializers/user_option_serializer.rb +++ b/app/serializers/user_option_serializer.rb @@ -8,6 +8,7 @@ class UserOptionSerializer < ApplicationSerializer :email_level, :email_messages_level, :external_links_in_new_tab, + :dark_scheme_id, :dynamic_favicon, :enable_quoting, :enable_defer, diff --git a/app/services/user_updater.rb b/app/services/user_updater.rb index 3b252a71de6..32446e0395b 100644 --- a/app/services/user_updater.rb +++ b/app/services/user_updater.rb @@ -25,6 +25,7 @@ class UserUpdater :external_links_in_new_tab, :enable_quoting, :enable_defer, + :dark_scheme_id, :dynamic_favicon, :automatically_unpin_topics, :digest_after_minutes, diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 8695fda8833..1bb1d5b2b28 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -909,6 +909,8 @@ en: first_notification: "Your first notification! Select it to begin." dynamic_favicon: "Show counts on browser icon" theme_default_on_all_devices: "Make this the default theme on all my devices" + dark_mode: "Dark Mode" + dark_mode_enable: "Enable automatic dark mode color scheme" text_size_default_on_all_devices: "Make this the default text size on all my devices" allow_private_messages: "Allow other users to send me personal messages" external_links_in_new_tab: "Open all external links in a new tab" diff --git a/config/site_settings.yml b/config/site_settings.yml index 46699dcd6ce..47d8d60383c 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -248,6 +248,7 @@ basic: default_dark_mode_color_scheme_id: default: -1 hidden: true + client: true relative_date_duration: client: true default: 30 diff --git a/db/migrate/20200805151752_add_dark_scheme_id_to_user_options.rb b/db/migrate/20200805151752_add_dark_scheme_id_to_user_options.rb new file mode 100644 index 00000000000..26230f5571a --- /dev/null +++ b/db/migrate/20200805151752_add_dark_scheme_id_to_user_options.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddDarkSchemeIdToUserOptions < ActiveRecord::Migration[6.0] + def change + add_column :user_options, :dark_scheme_id, :integer + end +end diff --git a/lib/stylesheet/manager.rb b/lib/stylesheet/manager.rb index 819c4bc33f5..426a6702e20 100644 --- a/lib/stylesheet/manager.rb +++ b/lib/stylesheet/manager.rb @@ -93,13 +93,17 @@ class Stylesheet::Manager end end - def self.color_scheme_stylesheet_details(color_scheme_id = nil) + def self.color_scheme_stylesheet_details(color_scheme_id = nil, media) color_scheme = begin ColorScheme.find(color_scheme_id) rescue + # don't load fallback when requesting dark color scheme + return false if media != "all" Theme.find_by_id(SiteSetting.default_theme_id)&.color_scheme || ColorScheme.base end + return false if !color_scheme + target = COLOR_SCHEME_STYLESHEET.to_sym current_hostname = Discourse.current_hostname color_scheme_name = Slug.for(color_scheme.name) @@ -119,7 +123,9 @@ class Stylesheet::Manager end def self.color_scheme_stylesheet_link_tag(color_scheme_id = nil, media = 'all') - stylesheet = color_scheme_stylesheet_details(color_scheme_id) + stylesheet = color_scheme_stylesheet_details(color_scheme_id, media) + return '' if !stylesheet + href = stylesheet[:new_href] %[].html_safe end diff --git a/spec/components/stylesheet/manager_spec.rb b/spec/components/stylesheet/manager_spec.rb index 692b7287a25..efcdaa3dbaa 100644 --- a/spec/components/stylesheet/manager_spec.rb +++ b/spec/components/stylesheet/manager_spec.rb @@ -177,9 +177,14 @@ describe Stylesheet::Manager do expect(link).not_to eq("") end - it "does not crash on missing color scheme" do + it "loads base scheme when defined scheme id is missing" do link = Stylesheet::Manager.color_scheme_stylesheet_link_tag(125) - expect(link).not_to eq("") + expect(link).to include("color_definitions_base") + end + + it "loads nothing when defined dark scheme id is missing" do + link = Stylesheet::Manager.color_scheme_stylesheet_link_tag(125, "(prefers-color-scheme: dark)") + expect(link).to eq("") end it "uses the correct color scheme from the default site theme" do diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index 7879aa9c569..96245121d55 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -349,6 +349,8 @@ describe ApplicationHelper do end describe 'discourse_color_scheme_stylesheets' do + fab!(:user) { Fabricate(:user) } + it 'returns a stylesheet link tag by default' do cs_stylesheets = helper.discourse_color_scheme_stylesheets expect(cs_stylesheets).to include("stylesheets/color_definitions") @@ -361,5 +363,40 @@ describe ApplicationHelper do expect(cs_stylesheets).to include("(prefers-color-scheme: dark)") expect(cs_stylesheets.scan("stylesheets/color_definitions").size).to eq(2) end + + it 'fails gracefully when the dark color scheme ID is set but missing' do + SiteSetting.default_dark_mode_color_scheme_id = -5 + cs_stylesheets = helper.discourse_color_scheme_stylesheets + + expect(cs_stylesheets).to include("stylesheets/color_definitions") + expect(cs_stylesheets).not_to include("(prefers-color-scheme: dark)") + end + + context "with a user option" do + before do + user.user_option.dark_scheme_id = -1 + user.user_option.save! + helper.request.env[Auth::DefaultCurrentUserProvider::CURRENT_USER_KEY] = user + + SiteSetting.default_dark_mode_color_scheme_id = ColorScheme.where(name: "Dark").pluck(:id).first + end + + it "returns no dark scheme stylesheet when user has disabled that option" do + color_stylesheets = helper.discourse_color_scheme_stylesheets + + expect(color_stylesheets).to include("stylesheets/color_definitions") + expect(color_stylesheets).not_to include("(prefers-color-scheme: dark)") + end + + it "returns user-selected dark color scheme stylesheet" do + new_cs = Fabricate(:color_scheme, name: 'Custom Color Scheme') + user.user_option.update!(dark_scheme_id: new_cs.id) + + color_stylesheets = helper.discourse_color_scheme_stylesheets + expect(color_stylesheets).to include("(prefers-color-scheme: dark)") + expect(color_stylesheets).to include("custom-color-scheme") + end + + end end end diff --git a/test/javascripts/acceptance/preferences-test.js b/test/javascripts/acceptance/preferences-test.js index 7f1e98af1cc..eb9aed022d4 100644 --- a/test/javascripts/acceptance/preferences-test.js +++ b/test/javascripts/acceptance/preferences-test.js @@ -482,3 +482,16 @@ QUnit.test("can select an option from a dropdown", async assert => { await field.selectRowByValue("Cat"); assert.equal(field.header().value(), "Cat", "it sets the value of the field"); }); + +acceptance("User Preferences disabling dark mode", { + loggedIn: true, + settings: { default_dark_mode_color_scheme_id: 1 } +}); + +QUnit.test("shows option to disable dark mode", async assert => { + await visit("/u/eviltrout/preferences/interface"); + assert.ok( + $(".control-group.dark-mode").length, + "it has the option to disable dark mode" + ); +});