Skip to content

Commit 252a831

Browse files
lis2lis2
committed
FEATURE: theme default option in user interface
Before the "theme default" option appeared conditionally, which caused confusion. For example, it was shown when the theme was using a color scheme that was not user-selectable. If the scheme was selectable, then a specific scheme like "Merigold" was preselected. If the theme changed default schemes, that change was not reflected on the user interface. Therefore, it would be better to always have the "Theme default" option, which would have `-1` id. It means that the user's color scheme will always follow theme defaults.
1 parent 025f5ba commit 252a831

File tree

9 files changed

+136
-78
lines changed

9 files changed

+136
-78
lines changed

‎app/assets/javascripts/discourse/app/controllers/preferences/interface.js

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { tracked } from "@glimmer/tracking";
22
import Controller, { inject as controller } from "@ember/controller";
33
import { action, computed } from "@ember/object";
4-
import { not, reads } from "@ember/object/computed";
4+
import { not } from "@ember/object/computed";
55
import { service } from "@ember/service";
66
import { reload } from "discourse/helpers/page-reloader";
77
import { popupAjaxError } from "discourse/lib/ajax-error";
@@ -52,12 +52,8 @@ export default class InterfaceController extends Controller {
5252
@propertyEqual("model.id", "currentUser.id") isViewingOwnProfile;
5353
subpageTitle = i18n("user.preferences_nav.interface");
5454

55-
@reads("userSelectableColorSchemes.length") showColorSchemeSelector;
56-
5755
@not("currentSchemeCanBeSelected") showColorSchemeNoneItem;
5856

59-
selectedColorSchemeNoneLabel = i18n("user.color_schemes.default_description");
60-
6157
init() {
6258
super.init(...arguments);
6359
this.set("selectedDarkColorSchemeId", this.session.userDarkSchemeId);
@@ -250,13 +246,14 @@ export default class InterfaceController extends Controller {
250246
});
251247
}
252248

249+
@discourseComputed("userSelectableColorSchemes")
250+
showColorSchemeSelector(lightSchemes) {
251+
return lightSchemes && lightSchemes.length > 1;
252+
}
253+
253254
@discourseComputed("userSelectableDarkColorSchemes")
254255
showDarkColorSchemeSelector(darkSchemes) {
255-
// when a default dark scheme is set
256-
// dropdown has two items (disable / use site default)
257-
// but we show a checkbox in that case
258-
const minToShow = this.defaultDarkSchemeId > 0 ? 2 : 1;
259-
return darkSchemes && darkSchemes.length > minToShow;
256+
return darkSchemes && darkSchemes.length > 1;
260257
}
261258

