Add Site Icon tests for Site Health and warnings in Customizer control #702
Conversation
… as maskable * Lighthouse gives an error if icon is not maskable. * Add a checkbox to allow user to save icon as maskable icon. * If user have not opt-in for maskable icon then manifest will add icon purpose as `any maskable`.
Yes, since the changes of
IMO yes, tests are passing on all other PHP versions except the 7.1 due to bugs related to images. |
|
|
||
| notifications.map((notification) => { | ||
| wp.customize | ||
| .section('title_tagline') |
There was a problem hiding this comment.
Since the control can be moved to another section, it would be best to get the section via siteIconControl.section():
| .section('title_tagline') | |
| .section(siteIconControl.section()) |
There was a problem hiding this comment.
Nevertheless, the notification should rather be added to the control instead of being added to the section.
There was a problem hiding this comment.
A key reason is that the top of the section may be scrolled out of view, so the user may not see it.
| const iconData = wp.customize | ||
| .control('site_icon') | ||
| .container.find('img.app-icon-preview'); | ||
| const NotificationData = { |
There was a problem hiding this comment.
The casing of this variable makes it look like a class. So it should be initially lower-cased.
| const iconData = wp.customize | ||
| .control('site_icon') |
There was a problem hiding this comment.
We actually already have a variable containing the site icon control here: siteIconControl
| const NotificationData = { | ||
| dismissible: true, | ||
| message: '', | ||
| type: 'error', |
There was a problem hiding this comment.
Probably better to make this a warning:
| type: 'error', | |
| type: 'warning', |
|
|
||
| if (!iconData.length) { | ||
| notifications.push( | ||
| new wp.customize.Notification('pwa_icon_not_set', { |
There was a problem hiding this comment.
I don't see this notification being added initially when no icon was set.
There was a problem hiding this comment.
Also, it should get automatically cleared when an icon is set.
| $icon_validation_messages = array( | ||
| 'pwa_icon_not_set' => __( 'Site icon is not present', 'pwa' ), | ||
| 'pwa_icon_too_small' => __( 'Site icon needs to be at least 512 x 512', 'pwa' ), | ||
| 'pwa_icon_no_png' => __( 'Site icon should .png', 'pwa' ), |
There was a problem hiding this comment.
This message isn't being used currently.
| .control('site_icon') | ||
| .container.find('img.app-icon-preview'); | ||
| const NotificationData = { | ||
| dismissible: true, |
There was a problem hiding this comment.
There won't be a need to be dismissible if the notification automatically is removed when the issue is fixed.
| $results['badge']['color'] = 'orange'; | ||
| $results['actions'] = wp_kses_post( | ||
| sprintf( | ||
| '<a class="button button-primary" href="%s">%s</a>', |
There was a problem hiding this comment.
| '<a class="button button-primary" href="%s">%s</a>', | |
| '<a class="button button-secondary" href="%s">%s</a>', |
| return $icon_errors; | ||
| } | ||
|
|
||
| if ( $site_icon_metadata['width'] < 512 && $site_icon_metadata['height'] < 512 ) { |
There was a problem hiding this comment.
| if ( $site_icon_metadata['width'] < 512 && $site_icon_metadata['height'] < 512 ) { | |
| if ( $site_icon_metadata['width'] < 512 || $site_icon_metadata['height'] < 512 ) { |
|
With my latest changes… When a user selects a non-square GIF that is too small in the Customizer:
And in the Site Health test:
|
westonruter
changed the title
Description
When user uploads the site icon via customizer, validate its presence as well as size.
If site icon is not set or is smaller than 512 x 512, issue an error notification in customizer.
In site health screen, check the following.
If any of above fails, issue the warning in site health screen.
Closes #229