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

[plugin-manifest] Option to generate maskable favicons #12793

Closed
kripod opened this issue Mar 23, 2019 · 7 comments
Closed

[plugin-manifest] Option to generate maskable favicons #12793

kripod opened this issue Mar 23, 2019 · 7 comments

Comments

@kripod
Copy link
Contributor

kripod commented Mar 23, 2019

Summary

Currently, icon masks are not supported by gatsby-plugin-favicon, thus, Android releases with circular icons may present the application icon as a rectangle inside a circle:

image

Basic example

Add a boolean icon_maskable option or an object-specifiable icon_options override to be merged with each item of the generated or manually-specified icons array, e.g.:

// gatsby-config.js
module.exports = {
  plugins: [
    {
      resolve: `gatsby-plugin-manifest`,
      options: {
        // ...
        icon: `src/images/icon.png`,
        icon_options: {
          purpose: `maskable`,
        },
      },
    },
  ],
}

Motivation

PWAs with maskable icons feel more "native" than uncroppable rectangular logos.

@moonmeister
Copy link
Contributor

moonmeister commented Mar 23, 2019

@kripod Check out the "Hybrid" mode in the docs. It allows you to specify your on icons array where you could set these features. As far as supporting it by default, I'm not sure how we could guarantee it's safe to mask anything the dev provides. It also seems like purpose: could be set to any?

@kripod
Copy link
Contributor Author

kripod commented Mar 23, 2019

@moonmeister I already took a peek on hybrid mode, but as far as I understand, I would have to specify the icons array as follows in order to keep all the sizes generated by default:

[
  {
    src: `icons/icon-48x48.png`,
    sizes: `48x48`,
    type: `image/png`,
    purpose: `maskable`,
  },
  {
    src: `icons/icon-72x72.png`,
    sizes: `72x72`,
    type: `image/png`,
    purpose: `maskable`,
  },
  {
    src: `icons/icon-96x96.png`,
    sizes: `96x96`,
    type: `image/png`,
    purpose: `maskable`,
  },
  {
    src: `icons/icon-144x144.png`,
    sizes: `144x144`,
    type: `image/png`,
    purpose: `maskable`,
  },
  {
    src: `icons/icon-192x192.png`,
    sizes: `192x192`,
    type: `image/png`,
    purpose: `maskable`,
  },
  {
    src: `icons/icon-256x256.png`,
    sizes: `256x256`,
    type: `image/png`,
    purpose: `maskable`,
  },
  {
    src: `icons/icon-384x384.png`,
    sizes: `384x384`,
    type: `image/png`,
    purpose: `maskable`,
  },
  {
    src: `icons/icon-512x512.png`,
    sizes: `512x512`,
    type: `image/png`,
    purpose: `maskable`,
  },
]

Please see #12794 for a possible implementation of icon_options.

kripod added a commit that referenced this issue Mar 23, 2019
@moonmeister
Copy link
Contributor

moonmeister commented Mar 23, 2019

Yeah, that'd be correct. This doesn't seem unreasonable. IMO this plugin is already getting an un-wiedly set of features, and I don't see a lot of benefit in this but plenty of opportunity for confusion. If you were maintaining a manifest without the plugin it'd be no different. We can see what some of the core maintainers think but that's my 2 cents. // CC: @gatsbyjs/core

@wardpeet
Copy link
Contributor

We're pretty stoked to have this. We know that there is already so much but the icon_options give us a good way to enable more options without editing the plugin.

@moonmeister
Copy link
Contributor

@wardpeet awesome. Let's make it happen.

wardpeet pushed a commit that referenced this issue Mar 27, 2019
…t the purpose property (#12794)

<!--
  Have any questions? Check out the contributing docs at https://gatsby.dev/contribute, or
  ask in this Pull Request and a Gatsby maintainer will be happy to help :)
-->

## Description

Adds an option to specify PWA icon options, e.g. [`purpose`](https://www.w3.org/TR/appmanifest/#purpose-member).

## Related Issues

Resolves #12793
@wardpeet
Copy link
Contributor

thanks for your feedback @moonmeister it's really valuable to push back on options if necessary so keep those comments coming! :)

@moonmeister
Copy link
Contributor

Thanks!

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

No branches or pull requests

3 participants