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
`<link>` tags for the CSS in the head so we can check if they
are loaded correctly for easier testing in system specs.

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
+    );
   }
 
   <template>
@@ -153,7 +183,10 @@ export default class UserColorPaletteSelector extends Component {
         @identifier="user-color-palette-selector"
         @placementStrategy="fixed"
         @onRegisterApi={{this.onRegisterMenu}}
-        class="btn-flat user-color-palette-selector sidebar-footer-actions-button"
+        class={{concatClass
+          "btn-flat user-color-palette-selector sidebar-footer-actions-button"
+          (if this.cssLoaded "user-color-palette-css-loaded")
+        }}
         data-selected-color-palette-id={{this.selectedColorPaletteId}}
         @inline={{true}}
       >

spec/system/page_objects/components/user_color_palette_selector.rb

@@ -29,6 +29,10 @@ module PageObjects
         has_css?(".user-color-palette-selector[data-selected-color-palette-id='#{palette.id}']")
       end
 
+      def has_loaded_css?
+        has_css?(".user-color-palette-selector.user-color-palette-css-loaded")
+      end
+
       def has_tertiary_color?(palette)
         computed_color_hex =
           page.evaluate_script(

spec/system/user_color_palette_selector_spec.rb

@@ -3,8 +3,9 @@
 require_relative "./page_objects/components/user_color_palette_selector"
 
 describe "Horizon theme | User color palette selector", type: :system do
+  let(:set_theme_as_default) { true }
   let!(:theme) do
-    horizon_theme = upload_theme
+    horizon_theme = upload_theme(set_theme_as_default: set_theme_as_default)
     ColorScheme.where(theme_id: horizon_theme.id).update_all(user_selectable: true)
     horizon_theme
   end
@@ -33,10 +34,10 @@ describe "Horizon theme | User color palette selector", type: :system do
       visit "/"
       palette_selector.open_palette_menu
       palette_selector.click_palette_menu_item(marigold_palette.name)
-      expect(palette_selector).to have_no_palette_menu
-      page.refresh
 
+      expect(palette_selector).to have_no_palette_menu
       expect(palette_selector).to have_selected_palette(marigold_palette)
+      expect(palette_selector).to have_loaded_css
       expect(palette_selector).to have_tertiary_color(marigold_palette)
 
       page.refresh
@@ -47,10 +48,10 @@ describe "Horizon theme | User color palette selector", type: :system do
       visit "/"
       palette_selector.open_palette_menu
       palette_selector.click_palette_menu_item(marigold_palette.name)
-      expect(palette_selector).to have_no_palette_menu
-      page.refresh
 
+      expect(palette_selector).to have_no_palette_menu
       expect(palette_selector).to have_selected_palette(marigold_palette)
+      expect(palette_selector).to have_loaded_css
       expect(palette_selector).to have_computed_color("oklch(0.92 0.0708528 68.5036)")
 
       interface_color_selector.expand
@@ -62,6 +63,23 @@ describe "Horizon theme | User color palette selector", type: :system do
       expect(palette_selector).to have_selected_palette(marigold_palette)
       expect(palette_selector).to have_computed_color("oklch(0.481966 0.0354264 68.5036)")
     end
+
+    context "when the theme is not default but is selected by a user" do
+      let(:set_theme_as_default) { false }
+
+      it "can open the user color palette menu and select a palette, which is preseved on reload" do
+        theme.update!(user_selectable: true)
+        current_user.user_option.update!(theme_ids: [theme.id])
+        visit "/"
+        palette_selector.open_palette_menu
+        palette_selector.click_palette_menu_item(marigold_palette.name)
+
+        expect(palette_selector).to have_no_palette_menu
+        expect(palette_selector).to have_selected_palette(marigold_palette)
+        expect(palette_selector).to have_loaded_css
+        expect(palette_selector).to have_tertiary_color(marigold_palette)
+      end
+    end
   end
 
   describe "for anon" do
@@ -69,10 +87,10 @@ describe "Horizon theme | User color palette selector", type: :system do
       visit "/"
       palette_selector.open_palette_menu
       palette_selector.click_palette_menu_item(marigold_palette.name)
-      expect(palette_selector).to have_no_palette_menu
-      page.refresh
 
+      expect(palette_selector).to have_no_palette_menu
       expect(palette_selector).to have_selected_palette(marigold_palette)
+      expect(palette_selector).to have_loaded_css
       expect(palette_selector).to have_tertiary_color(marigold_palette)
     end
 
@@ -80,16 +98,17 @@ describe "Horizon theme | User color palette selector", type: :system do
       visit "/"
       palette_selector.open_palette_menu
       palette_selector.click_palette_menu_item(marigold_palette.name)
-      expect(palette_selector).to have_no_palette_menu
-      page.refresh
 
+      expect(palette_selector).to have_no_palette_menu
       expect(palette_selector).to have_selected_palette(marigold_palette)
+      expect(palette_selector).to have_loaded_css
       expect(palette_selector).to have_computed_color("oklch(0.92 0.0708528 68.5036)")
 
       interface_color_selector.expand
       interface_color_selector.dark_option.click
       expect(interface_color_mode).to have_dark_mode_forced
       expect(palette_selector).to have_selected_palette(marigold_palette)
+      expect(palette_selector).to have_loaded_css
       expect(palette_selector).to have_computed_color("oklch(0.481966 0.0354264 68.5036)")
 
       page.refresh