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

chore(README): fix up conditional logo rendering #2199

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

CompeyDev
Copy link

@CompeyDev CompeyDev commented Mar 5, 2024

A really simple 1 liner change, which has been annoying me a bit lately.

This PR makes use of GitHub's provided theme URL fragments, instead of separate conditional media sources which was previously used, and didn't seem to apply correctly for me sometimes (see below). I was able to reproduce this without any userCSS themes as well.

Overall, I just think it makes more sense to offload things like this to GitHub themselves, instead of handling it on our own.

erroneous logo formatting

Signed-off-by: Erica Marigold <hi@devcomp.xyz>
@TayouVR
Copy link
Member

TayouVR commented Mar 5, 2024

I don't think this is working right like this..
image

@lordofpipes
Copy link

lordofpipes commented Mar 5, 2024

The syntax already used in the README.md matches exactly how Github says the feature should be used:

https://docs.github.com/en/get-started/writing-on-github/getting-started-with-writing-and-formatting-on-github/basic-writing-and-formatting-syntax#specifying-the-theme-an-image-is-shown-to

This feature used to be a Github beta feature but it has since rolled out to all users, so if it's not working, it's a bug with Github. So this should probably just be reported to Github since most people can't replicate the issue.

This article seems to frame #gh- as the old, clunky way to do it.

@lordofpipes
Copy link

lordofpipes commented Mar 5, 2024

Oh, I was able to replicate the issue by setting Theme mode to Sync with system, and then flipping the day theme to dark and the night theme to light. The issue doesn't seem to manifest when just setting a non-synced theme.

@CompeyDev
Copy link
Author

Oh, I was able to replicate the issue by setting Theme mode to Sync with system, and then flipping the day theme to dark and the night theme to light. The issue doesn't seem to manifest when just setting a non-synced theme.

Hm, I have the dark dimmed theme set, which isn't synced with system and I seem to face it too. Odd.

@CompeyDev
Copy link
Author

I don't think this is working right like this.. image

This is exactly how it functions, this is intended functionality. When on dark mode, any light mode images, when fetched, are returned as empty images and vice versa.

@CompeyDev
Copy link
Author

After a reading of the article, I feel like the problems highlighted there should mainly apply to things to be published to external sites. Unless a GitHub scraper or something similar just extracts and displays the Prism launcher README, I don't think we should worry about that, tbh. Regardless, it should probably be a feature of such scrapers to maintain an environment similar to GitHub since they would possibly be rendering things configured for GitHub.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants