feat(gatsby-plugin-manifest): add option to generate apple-touch-icons links

[datakurre]

Fixes #5887 /cc @seaneking @moonmeister

I'd like to have gatsby-plugin-manifest to generate apple-touch-icon meta tags (links) for me.

Would it be simple enough to generate tags under the same rules as for favicon (only when the icon is being generated by the plugin)?

This has been required to be a new major release, but is that v2 (still in beta) or v3?

The initial commit datakurre@4cf3ba4 should be the smallest possible change, but I'd probably like to move the code in a separate module where withPrefix is passed as a function argument and could be mocked in tests and make them simpler.

• discussion
• tests
• docs update?

[datakurre]

I may close this without merge. We are considering making a custom plugin for just iOS PWA compatibiliy at #7088

[m-allanson]

This has been required to be a new major release, but is that v2 (still in beta) or v3?

This can be v2 as there have only been beta releases for v2 of this plugin

I may close this without merge. We are considering making a custom plugin for just iOS PWA compatibiliy at #7088

I think it makes sense to have this functionality in gatsby-plugin-manifest instead of a separate plugin, even though adding iOS icons isn't really related to the manifest. I think people using this plugin would mostly expect the icons to work across platforms, so if Gatsby can do that and save people having to worry about adding more plugins then that seems like a good idea.

Are there situations where you'd want a web manifest but you would not want iOS icons?

[datakurre]

@m-allanson

Are there situations where you'd want a web manifest but you would not want iOS icons?

Probably no, of course, but there are more than just icons to fix Web App Manifest gaps with iOS.

So, you would propose that we merge these icon into gatsy-plugin-manifest, but create a new plugin gatsby-plugin-iospwa to

• add meta tags for iOS splash screens
• add meta tag for full viewport-fit
• add code to remember the last opened page between app openings?

[m-allanson]

you would propose that we merge these icon into gatsy-plugin-manifest, but create a new plugin gatsby-plugin-iospwa

Yeah I'm not sure which is the best approach - would be interested to hear what folks think. Roll it all into one plugin or have gatsby-plugin-manifest and a separate gatsby-plugin-iospwa plugin?

[datakurre]

Right. I consider this pull ready from my side, unless changes required.

Let's wait for comments about where stuff mentioned at #7088 should be included

[madeleineostoja]

I'd definitely vote to have this all in one plugin. Can't see a reason to split meta tags vs webmanifest outputs into separate plugins. They're different formats but do the same thing, the only reason you'd have to support iOS meta tags is because Safari doesn't yet support the manifest standard, in that sense this is just an implementation detail for cross browser compat of manifest config.

If you want to play it careful can always throw this behind an option. And re: icons, they were always a little out of the scope of this plugin, but I find it super handy, and again since we're already generating for Android no reason not to include Apple in that.

[madeleineostoja]

Oh one thing I would say though is that generating favicon meta (shortcut icon) should not be enabled automatically. It's pretty common to want different assets for favicon vs pwa icon, since they're vastly different resolutions/uses.

Tbh I think favicons are pretty far out of the scope of this plugin. I've commented elsewhere that they should be included for convenience, but I'm backtracking on that. Just muddies the water, and it's not hard to include your own favicon asset or two manually, as opposed to the million PWA icon formats you need.

[datakurre]

@seaneking Would you have opinion on, where #7088 should be?

Could favicon be opt-out? If so, what would be a good option? favicon with path to a file to be used as favicon or false to skip favicon link?

Funny that even manifest are favicon for "different uses", somehow gatsby-plugin-favicon expanded to cover manifest and gatsby-plugin-manifest expanded to cover favicon :)

[madeleineostoja]

Hmm #7088 is a bit hazier, but I think it fits in the scope of this plugin, since it's still a hack for apple not supporting the manifest standard, which could go away of they ever do. Really this plugin could be called gatsby-plugin-pwa now that it does icon generation as well, but I digress.

