feat(icon): add support for icon purpose #246
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #246 +/- ##
==========================================
+ Coverage 73.81% 74.46% +0.64%
==========================================
Files 11 11
Lines 317 325 +8
Branches 98 102 +4
==========================================
+ Hits 234 242 +8
Misses 72 72
Partials 11 11
Continue to review full report at Codecov.
|
docs/modules/icon.md
Outdated
| **purpose** | ||
| - Default: null | ||
|
|
||
| Array of icon purpose. |
There was a problem hiding this comment.
Can you please improve docs by putting more description or links?
There was a problem hiding this comment.
I added some description, please review.
There was a problem hiding this comment.
Hi @hans00, sorry for the late review.
Another note, you need to include tests for this feature.
|
Hey guys, just wanted to say maskable icons are officially out what do we need to get this rolling? Can I help? |
| if (options.purpose) { | ||
| const purpose = Array.isArray(options.purpose) ? options.purpose : [options.purpose] | ||
| const validPurpose = ['badge', 'maskable', 'any'] | ||
| options.purpose = purpose.filter(item => validPurpose.includes(item)) |
There was a problem hiding this comment.
Silently filtering invalid items is not a good idea. Maybe we can show a warning instead? (one idea can be keeping length before filter and warn if length changed)
There was a problem hiding this comment.
warn user is a good idea
lib/icon/module.js
Outdated
| @@ -109,11 +117,15 @@ async function generateIcons (options) { | |||
| options._icons[size] = `${options.targetDir}/icon_${size}.${options.iconHash}.png` | |||
| } | |||
|
|
|||
| // Generate _purpose | |||
| options._purpose = options.purpose ? options.purpose.join(' ') : undefined | |||
There was a problem hiding this comment.
Do we need _purpose anywhere else? If not, can we just use const purpose = ...?
|
Can someone merge this change in, as without this icons have a white background on some phones and don't look very good |
pi0
changed the title
|
I've fixed remaining tasks. Thanks @hans00 and @ricardogobbosouza ❤️ |
Let icon maskable.
https://developer.mozilla.org/en-US/docs/Web/Manifest/icons