Skip to content

Commit 8aa45fb

Browse files
lis2lis2
authored
Revert "FEATURE: theme default option in user interface" (#34125)
Reverts #34110
1 parent 4a604f0 commit 8aa45fb

File tree

11 files changed

+87
-147
lines changed

11 files changed

+87
-147
lines changed

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

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +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";
45
import { service } from "@ember/service";
56
import { reload } from "discourse/helpers/page-reloader";
67
import { popupAjaxError } from "discourse/lib/ajax-error";
@@ -51,6 +52,12 @@ export default class InterfaceController extends Controller {
5152
@propertyEqual("model.id", "currentUser.id") isViewingOwnProfile;
5253
subpageTitle = i18n("user.preferences_nav.interface");
5354

55+
@reads("userSelectableColorSchemes.length") showColorSchemeSelector;
56+
57+
@not("currentSchemeCanBeSelected") showColorSchemeNoneItem;
58+
59+
selectedColorSchemeNoneLabel = i18n("user.color_schemes.default_description");
60+
5461
init() {
5562
super.init(...arguments);
5663
this.set("selectedDarkColorSchemeId", this.session.userDarkSchemeId);
@@ -243,14 +250,13 @@ export default class InterfaceController extends Controller {
243250
});
244251
}
245252

246-
@discourseComputed("userSelectableColorSchemes")
247-
showColorSchemeSelector(lightSchemes) {
248-
return lightSchemes && lightSchemes.length > 1;
249-
}
250-
251253
@discourseComputed("userSelectableDarkColorSchemes")
252254
showDarkColorSchemeSelector(darkSchemes) {
253-
return darkSchemes && darkSchemes.length > 1;
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;
254260
}
255261

256262
get interfaceColorModes() {

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

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

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

8384
let iconList = setupData.svgIconList;
8485
if (isDevelopment() && iconList) {

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

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

3131
const defaultLightColorScheme = site.get("default_light_color_scheme");
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-
});
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+
}
4162

4263
return results.length === 0 ? null : results;
4364
}

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,10 @@ export default RouteTemplate(
6262
@content={{@controller.userSelectableColorSchemes}}
6363
@value={{@controller.selectedColorSchemeId}}
6464
@onChange={{@controller.loadColorScheme}}
65+
@options={{hash
66+
translatedNone=@controller.selectedColorSchemeNoneLabel
67+
autoInsertNoneItem=@controller.showColorSchemeNoneItem
68+
}}
6569
/>
6670
</div>
6771
</div>

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

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,27 @@ 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+
6889
skip("shows no default option for light scheme when theme's color scheme is user selectable", async function (assert) {
6990
let meta = document.createElement("meta");
7091
meta.name = "discourse_theme_id";
@@ -181,15 +202,12 @@ acceptance(
181202
);
182203
});
183204

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

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

202220
assert.dom(".light-color-scheme").exists("has regular dropdown");
203221
const dropdownObject = selectKit(".light-color-scheme .select-kit");
204-
assert.strictEqual(dropdownObject.header().value(), "-1");
222+
assert.strictEqual(dropdownObject.header().value(), null);
205223
assert.strictEqual(
206224
dropdownObject.header().label(),
207225
i18n("user.color_schemes.default_description")
208226
);
209227

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

213231
document.querySelector("meta[name='discourse_theme_id']").remove();
214232
});
@@ -284,8 +302,8 @@ acceptance(
284302
// dark scheme
285303
await selectKit(".dark-color-scheme .combobox").expand();
286304
assert.true(
287-
selectKit(".dark-color-scheme .combobox").rowByValue(3).exists(),
288-
"user selectable dark scheme is included"
305+
selectKit(".dark-color-scheme .combobox").rowByValue(1).exists(),
306+
"default dark scheme is included"
289307
);
290308

291309
await click("button.undo-preview");

‎app/helpers/application_helper.rb

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -566,16 +566,14 @@ 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-
575569
def scheme_id
576570
return @scheme_id if defined?(@scheme_id)
577571

578-
return user_scheme_id if user_scheme_id
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+
579577
return if theme_id.blank?
580578

581579
if SiteSetting.use_overhauled_theme_color_palette
@@ -584,17 +582,11 @@ def scheme_id
584582
@scheme_id ||= Theme.where(id: theme_id).pick(:color_scheme_id)
585583
end
586584

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-
593585
def dark_scheme_id
594586
if SiteSetting.use_overhauled_theme_color_palette
595587
scheme_id
596588
else
597-
user_dark_scheme_id ||
589+
cookies[:dark_scheme_id] || current_user&.user_option&.dark_scheme_id ||
598590
(theme_id ? Theme.find_by_id(theme_id) : Theme.find_default)&.dark_color_scheme_id || -1
599591
end
600592
end
@@ -840,8 +832,8 @@ def client_side_setup_data
840832
svg_sprite_path: SvgSprite.path(theme_id),
841833
enable_js_error_reporting: GlobalSetting.enable_js_error_reporting,
842834
color_scheme_is_dark: dark_color_scheme?,
843-
user_color_scheme_id: user_scheme_id || -1,
844-
user_dark_scheme_id: user_dark_scheme_id || -1,
835+
user_color_scheme_id: scheme_id,
836+
user_dark_scheme_id: dark_scheme_id,
845837
}
846838

847839
if Rails.env.development?

‎spec/helpers/application_helper_spec.rb

Lines changed: 2 additions & 6 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.interface_color_mode = UserOption::LIGHT_MODE
871+
user.user_option.dark_scheme_id = -1
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,18 +884,14 @@ def script_tag(url, entrypoint, nonce)
884884
end
885885

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

892889
color_stylesheets = helper.discourse_color_scheme_stylesheets
893890
expect(color_stylesheets).to include("(prefers-color-scheme: dark)")
894891
expect(color_stylesheets).to include("custom-color-scheme")
895892
end
896893

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

901897
color_stylesheets = helper.discourse_color_scheme_stylesheets

‎spec/system/interface_color_selector_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@
6262
end
6363

6464
it "is not available if the user uses the same scheme for dark mode as the light mode" do
65-
user.user_option.update!(color_scheme_id: light_scheme.id, dark_scheme_id: light_scheme.id)
65+
user.user_option.update!(color_scheme_id: light_scheme.id, dark_scheme_id: -1)
6666
sign_in(user)
6767

6868
visit("/")

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

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -24,28 +24,6 @@ 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-
4927
def color_mode_dropdown
5028
PageObjects::Components::SelectKit.new(".interface-color-mode .select-kit")
5129
end

‎spec/system/user_page/user_preferences_interface_spec.rb

Lines changed: 0 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,6 @@
6767
before do
6868
dark = ColorScheme.find_by(base_scheme_id: "Dark")
6969
ColorScheme.where.not(id: dark.id).destroy_all
70-
dark.update!(user_selectable: false)
7170
user.user_option.update!(dark_scheme_id: dark.id, theme_ids: [SiteSetting.default_theme_id])
7271
end
7372

@@ -97,74 +96,6 @@
9796
before { SiteSetting.interface_color_selector = "sidebar_footer" }
9897

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

0 commit comments

Comments
 (0)