feat: add apple platform for single apple-touch-icon #309
Conversation
- set default to false - add platform to html - add config icon - add to all platform option - include apple platform in tests - update readme with explanation from webhint
|
/cc @dmnsgn sorry for delay, friendly ping |
| @@ -9,6 +9,12 @@ module.exports = { | |||
| ({ theme_color, background }) => `<meta name="theme-color" content="${theme_color || background}">`, | |||
| ({ appName }) => appName ? `<meta name="application-name" content="${appName}">` : `<meta name="application-name">` | |||
| ], | |||
| apple: [ | |||
There was a problem hiding this comment.
What about do not add new options, and use existing appleIcon? For example:
appleIcon: true- generate all iconsappleIcon: []- allow to specify sizesappleIcon: 'minimum'generate only minimum required icons and tags
There was a problem hiding this comment.
It looks like moving forward, what you have named minimum is actually be the norm and recommended on Apple platforms. So I'd actually go the other way around:
appleIcon: 'all'- generate all iconsappleIcon: []- allow to specify sizesappleIcon: true- generate only minimum required icons and tags
There was a problem hiding this comment.
I am afraid changing logic for appleIcon: true is will be breaking change, we can do it in the next major release, not right now
There was a problem hiding this comment.
Actually, isn't this PR #312 solving the problem by allowing to pass appleIcon: ["apple-touch-icon.png"]? I don't think the generated html is updated though (eg. all sizes are still in html, just images are generated) as I don't see any filter here: https://github.com/itgalaxy/favicons/blob/master/src/config/html.js#L12
There was a problem hiding this comment.
Feel free to send a fix for #312, appleIcon: true- generate all icons is not generate all icons, we can't change it by default
This PR add a new platform:
appleand takes care of generating a single icon image for apple devices (<link rel="apple-touch-icon" href="apple-touch-icon.png">. It follows the recommendation from webhint and lighthouse to have a single image instead of the dozen currently generated by theappleIconplatform:I went the separate platform way instead of a
onlyNewAppleSizesoption as suggested in @evilebottnawi comment (#130 (comment)) as it seems to be the new norm, it's quite specific to the apple platform and I see it as exclusive from the current appleIcon support that might disappear. Let me know if that makes sense!Close #130
Close #198