From 6a4b979514c19bca33ef4759adb52d990dfdc56f Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Mon, 31 Mar 2025 18:23:07 +1000 Subject: [PATCH] FIX: Color palette switching when Horizon is not default (#95) When Horizon is not the default theme, and the color palette switcher is used, we were ending up in a broken CSS state because the color definitions were not loaded correctly when calling `/color-scheme-stylesheet/` to get the new stylesheet URLs. The problem was that we were not giving the theme ID to this URL, we were only doing `/color-scheme-stylesheet/:palette_id`, not `/color-scheme-stylesheet/:palette_id/:theme_id`, so the theme color definitions like the ones that define the background color were not loaded. This commit fixes the issue and also changes to recreating the `` tags for the CSS in the head so we can check if they are loaded correctly for easier testing in system specs. --- .../user-color-palette-selector.gjs | 61 ++++++++++++++----- .../components/user_color_palette_selector.rb | 4 ++ .../user_color_palette_selector_spec.rb | 37 ++++++++--- 3 files changed, 79 insertions(+), 23 deletions(-) diff --git a/javascripts/discourse/components/user-color-palette-selector.gjs b/javascripts/discourse/components/user-color-palette-selector.gjs index aec58d2..82e845f 100644 --- a/javascripts/discourse/components/user-color-palette-selector.gjs +++ b/javascripts/discourse/components/user-color-palette-selector.gjs @@ -3,6 +3,7 @@ import { tracked } from "@glimmer/tracking"; import { action } from "@ember/object"; import { service } from "@ember/service"; import { isEmpty } from "@ember/utils"; +import concatClass from "discourse/helpers/concat-class"; import icon from "discourse/helpers/d-icon"; import { reload } from "discourse/helpers/page-reloader"; import { ajax } from "discourse/lib/ajax"; @@ -31,6 +32,7 @@ export default class UserColorPaletteSelector extends Component { @service interfaceColor; @tracked anonColorPaletteId = this.#loadAnonColorPalette(); @tracked userColorPaletteId = this.session.userColorSchemeId; + @tracked cssLoaded = true; get userColorPalettes() { const availablePalettes = listColorSchemes(this.site) @@ -96,10 +98,7 @@ export default class UserColorPaletteSelector extends Component { } this.#updatePreference(selectedPalette); - this.#changeSiteColorPalette( - selectedPalette.id, - selectedPalette.correspondingDarkModeId - ); + this.#changeSiteColorPalette(selectedPalette); this.dMenu.close(); } @@ -123,28 +122,59 @@ export default class UserColorPaletteSelector extends Component { } } - async #changeSiteColorPalette(lightPaletteId, darkPaletteId) { + async #changeSiteColorPalette(colorPalette) { + this.cssLoaded = false; + + const lightPaletteId = colorPalette.id; + const darkPaletteId = colorPalette.correspondingDarkModeId; const lightTag = document.querySelector("link.light-scheme"); const darkTag = document.querySelector("link.dark-scheme"); + // TODO(osama) once we have built-in light/dark modes for each palette, we + // can stop making the 2nd ajax call for the dark palette and replace it + // with a include_dark_scheme param on the ajax call for the light + // palette which will include the href for the dark palette in the response if (!darkTag) { reload(); return; } - // TODO(osama) once we have built-in light/dark modes for each palette, we - // can stop making the 2nd ajax call for the dark palette and replace it - // with a `include_dark_scheme` param on the ajax call for the light - // palette which will include the href for the dark palette in the response const lightPaletteInfo = await ajax( - `/color-scheme-stylesheet/${lightPaletteId}.json` + `/color-scheme-stylesheet/${lightPaletteId}/${colorPalette.theme_id}.json` ); const darkPaletteInfo = await ajax( - `/color-scheme-stylesheet/${darkPaletteId}.json` + `/color-scheme-stylesheet/${darkPaletteId}/${colorPalette.theme_id}.json` ); - lightTag.href = lightPaletteInfo.new_href; - darkTag.href = darkPaletteInfo.new_href; + const replaceLinkTag = (oldTag, newHref, className, mode, paletteId) => { + const newTag = document.createElement("link"); + newTag.rel = "stylesheet"; + newTag.href = newHref; + newTag.className = className; + newTag.dataset.schemeId = paletteId; + newTag.media = `(prefers-color-scheme: ${mode})`; + + newTag.onload = () => { + this.cssLoaded = true; + }; + + oldTag.parentNode.replaceChild(newTag, oldTag); + }; + + replaceLinkTag( + lightTag, + lightPaletteInfo.new_href, + "light-scheme", + "light", + lightPaletteId + ); + replaceLinkTag( + darkTag, + darkPaletteInfo.new_href, + "dark-scheme", + "dark", + darkPaletteId + ); }