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

[gatsby-plugin-manifest] Favicon not updated on navigating to child app #17434

Closed
namukang opened this issue Sep 6, 2019 · 15 comments
Closed
Labels
topic: plugins-PWA Issues related to PWA: the gatsby-plugin-offline and gatsby-plugin-manifest plugins type: feature or enhancement Issue that is not a bug and requests the addition of a new feature or enhancement.

Comments

@namukang
Copy link
Contributor

namukang commented Sep 6, 2019

Description

First, some notes on terminology:

  • "Main app" refers to the PWA created by the main manifest by default.
  • "Child app" refers to the PWA created by the additional manifests generated by the use of the localize option.

When a child app in localize is loaded, the icon in the child manifest is not used for the favicon or apple-touch-icon links.

Conditions

The plugin options for gatsby-plugin-manifest include a child app via the localize array and it specifies a different icon.

// gatsby-config.js
  {
      resolve: "gatsby-plugin-manifest",
      options: {
        name: "Hello World",
        start_url: "/",
        icon: "src/images/gatsby-icon.png",
        localize: [
          {
            start_url: "/es/",
            lang: "es",
            name: "Hola Mundo",
            icon: "src/images/gatsby-icon-es.png",
          },
        ],
      },
}

Steps to reproduce

Demo repo: https://github.com/dkthehuman/gatsby-manifest-l10n-bug-demo
Netlify: https://affectionate-franklin-dc4bfb.netlify.com

  • Navigate to the Netlify demo (or run gatsby build && gatsby serve on the test repo)
    • The favicon and apple-touch-icon links point to gatsby-icon.png (the purple Gatsby icon) as expected
  • Click on the "Spanish" link to navigate to /es/.
    • The favicon does not update to gatsby-icon-es.png (a red dot) as it should
@namukang
Copy link
Contributor Author

namukang commented Sep 6, 2019

I've been working on fixing this and have a solution for the SSR case (when the page is initially loaded), but I'm having trouble updating the icons when the user navigates to a child app.

My current approach:

  • Hook into onRouteUpdate in gatsby-browser.js and detect when the current set of icons differs from what icons should be used given the location
  • Replace the existing set of <link> tags with ones that point to the right icons

For example, using the manifest shown in my initial post, if a user navigates from / to /es/, update all the icons from (the resized versions of) gatsby-icon.png to gatsby-icon-es.png.

Where I got stuck:

  • When the icon filenames for the main and child apps are generated, they include a digest for cache busting (e.g. /icons/icon-48x48-a69f99e1724eb69cbe3031f76a16203d.png)
  • When I detect that the current set of icons should be changed, I don't know how to get the appropriate filename for the icons that should be used because I can't read the file and generate the digest inside gatsby-browser.js to determine the filename like it's done in gatsby-ssr.js

My question:

How would I go about figuring out filenames that include digests from the client?

Is there some way to save a mapping from a child app to the names of its icons during the bootstrap or build stage in either gatsby-node.js or gatsby-ssr.js and access it later from gatsby-browser.js?

@namukang
Copy link
Contributor Author

namukang commented Sep 6, 2019

@moonmeister @wardpeet I'd appreciate any pointers you might have so I can contribute back to gatsby-plugin-manifest! If you have another approach in mind that I can investigate, please let me know.

@moonmeister
Copy link
Contributor

@dkthehuman Those file names are correctly stored in the manifest, which is json...you could parse that. This seems like a couple different issues though. Currently localization only applies to the web app manifest. Maybe just fix that in this PR. If you want to try and extend the localization to affect the iOS icons and favicon, then I'd recommend a different PR to add that functionality and the related onRouteUpdate changes.

Also, I'm not too familiar with how Gatsby handles data but I'm pretty sure it ships per page JSON objects with data used at build time. @wardpeet Is there a way to add data to that so we can reference which icon should be used on any given page?

@namukang
Copy link
Contributor Author

namukang commented Sep 6, 2019

