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.

Sign up for GitHub

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

Jump to bottom

[plugin-manifest] Write necessary meta tags to HTML as well #5887

Closed
madeleineostoja opened this issue Jun 14, 2018 · 6 comments · Fixed by #7256
Closed

[plugin-manifest] Write necessary meta tags to HTML as well #5887

madeleineostoja opened this issue Jun 14, 2018 · 6 comments · Fixed by #7256
Labels
help wanted Issue with a clear description that the community can help with.

Comments

@madeleineostoja
Copy link

madeleineostoja commented Jun 14, 2018 •
edited
Loading

Summary

gatsby-plugin-manifest is awesome both for creating manifest.webmanifest files and also generating icon sets for a site. Unfortunately Apple still doesn't support icons defined in a manifest file and needs <link> tags. That makes the icon feature of plugin-manifest only useful for Android PWAs. It would be great if plugin-manifest could write the relevant meta tags for icons (both Apple and favicons) to HTML as well.

Basic example

To avoid a breaking change you'd probably want this opt-in, something like a writeHTML: true option.

Motivation

This would mean you could feed a single icon file to plugin-manifest and have everything taken care of for you. Right now you still need to manually set icons for the use-cases that don't support an icon manifest (web and iOS).

Granted that automatically handling all this icon stuff for you is slightly outside the scope of a strict 'manifest' plugin, but so is generating icons in the first place. So might as well go all the way and do it properly.
@madeleineostoja madeleineostoja changed the title [plugin-manifest] Write necessary meta fields to HTML as well [plugin-manifest] Write necessary meta tags to HTML as well Jun 14, 2018
@KyleAMathews
Copy link
Contributor

Oh, didn't know that! Yeah, this would be great to change. Since this is a feature addition and we wouldn't be changing any existing behavior, it's not a breaking change. We can just start adding the meta tags to people's sites.

Would you like to put together a PR adding support for this?

@madeleineostoja
Copy link
Author

madeleineostoja commented Jun 14, 2018 •
edited
Loading

I was more thinking breaking in the sense that you'd start automatically tampering with their <head> (eg: overriding any manually defined icons they already had, if plugin-manifest runs after Helmet or whatever they're using). Though I guess that's a grey area, bumping a minor would probably cover it.

Sure I can take a stab, super new to Gatsby though so would have to do some digging on how plugins actually work haha. If anyone else more familiar with gatsby's innards wants to submit a PR in the meantime go for it.

@madeleineostoja madeleineostoja mentioned this issue Jun 14, 2018
Closed
@KyleAMathews
Copy link
Contributor

Ah, yeah that would be a breaking change. Hmmm yeah, not too big of deal though incrementing a major version. I really dislike adding a config option for something everyone would want.

@moonmeister
Copy link
Contributor

moonmeister commented Jun 15, 2018 •
edited
Loading

@seaneking I had originally planned to do this but didn't have time before heading out on my current endeavor. Glad someone is able to help with this.

Once you get into the code for this it should be fairly simple. I have really limited internet access but I will try and answer any questions if you have any.

I'd be in favor of a config option that defaults false so folks have to explicitly turn on apple compatibility and favicon support.

PS/fyi - saffari is working on webapp support, not sure when it'll be complete. If you look at my original pr for this feature I think I laid out all the current support by browser at the time. Safari Mac also has its own weird thing that's different then mobile devices.

@madeleineostoja
Copy link
Author

madeleineostoja commented Jun 15, 2018 •
edited
Loading

Wouldn't count on Safari shipping support for manifest icons any time soon - they may, they may not, there's no way to know. Don't have the best track record with community transparency in these things.

I'd echo @KyleAMathews that (if we're happy with a major ver) apple icons shouldn't be a config option, since that's just getting parity with the manifest icons this already does automatically. Favicons I could see an argument for either way.

I'm pretty flat out atm myself, but will see if I can get to this some time in the next couple of weeks.

@moonmeister
Copy link
Contributor

Sounds good. Well if you or anyone else doesn't get to it by October when I'm back in civilization I'll probably work on it.

@m-allanson m-allanson added help wanted Issue with a clear description that the community can help with. 🏷 type: feature labels Jun 18, 2018
@datakurre datakurre mentioned this issue Aug 12, 2018
Merged
3 tasks
pieh pushed a commit that referenced this issue Nov 10, 2018
@datakurre @pieh
feat(gatsby-plugin-manifest): add option to generate apple-touch-icon…
0b9d656 
…s links (#7256)

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.

* [x] discussion
* [x] tests
* [x] docs update?
gpetrioli pushed a commit to gpetrioli/gatsby that referenced this issue Jan 22, 2019
@datakurre @gpetrioli
feat(gatsby-plugin-manifest): add option to generate apple-touch-icon…
c443184 
…s links (gatsbyjs#7256)

Fixes gatsbyjs#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.

* [x] discussion
* [x] tests
* [x] docs update?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
help wanted Issue with a clear description that the community can help with.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(gatsby-plugin-manifest): add option to generate apple-touch-icons links
4 participants
@KyleAMathews @m-allanson @moonmeister @madeleineostoja