262259
get interfaceColorModes() {

‎app/assets/javascripts/discourse/app/initializers/discourse-bootstrap.js

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,8 @@ export default {
7777

7878
session.highlightJsPath = setupData.highlightJsPath;
7979
session.svgSpritePath = setupData.svgSpritePath;
80-
session.userColorSchemeId =
81-
parseInt(setupData.userColorSchemeId, 10) || null;
82-
session.userDarkSchemeId = parseInt(setupData.userDarkSchemeId, 10) || -1;
80+
session.userColorSchemeId = parseInt(setupData.userColorSchemeId, 10);
81+
session.userDarkSchemeId = parseInt(setupData.userDarkSchemeId, 10);
8382

8483
let iconList = setupData.svgIconList;
8584
if (isDevelopment() && iconList) {

‎app/assets/javascripts/discourse/app/lib/color-scheme-picker.js

Lines changed: 9 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -29,36 +29,15 @@ export function listColorSchemes(site, options = {}) {
2929
});
3030

3131
const defaultLightColorScheme = site.get("default_light_color_scheme");
32-
if (defaultLightColorScheme && !options.darkOnly) {
33-
const existing = schemes.findBy("id", defaultLightColorScheme.id);
34-
if (!existing) {
35-
results.unshift({
36-
id: defaultLightColorScheme.id,
37-
name: `${i18n("user.color_schemes.default_description")}`,
38-
theme_id: defaultLightColorScheme.theme_id,
39-
colors: defaultLightColorScheme.colors,
40-
});
41-
}
42-
}
43-
if (options.darkOnly) {
44-
const defaultDarkColorScheme = site.get("default_dark_color_scheme");
45-
if (defaultDarkColorScheme) {
46-
const existing = schemes.findBy("id", defaultDarkColorScheme.id);
47-
if (!existing) {
48-
results.unshift({
49-
id: defaultDarkColorScheme.id,
50-
name: `${i18n("user.color_schemes.default_description")}`,
51-
theme_id: defaultDarkColorScheme.theme_id,
52-
colors: defaultDarkColorScheme.colors,
53-
});
54-
}
55-
} else {
56-
results.unshift({
57-
id: -1,
58-
name: `${i18n("user.color_schemes.default_description")}`,
59-
});
60-
}
61-
}
32+
const defaultDarkColorScheme = site.get("default_dark_color_scheme");
33+
34+
results.unshift({
35+
id: -1,
36+
name: `${i18n("user.color_schemes.default_description")}`,
37+
colors: options.darkOnly
38+
? defaultDarkColorScheme?.colors
39+
: defaultLightColorScheme?.colors,
40+
});
6241

6342
return results.length === 0 ? null : results;
6443
}

‎app/assets/javascripts/discourse/app/templates/preferences/interface.gjs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@ export default RouteTemplate(
6363
@value={{@controller.selectedColorSchemeId}}
6464
@onChange={{@controller.loadColorScheme}}
6565
@options={{hash
66-
translatedNone=@controller.selectedColorSchemeNoneLabel
6766
autoInsertNoneItem=@controller.showColorSchemeNoneItem
6867
}}
6968
/>

‎app/assets/javascripts/discourse/tests/acceptance/user-preferences-interface-test.js

Lines changed: 6 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -65,27 +65,6 @@ acceptance("User Preferences - Interface", function (needs) {
6565
removeCookie("text_size");
6666
});
6767

68-
test("shows light color scheme default option when theme's color scheme is not user selectable", async function (assert) {
69-
let site = Site.current();
70-
site.set("user_themes", [
71-
{ theme_id: 1, name: "Cool Theme", color_scheme_id: null },
72-
]);
73-
74-
site.set("user_color_schemes", [{ id: 2, name: "Cool Breeze" }]);
75-
76-
await visit("/u/eviltrout/preferences/interface");
77-
assert.dom(".light-color-scheme").exists("has regular dropdown");
78-
79-
assert.strictEqual(
80-
selectKit(".light-color-scheme .select-kit").header().value(),
81-
null
82-
);
83-
assert.strictEqual(
84-
selectKit(".light-color-scheme .select-kit").header().label(),
85-
i18n("user.color_schemes.default_description")
86-
);
87-
});
88-
8968
skip("shows no default option for light scheme when theme's color scheme is user selectable", async function (assert) {
9069
let meta = document.createElement("meta");
9170
meta.name = "discourse_theme_id";
@@ -202,12 +181,15 @@ acceptance(
202181
);
203182
});
204183

205-
test("display 'Theme default' when default color scheme is not marked as selectable", async function (assert) {
184+
test("always display 'Theme default'", async function (assert) {
206185
let meta = document.createElement("meta");
207186
meta.name = "discourse_theme_id";
208187
meta.content = "1";
209188
document.getElementsByTagName("head")[0].appendChild(meta);
210189

190+
let session = Session.current();
191+
session.userColorSchemeId = -1;
192+
211193
let site = Site.current();
212194
site.set("user_themes", [
213195
{ theme_id: 1, name: "A Theme", color_scheme_id: 2, default: true },
@@ -219,14 +201,14 @@ acceptance(
219201

220202
assert.dom(".light-color-scheme").exists("has regular dropdown");
221203
const dropdownObject = selectKit(".light-color-scheme .select-kit");
222-
assert.strictEqual(dropdownObject.header().value(), null);
204+
assert.strictEqual(dropdownObject.header().value(), "-1");
223205
assert.strictEqual(
224206
dropdownObject.header().label(),
225207
i18n("user.color_schemes.default_description")
226208
);
227209

228210
await dropdownObject.expand();
229-
assert.strictEqual(dropdownObject.rows().length, 1);
211+
assert.strictEqual(dropdownObject.rows().length, 2);
230212

231213
document.querySelector("meta[name='discourse_theme_id']").remove();
232214
});

‎app/helpers/application_helper.rb

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -566,14 +566,16 @@ def stylesheet_manager
566566
@stylesheet_manager = Stylesheet::Manager.new(theme_id: theme_id)
567567
end
568568

569+
def user_scheme_id
570+
return @user_scheme_id if defined?(@user_scheme_id)
571+
scheme_id = cookies[:color_scheme_id] || current_user&.user_option&.color_scheme_id
572+
@user_scheme_id = scheme_id if scheme_id && ColorScheme.find_by_id(scheme_id)
573+
end
574+
569575
def scheme_id
570576
return @scheme_id if defined?(@scheme_id)
571577

572-
custom_user_scheme_id = cookies[:color_scheme_id] || current_user&.user_option&.color_scheme_id
573-
if custom_user_scheme_id && ColorScheme.find_by_id(custom_user_scheme_id)
574-
return custom_user_scheme_id
575-
end
576-
578+
return user_scheme_id if user_scheme_id
577579
return if theme_id.blank?
578580

579581
if SiteSetting.use_overhauled_theme_color_palette
@@ -582,11 +584,17 @@ def scheme_id
582584
@scheme_id ||= Theme.where(id: theme_id).pick(:color_scheme_id)
583585
end
584586

587+
def user_dark_scheme_id
588+
return @user_dark_scheme_id if defined?(@user_dark_scheme_id)
589+
scheme_id = cookies[:dark_scheme_id] || current_user&.user_option&.dark_scheme_id
590+
@user_dark_scheme_id = scheme_id if scheme_id && ColorScheme.find_by_id(scheme_id)
591+
end
592+
585593
def dark_scheme_id
586594
if SiteSetting.use_overhauled_theme_color_palette
587595
scheme_id
588596
else
589-
cookies[:dark_scheme_id] || current_user&.user_option&.dark_scheme_id ||
597+
user_dark_scheme_id ||
590598
(theme_id ? Theme.find_by_id(theme_id) : Theme.find_default)&.dark_color_scheme_id || -1
591599
end
592600
end
@@ -832,8 +840,8 @@ def client_side_setup_data
832840
svg_sprite_path: SvgSprite.path(theme_id),
833841
enable_js_error_reporting: GlobalSetting.enable_js_error_reporting,
834842
color_scheme_is_dark: dark_color_scheme?,
835-
user_color_scheme_id: scheme_id,
836-
user_dark_scheme_id: dark_scheme_id,
843+
user_color_scheme_id: user_scheme_id || -1,
844+
user_dark_scheme_id: user_dark_scheme_id || -1,
837845
}
838846

839847
if Rails.env.development?

‎spec/helpers/application_helper_spec.rb

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -868,7 +868,7 @@ def script_tag(url, entrypoint, nonce)
868868

869869
context "with dark scheme with user option and/or cookies" do
870870
before do
871-
user.user_option.dark_scheme_id = -1
871+
user.user_option.interface_color_mode = UserOption::LIGHT_MODE
872872
user.user_option.save!
873873
helper.request.env[Auth::DefaultCurrentUserProvider::CURRENT_USER_KEY] = user
874874
@new_cs = Fabricate(:color_scheme, name: "Custom Color Scheme")
@@ -884,14 +884,18 @@ def script_tag(url, entrypoint, nonce)
884884
end
885885

886886
it "returns user-selected dark color scheme stylesheet" do
887-
user.user_option.update!(dark_scheme_id: @new_cs.id)
887+
user.user_option.update!(
888+
dark_scheme_id: @new_cs.id,
889+
interface_color_mode: UserOption::AUTO_MODE,
890+
)
888891

889892
color_stylesheets = helper.discourse_color_scheme_stylesheets
890893
expect(color_stylesheets).to include("(prefers-color-scheme: dark)")
891894
expect(color_stylesheets).to include("custom-color-scheme")
892895
end
893896

894897
it "respects cookie value over user option for dark color scheme" do
898+
user.user_option.update!(interface_color_mode: UserOption::AUTO_MODE)
895899
helper.request.cookies["dark_scheme_id"] = @new_cs.id
896900

897901
color_stylesheets = helper.discourse_color_scheme_stylesheets

‎spec/system/page_objects/pages/user_preferences_interface.rb

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,28 @@ def dark_mode_checkbox
2424
page.find('.dark-mode input[type="checkbox"]')
2525
end
2626

27+
def light_scheme_dropdown
28+
PageObjects::Components::SelectKit.new(".light-color-scheme .select-kit")
29+
end
30+
31+
def dark_scheme_dropdown
32+
PageObjects::Components::SelectKit.new(".dark-color-scheme .select-kit")
33+
end
34+
35+
def has_light_scheme_css?(color_scheme)
36+
expect(page).to have_css(
37+
"link.light-scheme[data-scheme-id=\"#{color_scheme.id}\"]",
38+
visible: false,
39+
)
40+
end
41+
42+
def has_dark_scheme_css?(color_scheme)
43+
expect(page).to have_css(
44+
"link.dark-scheme[data-scheme-id=\"#{color_scheme.id}\"]",
45+
visible: false,
46+
)
47+
end
48+
2749
def color_mode_dropdown
2850
PageObjects::Components::SelectKit.new(".interface-color-mode .select-kit")
2951
end

‎spec/system/user_page/user_preferences_interface_spec.rb

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,74 @@
9696
before { SiteSetting.interface_color_selector = "sidebar_footer" }
9797

9898
context "when changing own preferences" do
99+
fab!(:color_scheme_light_1) { Fabricate(:color_scheme, base_scheme_id: "Light") }
100+
fab!(:color_scheme_dark_1) { Fabricate(:color_scheme, base_scheme_id: "Dark") }
101+
fab!(:color_scheme_light_2) { Fabricate(:color_scheme, base_scheme_id: "Light") }
102+
fab!(:color_scheme_dark_2) { Fabricate(:color_scheme, base_scheme_id: "Dark") }
103+
fab!(:color_scheme_light_3) do
104+
Fabricate(:color_scheme, base_scheme_id: "Light", user_selectable: true)
105+
end
106+
fab!(:color_scheme_dark_3) do
107+
Fabricate(:color_scheme, base_scheme_id: "Dark", user_selectable: true)
108+
end
109+
110+
it "has and can change default color scheme for light and dark" do
111+
Theme.find_default.update!(
112+
color_scheme: color_scheme_light_1,
113+
dark_color_scheme: color_scheme_dark_1,
114+
)
115+
user_preferences_interface_page.visit(user)
116+
117+
expect(user_preferences_interface_page.light_scheme_dropdown).to have_selected_name(
118+
I18n.t("js.user.color_schemes.default_description"),
119+
)
120+
expect(user_preferences_interface_page.light_scheme_dropdown).to have_selected_value(-1)
121+
expect(user_preferences_interface_page.dark_scheme_dropdown).to have_selected_name(
122+
I18n.t("js.user.color_schemes.default_description"),
123+
)
124+
expect(user_preferences_interface_page.dark_scheme_dropdown).to have_selected_value(-1)
125+
expect(user_preferences_interface_page).to have_light_scheme_css(color_scheme_light_1)
126+
expect(user_preferences_interface_page).to have_dark_scheme_css(color_scheme_dark_1)
127+
128+
Theme.find_default.update!(
129+
color_scheme: color_scheme_light_2,
130+
dark_color_scheme: color_scheme_dark_2,
131+
)
132+
user_preferences_interface_page.visit(user)
133+
134+
expect(user_preferences_interface_page.light_scheme_dropdown).to have_selected_name(
135+
I18n.t("js.user.color_schemes.default_description"),
136+
)
137+
expect(user_preferences_interface_page.light_scheme_dropdown).to have_selected_value(-1)
138+
expect(user_preferences_interface_page.dark_scheme_dropdown).to have_selected_name(
139+
I18n.t("js.user.color_schemes.default_description"),
140+
)
141+
expect(user_preferences_interface_page.dark_scheme_dropdown).to have_selected_value(-1)
142+
expect(user_preferences_interface_page).to have_light_scheme_css(color_scheme_light_2)
143+
expect(user_preferences_interface_page).to have_dark_scheme_css(color_scheme_dark_2)
144+
145+
user_preferences_interface_page.light_scheme_dropdown.expand
146+
user_preferences_interface_page.light_scheme_dropdown.select_row_by_value(
147+
color_scheme_light_3.id,
148+
)
149+
user_preferences_interface_page.dark_scheme_dropdown.expand
150+
user_preferences_interface_page.dark_scheme_dropdown.select_row_by_value(
151+
color_scheme_dark_3.id,
152+
)
153+
user_preferences_interface_page.save_changes
154+
155+
user_preferences_interface_page.visit(user)
156+
157+
expect(user_preferences_interface_page.light_scheme_dropdown).to have_selected_name(
158+
color_scheme_light_3.name,
159+
)
160+
expect(user_preferences_interface_page.dark_scheme_dropdown).to have_selected_name(
161+
color_scheme_dark_3.name,
162+
)
163+
expect(user_preferences_interface_page).to have_light_scheme_css(color_scheme_light_3)
164+
expect(user_preferences_interface_page).to have_dark_scheme_css(color_scheme_dark_3)
165+
end
166+
99167
it "can change the color mode for the current device only" do
100168
user_preferences_interface_page.visit(user)
101169

0 commit comments

Comments
 (0)