To be honest I think automated generation of actual favicons (ie: the 1 or 2 files needed for web tabs) is of limited value. 99% of what gatsby-plugin-favicon does is either manifest/PWA stuff (which this plugin covers better) or really obscure platforms (which this plugin definitely shouldn't handle).

I'm just one data point though. Would be keen to hear from others that rely on plugin-favicon outside of it's PWA coverage.

[calcsam]

@datakurre -- is these a reason you deleted those comments from @KyleAMathews? If a comment has been addressed, can you hide rather than deleting?

[datakurre]

@calcsam I’m sorry to miss the hide button (I still don’t see it at least on mobile). They were bot comments about various builds failing because of unrelated issues. I deleted them to make the discussion here more readable.

[m-allanson]

No problem @datakurre, the Kyle bots should be switched off now, as they were a bit noisy.

[datakurre]

@seaneking As an author of #5887, would you be able to review this and formally require minimal changes to get this approved? In my understanding, this solves the initial feature request in that issues. (This also refactors those features and includes snapshot tests for them.) Favicon generation did exist already before this pull, so this has no effect on that.

I agree on having a more complete and flexible PWA plugin (for more serious PWA we'd also need different icons for Android and iOS, because iOS doesn't support transparency), but that seems to require more discussion and go far beyond the scope this pull.

The related standard is called "Web App Manifest", so gatsby-plugin-manifest is not completely bad name for all PWA stuff.

[madeleineostoja]

Yeah in my mind this PR is pretty much good to go. Just on favicons – the 48x48 size may have already been generated, but adding the rel="shortcut icon" link tag to it is a breaking change that now can't be opted out of, meaning that devs have to use the same icon for their PWA manifest and web favicon. I think this (writing the HTML link for favicons) should just be removed from this PR, and if there's demand for it can always throw it in behind an option flag later.

Other than that 👍 LGTM

[datakurre]

@seaneking What I still don’t understand is, how adding favicon is breaking change, if the latest release already does it.

Sure I can remove it, but how to properly communicate it to the existing users? 🤔

[pieh]

What about overriding the icon? do you think it's needed?

I don't have opinion on that and I think we shouldn't put more features and discussions at this point. We should just do minimum required work to get this merged (so adding opt-in mechanism) and additional features should be considered in separate issue / PR to not create scope creep here as this PR was already opened for far too long

[madeleineostoja]

Just FWIW I remember @KyleAMathews mentioning in the original issue that he wouldn't want this an opt-in purely to avoid a breaking change, and I agree – there's no case where somebody wanting to have this plugin handle icons wouldn't want that shimmed across browsers. Nothing wrong with a breaking change here, that's what semver is for.

[DaleLJefferson]

This PR does not include apple-touch-startup-image is this something we could add?

[LekoArts]

This PR does not include apple-touch-startup-image is this something we could add?

You'd need to create a fitting image for every size and I think that’s just too much. And when I tried it it didn’t even work.

there's no case where somebody wanting to have this plugin handle icons wouldn't want that shimmed across browsers. Nothing wrong with a breaking change here, that's what semver is for.

@seaneking what do you mean? I personally don’t want this plugin do generate any icons for me by default (!)

[pieh]

Just FWIW I remember @KyleAMathews mentioning in the original issue that he wouldn't want this an opt-in purely to avoid a breaking change

I do agree that major version bump isn't reason to not change behaviour, but it's other way around now - iOS safari in 11.3 started supporting manifest, so those extra head tags are no longer needed for them. And because <=11.2 iOS has low (and shrinking) usage share is why I would want this as opt-in. And because it would be opt-in it wouldn't be breaking change that would need major bump.

So the question here is not around version bump (I don't care if we would bump or not), but about default setting.

[LekoArts]

btw, if Apple pushes manifest support further the startup image isn’t necessary anyway. And I agree, the iOS adoption rate is so high that this definitely should be opt-in.

[madeleineostoja]

It's other way around now - iOS safari in 11.3 started supporting manifest, so those extra head tags are no longer needed for them

Good point – legacy support should be opt-in, you're right.

what do you mean? I personally don’t want this plugin do generate any icons for me by default (!)

No, I meant that if you have icons defined for this plugin, then the iOS shimming should be automatic. But as @pieh rightly points out, this PR now falls under legacy support, so yep opt-in is the way to go 👍

[moonmeister]

Given this is legacy maybe we add a flag called "legacy or legacyShims" to the config? Simple Boolean. That way if any other legacy is support gets added it's all one simple flag.

[pieh]

Given this is legacy maybe we add a flag called "legacy or legacyShims" to the config? Simple Boolean. That way if any other legacy is support gets added it's all one simple flag.

I think this make sense (was updating code and didn't notice your comment, so my changes use different silly name for now). Might also be legacy: true for all legacy features and we ever get more legacy (than just apple touch icons) we could change it to legacy: { appleTouchIcons: false, somethingElse: true } and internally transform legacy: true to set true for all those specific options

[moonmeister]

Looks good otherwise.

[moonmeister]

@pieh Everything looks good to me.

As far as the version bump i see no need for a major version bump. This is a simple feature add and is completely backwards compatible since it is opt in.