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

Added favicon support for safari #19

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

Conversation

Mcrich23
Copy link
Contributor

@Mcrich23 Mcrich23 commented Apr 1, 2024

As with favicon support before (#8, #9), safari was not supported. This pr should fix that.

Because Apple has to be different
@twostraws
Copy link
Owner

This has been sitting waiting review for a while, which is my fault – I got into a bit of a rabbit hole on what exactly modern favicon support looks like, and in doing so even updated https://www.hackingwithswift.com to include the latest recommended approach. It's hard to get a great answer because so much just seems to be 10 years old, but from what I can see a good approach is:

  1. Using an SVG for the main favicon
  2. Using a 180x180 PNG for the Apple touch icon

I have no idea whether using 1 automatically makes 2 work, because if it doesn't it really ought to!

However, I'm wondering whether a better solution might work here: can we detect what favicon files they have in their Assets folder, and use that to generate all the correct tags automatically?

@Mcrich23
Copy link
Contributor Author

we can. I was looking at this as well and apple requires an extra metatag for safari. Thats what this pr is for. If you want me to, I can set it to auto look for favicon.png/svg/jpeg, and then people can override it if they choose.

@aduryagin
Copy link

aduryagin commented Jun 10, 2024

apple requires an extra metatag for safari

Worth to mention that apple-touch-icon has been deprecated. Apple requires this only for Safari versions prior to 15.4
Google Lighthouse also deprecated apple-touch-icon audit

GoogleChrome/lighthouse#14243
https://developer.apple.com/documentation/safari-release-notes/safari-15_4-release-notes#Web-API
https://developer.mozilla.org/en-US/docs/Web/Manifest/icons#browser_compatibility

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.

3 participants