-
Notifications
You must be signed in to change notification settings - Fork 166
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
Add support for Android 13 themed icons #650
base: main
Are you sure you want to change the base?
Conversation
722b65b
to
ffecea9
Compare
ffecea9
to
127513b
Compare
Code looks good to me - I agree with Andre about putting this behind a flag until the Android 13 APIs are fixed. |
Hi @NotWoods and @andreban , two days ago we had the official release of Android 13 for Pixel devices. https://developer.android.com/about/versions/13 Would be nice if this feature could be implemented so my apps will fit with the new themes icons. Anyways thanks for your amazing work and making this possible! |
is it worth adding some description to the readme? |
hey all! love this PR. It would be awesome if this could be approved and merged now that Android 13 is no longer in beta :) |
I haven't tested it since Android 13 came out of beta. If anyone has a chance to do it and let me know if it works, happy to merge. |
Could you please rebase this PR and publish alpha npm package for this branch (like ardatan/graphql-tools#2065 (comment)) for easier testing by more people? Is that exactly what the |
Yep, that's what |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you!
fixes a misleading statement that can be a problem to new users, example: [thread discord](https://discord.com/channels/930156205542883409/1265639864246996992/1265672310287892560) ## PR Type <!-- Please uncomment one ore more that apply to this PR --> - Documentation content changes ## Describe the current behavior? <!-- Please describe the current behavior that is being modified or link to a relevant issue. --> - The android docs are outdated compared to the builder forms page and tooltips. ## Describe the new behavior? - Docs synced with page/tooltips. ![image](https://github.com/user-attachments/assets/918eb610-0dc4-4d2d-860a-92168dcdb514) ## PR Checklist - [X] Test: run `npm run test` and ensure that all tests pass - [X] Target main branch (or an appropriate release branch if appropriate for a bug fix) - [ ] Ensure that your contribution follows [standard accessibility guidelines](https://docs.microsoft.com/en-us/microsoft-edge/accessibility/design). Use tools like https://webhint.io/ to validate your changes. ## Additional Information - Also added an additional note for monochrome icons which won't work as themed icons because it's missing the support from bubblewrap here: GoogleChromeLabs/bubblewrap#650
I'm guessing this is waiting for WebAPK to support themed icons first? If so, looks like it's blocked: https://issues.chromium.org/issues/40277264
|
The WebAPK system is separate. I've also commented in the issue tracker that sinansahin's comment was inaccurate as WebAPK and bubblewrap create launcher icons, not dynamic shortcuts. |
Android 13 introduces an API for Themed Icons, which are monochrome icons on a colored background. This change adds support to create those icons from a web app manifest.
Note that Android requires an Adaptive Icon for this to work. Consequently, you must have a maskable icon AND a monochrome icon.
Tested with https://monochrome.fyi/manifest.json