Hadn't thought of that, thanks! Still, I'm hoping that there's some way to solve this without fetching the manifest and parsing it. :)

@gatsbot

This comment has been minimized.

@gatsbot gatsbot bot added the stale? Issue that may be closed soon due to the original author not responding any more. label Oct 14, 2019
@moonmeister moonmeister added not stale and removed stale? Issue that may be closed soon due to the original author not responding any more. labels Oct 15, 2019
@moonmeister
Copy link
Contributor

@dkthehuman Was this fixed by #17426 or is that a different issue? If not can you clarify how this issue is different?

@namukang
Copy link
Contributor Author

@moonmeister Nope, that was a different issue.

This issue has to do with the favicons not updating when navigating to a part of the app under a different manifest (what I'm calling a "child app"). With #17426, the manifest is now updated but the favicon is not.

@moonmeister
Copy link
Contributor

With the merging of #23077 this might be easier to resolve now that we completely control the favicon and its naming.

@moonmeister moonmeister changed the title [gatsby-plugin-manifest] Icons not updated on navigating to child app [gatsby-plugin-manifest] Favicon not updated on navigating to child app Apr 21, 2020
@sfonua10
Copy link

why is it so difficult to add something simple like a favicon in gatsbyjs?

@moonmeister
Copy link
Contributor

moonmeister commented May 15, 2020

It's not. No one has needed to scale favicons to be unique across languages and locales; thus, no one has taken the time to solve this problem.

If this is your trouble, then we welcome you to help solve this problem with constructive conversation, thoughtful ideas, and good code.

@danabrit danabrit added topic: plugins-PWA Issues related to PWA: the gatsby-plugin-offline and gatsby-plugin-manifest plugins type: feature or enhancement Issue that is not a bug and requests the addition of a new feature or enhancement. labels May 21, 2020
@agazso
Copy link

agazso commented Jun 26, 2020

I ran into the same problem and I solved it on the server side by creating the digest based on the localize icon.

In the browser I ended up with the same problem as @dkthehuman, namely that the digest is missing from the information that is available in the runtime. Or more precisely only the Main app digest is there, but it's missing for all the localized pages. Here I am talking about an object that is referenced in the code as pluginObject and it originally comes from the gatsby-config.js where it is defined as the options property of the gatsby-plugin-manifest description, but it is augmented during the server side rendering, fore example with cacheDigest property for the main page.

The idea I came up with is the following: we could also calculate the digest for all localized objects where there is an icon and store it as a cacheDigest property as well during the server side rendering.

Then in the browser the pluginObject is passed to the onRouteUpdate and it could use it to rewrite the link elements when the route changes to a localized page or back. There are many small details of course that need to be taken care of but this would be a big picture.

What do you think?

@namukang
Copy link
Contributor Author

Hey @agazso, I wasn't able to figure out a way to support cache busting so I ended up building a custom plugin to handle this use case without cache busting support: https://github.com/dkthehuman/gatsby-plugin-manifest-multiple

I got a bit carried away and ended up refactoring all of gatsby-plugin-manifest to use a more modular structure, but I wrote this back in September and haven't kept it in sync with changes since then so use at your own risk. 😅

@agazso
Copy link

agazso commented Jun 26, 2020

@dkthehuman Nice!

I will take a look at your plugin because it seems that it supports my use-case (having sub-sites with different icon instead of localized pages) better and the code also looks cleaner at first glance. Maybe combined that with my approach for cache busting we can have the best of both.

@LekoArts
Copy link
Contributor

Since this issue didn't gain a lot of traction / a lot of people requested this I'll defer to the community plugin if you need this functionality. Thanks for creating it!

@moonmeister
Copy link
Contributor

@LekoArts this was a bug not a feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: plugins-PWA Issues related to PWA: the gatsby-plugin-offline and gatsby-plugin-manifest plugins type: feature or enhancement Issue that is not a bug and requests the addition of a new feature or enhancement.
Projects
None yet
Development

No branches or pull requests

6 participants