FIX: Use the category logo matching the theme (#24033)

This commit fixes a bug in which the dark category logo would be used in a light theme if the system preference was set to dark and the user forced the use of a light theme in Discourse

app/assets/javascripts/discourse/app/components/category-logo.gjs

@@ -0,0 +1,12 @@
+import LightDarkImg from "discourse/components/light-dark-img";
+
+const CategoryLogo = <template>
+  <div class="category-logo aspect-image">
+    <LightDarkImg
+      @lightImg={{@category.uploaded_logo}}
+      @darkImg={{@category.uploaded_logo_dark}}
+    />
+  </div>
+</template>;
+
+export default CategoryLogo;

app/assets/javascripts/discourse/app/components/category-logo.hbs

@@ -1,23 +0,0 @@
-<div class="category-logo aspect-image">
-  {{#if (and @category.uploaded_logo.url @category.uploaded_logo_dark.url)}}
-    <picture>
-      <source
-        srcset={{@category.uploaded_logo_dark.url}}
-        width={{@category.uploaded_logo_dark.width}}
-        height={{@category.uploaded_logo_dark.height}}
-        media="(prefers-color-scheme: dark)"
-      />
-      <CdnImg
-        @src={{this.defaultCategoryLogo.url}}
-        @width={{this.defaultCategoryLogo.width}}
-        @height={{this.defaultCategoryLogo.height}}
-      />
-    </picture>
-  {{else if @category.uploaded_logo.url}}
-    <CdnImg
-      @src={{@category.uploaded_logo.url}}
-      @width={{@category.uploaded_logo.width}}
-      @height={{@category.uploaded_logo.height}}
-    />
-  {{/if}}
-</div>

app/assets/javascripts/discourse/app/components/category-logo.js

@@ -1,19 +0,0 @@
-import Component from "@glimmer/component";
-import { inject as service } from "@ember/service";
-
-export default class CategoryLogo extends Component {
-  @service session;
-
-  get defaultCategoryLogo() {
-    // use dark logo by default in edge case
-    // when scheme is dark and dark logo is present
-    if (
-      this.session.defaultColorSchemeIsDark &&
-      this.args.category.uploaded_logo_dark
-    ) {
-      return this.args.category.uploaded_logo_dark;
-    }
-
-    return this.args.category.uploaded_logo;
-  }
-}

app/assets/javascripts/discourse/app/components/cdn-img.hbs

@@ -1,5 +1,6 @@
 {{#if this.src}}
   <img
+    ...attributes
     src={{this.cdnSrc}}
     width={{this.width}}
     height={{this.height}}

app/assets/javascripts/discourse/app/components/light-dark-img.gjs

@@ -0,0 +1,56 @@
+import Component from "@glimmer/component";
+import { inject as service } from "@ember/service";
+import CdnImg from "discourse/components/cdn-img";
+import { getURLWithCDN } from "discourse-common/lib/get-url";
+
+export default class LightDarkImg extends Component {
+  @service session;
+
+  get isDarkImageAvailable() {
+    return (
+      this.args.lightImg?.url && // the light image must be present
+      this.args.darkImg?.url &&
+      (this.session.defaultColorSchemeIsDark || this.session.darkModeAvailable)
+    );
+  }
+
+  get defaultImg() {
+    // use dark logo by default in edge case
+    // when scheme is dark and dark logo is present
+    if (this.session.defaultColorSchemeIsDark && this.args.darkImg) {
+      return this.args.darkImg;
+    }
+
+    return this.args.lightImg;
+  }
+
+  get darkImgCdnSrc() {
+    return getURLWithCDN(this.args.darkImg.url);
+  }
+
+  <template>
+    {{#if this.isDarkImageAvailable}}
+      <picture>
+        <source
+          srcset={{this.darkImgCdnSrc}}
+          width={{@darkImg.width}}
+          height={{@darkImg.height}}
+          media="(prefers-color-scheme: dark)"
+        />
+        <CdnImg
+          ...attributes
+          @src={{this.defaultImg.url}}
+          @width={{this.defaultImg.width}}
+          @height={{this.defaultImg.height}}
+        />
+      </picture>
+    {{else if @lightImg.url}}
+      <CdnImg
+        ...attributes
+        @src={{@lightImg.url}}
+        @width={{@lightImg.width}}
+        @height={{@lightImg.height}}
+      />
+    {{/if}}
+  </template>
+}

app/assets/javascripts/discourse/tests/integration/components/light-dark-img-test.gjs

@@ -0,0 +1,214 @@
+import { getOwner } from "@ember/application";
+import { render } from "@ember/test-helpers";
+import { module, test } from "qunit";
+import LigthDarkImg from "discourse/components/light-dark-img";
+import { setupRenderingTest } from "discourse/tests/helpers/component-test";
+import { count, exists, query } from "discourse/tests/helpers/qunit-helpers";
+
+const lightSrc = { url: "/images/light.jpg", width: 376, height: 500 };
+const darkSrc = { url: "/images/light.jpg", width: 432, height: 298 };
+
+module("Integration | Component | light-dark-img", function (hooks) {
+  setupRenderingTest(hooks);
+
+  hooks.afterEach(function () {
+    this.session = getOwner(this).lookup("service:session");
+    this.session.set("darkModeAvailable", null);
+    this.session.set("defaultColorSchemeIsDark", null);
+  });
+
+  test("light theme with no images provided | dark mode not available", async function (assert) {
+    this.session.set("defaultColorSchemeIsDark", false);
+    this.session.set("darkModeAvailable", false);
+
+    await render(<template><LigthDarkImg /></template>);
+
+    assert.ok(!exists("picture"), "there is no picture tag");
+    assert.ok(!exists("img"), "there is no img tag");
+    assert.ok(!exists("source"), "there are no source tags");
+  });
+
+  test("light theme with only light image provided | dark mode not available", async function (assert) {
+    this.session.set("defaultColorSchemeIsDark", false);
+    this.session.set("darkModeAvailable", false);
+
+    await render(<template><LigthDarkImg @lightImg={{lightSrc}} /></template>);
+
+    assert.ok(!exists("picture"), "there is no picture tag");
+    assert.strictEqual(count("img"), 1, "there is an img tag");
+    assert.strictEqual(
+      query("img").getAttribute("src"),
+      lightSrc.url,
+      "the img src is the light image"
+    );
+    assert.ok(!exists("source"), "there are no source tags");
+  });
+
+  test("light theme with light and dark images provided | dark mode not available", async function (assert) {
+    this.session.set("defaultColorSchemeIsDark", false);
+    this.session.set("darkModeAvailable", false);
+
+    await render(<template>
+      <LigthDarkImg @lightImg={{lightSrc}} @darkImg={{darkSrc}} />
+    </template>);
+
+    assert.ok(!exists("picture"), "there is no picture tag");
+    assert.strictEqual(count("img"), 1, "there is an img tag");
+    assert.strictEqual(
+      query("img").getAttribute("src"),
+      lightSrc.url,
+      "the img src is the light image"
+    );
+    assert.ok(!exists("source"), "there are no source tags");
+  });
+
+  test("light theme with no images provided | dark mode available", async function (assert) {
+    this.session.set("defaultColorSchemeIsDark", false);
+    this.session.set("darkModeAvailable", true);
+
+    await render(<template><LigthDarkImg /></template>);
+
+    assert.ok(!exists("picture"), "there is no picture tag");
+    assert.ok(!exists("img"), "there is no img tag");
+    assert.ok(!exists("source"), "there are no source tags");
+  });
+
+  test("light theme with only light image provided | dark mode available", async function (assert) {
+    this.session.set("defaultColorSchemeIsDark", false);
+    this.session.set("darkModeAvailable", true);
+
+    await render(<template><LigthDarkImg @lightImg={{lightSrc}} /></template>);
+
+    assert.ok(!exists("picture"), "there is no picture tag");
+    assert.strictEqual(count("img"), 1, "there is an img tag");
+    assert.strictEqual(
+      query("img").getAttribute("src"),
+      lightSrc.url,
+      "the img src is the light image"
+    );
+    assert.ok(!exists("source"), "there are no source tags");
+  });
+
+  test("light theme with light and dark images provided | dark mode available", async function (assert) {
+    this.session.set("defaultColorSchemeIsDark", false);
+    this.session.set("darkModeAvailable", true);
+
+    await render(<template>
+      <LigthDarkImg @lightImg={{lightSrc}} @darkImg={{darkSrc}} />
+    </template>);
+
+    assert.strictEqual(count("picture"), 1, "there is a picture tag");
+    assert.strictEqual(count("img"), 1, "there is an img tag");
+    assert.strictEqual(
+      query("img").getAttribute("src"),
+      lightSrc.url,
+      "the img src is the light image"
+    );
+    assert.strictEqual(count("source"), 1, "there is a source tag");
+    assert.strictEqual(
+      query("source").getAttribute("srcset"),
+      darkSrc.url,
+      "the source srcset is the dark image"
+    );
+  });
+
+  test("dark theme with no images provided | dark mode not available", async function (assert) {
+    this.session.set("defaultColorSchemeIsDark", true);
+    this.session.set("darkModeAvailable", false);
+
+    await render(<template><LigthDarkImg /></template>);
+
+    assert.ok(!exists("picture"), "there is no picture tag");
+    assert.ok(!exists("img"), "there is no img tag");
+    assert.ok(!exists("source"), "there are no source tags");
+  });
+
+  test("dark theme with only light image provided | dark mode not available", async function (assert) {
+    this.session.set("defaultColorSchemeIsDark", true);
+    this.session.set("darkModeAvailable", false);
+
+    await render(<template><LigthDarkImg @lightImg={{lightSrc}} /></template>);
+
+    assert.ok(!exists("picture"), "there is no picture tag");
+    assert.strictEqual(count("img"), 1, "there is an img tag");
+    assert.strictEqual(
+      query("img").getAttribute("src"),
+      lightSrc.url,
+      "the img src is the light image"
+    );
+    assert.ok(!exists("source"), "there are no source tags");
+  });
+
+  test("dark theme with light and dark images provided | dark mode not available", async function (assert) {
+    this.session.set("defaultColorSchemeIsDark", true);
+    this.session.set("darkModeAvailable", false);
+
+    await render(<template>
+      <LigthDarkImg @lightImg={{lightSrc}} @darkImg={{darkSrc}} />
+    </template>);
+
+    assert.strictEqual(count("picture"), 1, "there is a picture tag");
+    assert.strictEqual(count("img"), 1, "there is an img tag");
+    assert.strictEqual(
+      query("img").getAttribute("src"),
+      darkSrc.url,
+      "the img src is the dark image"
+    );
+    assert.strictEqual(count("source"), 1, "there is a source tag");
+    assert.strictEqual(
+      query("source").getAttribute("srcset"),
+      darkSrc.url,
+      "the source srcset is the dark image"
+    );
+  });
+
+  test("dark theme with no images provided | dark mode available", async function (assert) {
+    this.session.set("defaultColorSchemeIsDark", true);
+    this.session.set("darkModeAvailable", true);
+
+    await render(<template><LigthDarkImg /></template>);
+
+    assert.ok(!exists("picture"), "there is no picture tag");
+    assert.ok(!exists("img"), "there is no img tag");
+    assert.ok(!exists("source"), "there are no source tags");
+  });
+
+  test("dark theme with only light image provided | dark mode available", async function (assert) {
+    this.session.set("defaultColorSchemeIsDark", true);
+    this.session.set("darkModeAvailable", true);
+
+    await render(<template><LigthDarkImg @lightImg={{lightSrc}} /></template>);
+
+    assert.ok(!exists("picture"), "there is no picture tag");
+    assert.strictEqual(count("img"), 1, "there is an img tag");
+    assert.strictEqual(
+      query("img").getAttribute("src"),
+      lightSrc.url,
+      "the img src is the light image"
+    );
+    assert.ok(!exists("source"), "there are no source tags");
+  });
+
+  test("dark theme with light and dark images provided | dark mode available", async function (assert) {
+    this.session.set("defaultColorSchemeIsDark", true);
+    this.session.set("darkModeAvailable", true);
+
+    await render(<template>
+      <LigthDarkImg @lightImg={{lightSrc}} @darkImg={{darkSrc}} />
+    </template>);
+
+    assert.strictEqual(count("picture"), 1, "there is a picture tag");
+    assert.strictEqual(count("img"), 1, "there is an img tag");
+    assert.strictEqual(
+      query("img").getAttribute("src"),
+      darkSrc.url,
+      "the img src is the dark image"
+    );
+    assert.strictEqual(count("source"), 1, "there is a source tag");
+    assert.strictEqual(
+      query("source").getAttribute("srcset"),
+      darkSrc.url,
+      "the source srcset is the dark image"
+    );
+  });
+});