FIX: Render the correct logo for the active scheme when logo is minimized (#57)

There's currently a bug where if you use the color scheme toggler to switch to the opposite color mode of your system preference, i.e. enable light mode when your system preference is dark and vice versa, the header logo renders the version matching your system preference instead of the selected choice made via the toggler the next time the logo is minimized or expanded (e.g. when scrolling in a long topic).

The bug occurs because when you switch your color scheme via the toggler component, the component updates all the relevant DOM elements that are currently in-view, but subsequent changes to the DOM made by the app, e.g. when the header logo is minimized/expanded, aren't aware of the new choice and resort to the system preference.

This commit fixes the bug by adding a hook to the header logo that gets called when the logo is minimized/expanded and making sure the user's scheme choice is applied to the logo when it's re-rendered.

Internal topic: t/144688.

Author: OsamaSayegh

javascripts/discourse/connectors/home-logo-contents__after/minimized-hook.gjs

@@ -0,0 +1,29 @@
+import Component from "@glimmer/component";
+import { action } from "@ember/object";
+import didUpdate from "@ember/render-modifiers/modifiers/did-update";
+import { service } from "@ember/service";
+import {
+  changeHomeLogo,
+  COLOR_SCHEME_OVERRIDE_KEY,
+} from "../../lib/color-scheme-override";
+
+export default class MinimizedHook extends Component {
+  @service keyValueStore;
+
+  @action
+  onMinimizedChange() {
+    const storedOverride = this.keyValueStore.getItem(
+      COLOR_SCHEME_OVERRIDE_KEY
+    );
+    if (storedOverride) {
+      changeHomeLogo(storedOverride);
+    }
+  }
+
+  <template>
+    <span
+      class="hidden color-toggler-minimized-hook"
+      {{didUpdate this.onMinimizedChange @outletArgs.minimized}}
+    />
+  </template>
+}

javascripts/discourse/lib/color-scheme-override.js

@@ -10,28 +10,18 @@ export function colorSchemeOverride(type) {
     return;
   }
 
-  const logoDarkSrc = document.querySelector(".title picture source");
-
   switch (type) {
     case "dark":
       lightScheme.origMedia = lightScheme.media;
       lightScheme.media = "none";
       darkScheme.origMedia = darkScheme.media;
       darkScheme.media = "all";
-      if (logoDarkSrc) {
-        logoDarkSrc.origMedia = logoDarkSrc.media;
-        logoDarkSrc.media = "all";
-      }
       break;
     case "light":
       lightScheme.origMedia = lightScheme.media;
       lightScheme.media = "all";
       darkScheme.origMedia = darkScheme.media;
       darkScheme.media = "none";
-      if (logoDarkSrc) {
-        logoDarkSrc.origMedia = logoDarkSrc.media;
-        logoDarkSrc.media = "none";
-      }
       break;
     default:
       if (lightScheme.origMedia) {
@@ -42,7 +32,29 @@ export function colorSchemeOverride(type) {
         darkScheme.media = darkScheme.origMedia;
         darkScheme.removeAttribute("origMedia");
       }
-      if (logoDarkSrc?.origMedia) {
+      break;
+  }
+  changeHomeLogo(type);
+}
+
+export function changeHomeLogo(type) {
+  const logoDarkSrc = document.querySelector(".title picture source");
+
+  if (!logoDarkSrc) {
+    return;
+  }
+
+  switch (type) {
+    case "dark":
+      logoDarkSrc.origMedia = logoDarkSrc.media;
+      logoDarkSrc.media = "all";
+      break;
+    case "light":
+      logoDarkSrc.origMedia = logoDarkSrc.media;
+      logoDarkSrc.media = "none";
+      break;
+    default:
+      if (logoDarkSrc.origMedia) {
         logoDarkSrc.media = logoDarkSrc.origMedia;
       }
       break;

locales/en.yml

@@ -1,2 +1,2 @@
 en:
-  toggle_button_title: Toggle color scheme
+  toggle_button_title: Toggle color palette

spec/system/color_scheme_toggle_spec.rb

@@ -0,0 +1,60 @@
+# frozen_string_literal: true
+
+RSpec.describe "", system: true do
+  fab!(:topic) { Fabricate(:post).topic }
+  fab!(:user)
+  fab!(:post) { Fabricate(:post, topic:, raw: <<~POST * 20) }
+    Very lengthy post with lots of height for testing logo change when scrolling
+
+    Here's another paragraph to make the post take very large vertical space\n
+  POST
+
+  fab!(:dark_mode_image) { Fabricate(:image_upload, color: "white", width: 400, height: 120) }
+  fab!(:light_mode_image) { Fabricate(:image_upload, color: "black", width: 400, height: 120) }
+
+  fab!(:small_dark_mode_image) { Fabricate(:image_upload, color: "white", width: 120, height: 120) }
+  fab!(:small_light_mode_image) do
+    Fabricate(:image_upload, color: "black", width: 120, height: 120)
+  end
+
+  let!(:theme_component) { upload_theme_component }
+
+  let(:topic_page) { PageObjects::Pages::Topic.new }
+
+  before do
+    SiteSetting.logo = light_mode_image
+    SiteSetting.logo_small = small_light_mode_image
+    SiteSetting.logo_dark = dark_mode_image
+    SiteSetting.logo_small_dark = small_dark_mode_image
+    sign_in(user)
+  end
+
+  it "applies the correct `media` attribute on the source element when the logo is minimized upon scrolling" do
+    topic_page.visit_topic(topic)
+
+    dark_logo_source =
+      find(
+        ".title picture source[media=\"(prefers-color-scheme: dark)\"][srcset*=\"#{dark_mode_image.url}\"]",
+        visible: false,
+      )
+    expect(dark_logo_source).to be_present
+
+    find(".color-scheme-toggler").click
+
+    dark_logo_source =
+      find(
+        ".title picture source[media=\"all\"][srcset*=\"#{dark_mode_image.url}\"]",
+        visible: false,
+      )
+    expect(dark_logo_source).to be_present
+
+    page.scroll_to(find(".topic-footer-main-buttons .create"))
+
+    dark_logo_source =
+      find(
+        ".title picture source[media=\"all\"][srcset*=\"#{small_dark_mode_image.url}\"]",
+        visible: false,
+      )
+    expect(dark_logo_source).to be_present
+  end
+end