Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX: Use the category logo matching the theme #24033

Merged
merged 2 commits into from Oct 20, 2023

Conversation

megothss
Copy link
Contributor

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

This commit fixes a bug in which the dark category logo would be used in a light theme if the system preferences were set to dark and the user forced the use of a light theme in Discourse
import CdnImg from "discourse/components/cdn-img";
import { getURLWithCDN } from "discourse-common/lib/get-url";

export default class LightDarkImg extends Component {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created this component because this pair of light/dark images can be used in the future in other places.

I'm just not sure about the name, I couldn't think of anything better.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't think of a better name, either.

<template>
{{#if this.isDarkImageAvailable}}
<picture>
<source
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reasons we can't use the <CdnImg component here instead of relying on darkImgCndSrc? 🤔

Copy link
Contributor Author

@megothss megothss Oct 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because we're using it in a <source> tag. <CdnImg> is an <img>

Copy link
Contributor

@pmusaraj pmusaraj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice!

import CdnImg from "discourse/components/cdn-img";
import { getURLWithCDN } from "discourse-common/lib/get-url";

export default class LightDarkImg extends Component {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't think of a better name, either.

@megothss megothss merged commit 53c23cf into main Oct 20, 2023
13 checks passed
@megothss megothss deleted the fix/dark-category-logo-choice branch October 20, 2023 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants