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

fix(pwa): add maskable icon to web manifest #1456

Merged
merged 5 commits into from
Jan 25, 2023

Conversation

userquin
Copy link
Member

Latest changes on Chromium based browsers complains about not having a maskable icon (LightHouse): just added the variants adding 10px padding to current apple-touch-icon.png icons.

Tested on Android from Chrome:

Screenshot_20230125_181456_One UI Home

I need someone to check it on iOS...

/cc @antfu sorry man, can you try installing preview and check the icon is ok?

/cc @josh-collinsworth can you test preview PR on Edge Mobile?

closes #1449

@stackblitz
Copy link

stackblitz bot commented Jan 25, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@netlify
Copy link

netlify bot commented Jan 25, 2023

Deploy Preview for elk-zone ready!

Name Link
🔨 Latest commit 30566b1
🔍 Latest deploy log https://app.netlify.com/sites/elk-zone/deploys/63d18c6c55bef900080c4052
😎 Deploy Preview https://deploy-preview-1456--elk-zone.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Jan 25, 2023

Deploy Preview for elk-docs canceled.

Name Link
🔨 Latest commit 30566b1
🔍 Latest deploy log https://app.netlify.com/sites/elk-docs/deploys/63d18c6c5be43b000844ebb8

@josh-collinsworth
Copy link

@userquin Know what? I'm sorry, I'm wrong. I think the "bug" I reported is actually a normal effect from using non-Chrome browsers to install PWAs on Android, and not anything to do with the PWA itself.

I must have used Chrome to install the ones that worked properly, and used Edge to install the others, and completely forgotten.

So yeah, I can confirm that it installs properly when using Chrome as the installing user agent, and that it does not work with Edge—but, again, this is expected.

Still, the maskable icon is a win, though I'd recommend it be higher resolution. (Here's what the PWA loading screen looked like before—all black—and what it looks like with this PR—the round, lower-res icon.)

Before:
Screenshot_20230125-113813

After:
Screenshot_20230125-113425

@josh-collinsworth
Copy link

Unless there's time and energy to spend on making the maskable icon high-res, I'd suggest maybe this issue and PR should just be closed, since the default seemed to work well for me. (Again, I apologize for sending you on a wild goose chase with this report.)

@userquin
Copy link
Member Author

@josh-collinsworth thx for feedback, I'll add a 512x512 maskable icon, I need to create and optimize them...

pref: optimize pwa images
@userquin
Copy link
Member Author

userquin commented Jan 25, 2023

Unless there's time and energy to spend on making the maskable icon high-res, I'd suggest maybe this issue and PR should just be closed, since the default seemed to work well for me. (Again, I apologize for sending you on a wild goose chase with this report.)

can you test again when ready?

@josh-collinsworth
Copy link

Looks much better, resolution-wise. It does still have the light-colored circle background. If that's the desired effect, I'd say 👍

fix: dev apple-touch-icon with white bg color
@userquin
Copy link
Member Author

Looks much better, resolution-wise. It does still have the light-colored circle background. If that's the desired effect, I'd say 👍

Looks much better, resolution-wise. It does still have the light-colored circle background. If that's the desired effect, I'd say 👍

can you test again? maskable icons sizes were wrong (180px, updated to 512px)

@josh-collinsworth
Copy link

Still looks good on my end.

@userquin userquin marked this pull request as ready for review January 25, 2023 20:10
@patak-dev
Copy link
Member

Awesome!

@patak-dev patak-dev merged commit 050092b into main Jan 25, 2023
@patak-dev patak-dev deleted the userquin/fix-add-maskable-icon branch January 25, 2023 20:16
@dunxen
Copy link

dunxen commented Feb 22, 2023

Sorry to draw attention to this again, but it seems on the iOS 16.4 Beta (which I'm running now) it seems it prefers the PWA icons (since they're supporting a lot more on the manifest side of things also push which has been working for me with Elk 🎉).

This is how the icon looks for me now:
signal-2023-02-22-133235

I can open another issue if one does not currently exist.

@userquin
Copy link
Member Author

@dunxen so, it seems iOS now using 512x512 pwa icon instead maskable one and ignoring apple-touch-icon?

@dunxen
Copy link

dunxen commented Feb 22, 2023

@dunxen so, it seems iOS now using 512x512 pwa icon instead maskable one and ignoring apple-touch-icon?

Yes looks like that!

@userquin
Copy link
Member Author

@dunxen FYI: https://twitter.com/firt/status/1628378241726545921

@dunxen
Copy link

dunxen commented Mar 2, 2023

@dunxen FYI: https://twitter.com/firt/status/1628378241726545921

Thanks! Seems to be working on 16.4 Dev Beta 2:

image

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

Successfully merging this pull request may close these issues.

PWA is not fully configured for Android
4 participants