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

Updates favicons to support dark mode #1185

Merged
merged 1 commit into from
Jul 12, 2020

Conversation

sl4m
Copy link
Contributor

@sl4m sl4m commented Jul 11, 2020

This fixes #1157 using only prefers-color-scheme CSS media query. This was tested on the latest Firefox, Chrome, and Microsoft Edge.

I modified the existing favicon.svg to include CSS that will detect the system color scheme and appropriately apply the CSS. It seems the order in which the icons are referenced in the template matters, so I've re-ordered them in a way that worked during testing. For browsers that do not support SVG, it falls back on the modified png favicons which now have a white background and works on either system color scheme. Also, the reference to favicon.ico has been removed in the template as it will take precedence over all other available favicons.

I'm happy to make these changes for all the other repos that use the Rust logo favicon. I believe it is:

  • blog
  • thanks
  • ???

Chrome / Microsoft Edge:
Screenshot 2020-07-10 at 18 34 11
Screenshot 2020-07-10 at 18 28 35

Firefox:
Screenshot 2020-07-10 at 18 28 02
Screenshot 2020-07-10 at 18 29 36

PNG fallback:
Screenshot 2020-07-10 at 17 46 58
Screenshot 2020-07-10 at 17 47 22

@sl4m sl4m requested a review from a team as a code owner July 11, 2020 01:44
Copy link
Member

@Manishearth Manishearth left a comment

Choose a reason for hiding this comment

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

Seems okay.

@XAMPPRocky
Copy link
Member

Thank you for your PR! The PNG fallback had me thinking, do you think it would be possible to have the dark mode version have just a white border and white fill in through CSS? I was thinking something along these lines though I would probably give the favicon a thicker outline than in the picture.

rust-dark

@sl4m
Copy link
Contributor Author

sl4m commented Jul 12, 2020

@XAMPPRocky I spent some time trying to figure out how to manipulate the SVG to support what you've requested. I believe the SVG needs additional elements to support a border and fill. Unfortunately, my SVG knowledge stops here. If someone else can create the SVG with the additional elements, I can apply the CSS appropriately.

@XAMPPRocky
Copy link
Member

@sl4m I suspected that would be case. Either way I think this fine to merge Thank you for your PR and congrats on your first contribution!

@XAMPPRocky XAMPPRocky merged commit d5f6e95 into rust-lang:master Jul 12, 2020
@sl4m sl4m deleted the update-favicon branch July 12, 2020 05:40
@sl4m
Copy link
Contributor Author

sl4m commented Jul 13, 2020

@XAMPPRocky What are your thoughts about propagating these changes to other repos that are associated with rust-lang.org's subdomains? This is not a complete list, but I'm thinking:

  • blog.rust-lang.org
  • thanks

@XAMPPRocky
Copy link
Member

I think it would be fine to make a PR for each of those.

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.

Adaptive favicon (light vs. dark mode)
3 participants