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

Automatically copy name to short_name in web app manifest if not greater than 12 characters #212

Merged
merged 5 commits into from
Aug 20, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
language: php

dist: trusty

addons:
apt:
packages:
Expand Down
2 changes: 1 addition & 1 deletion pwa.php
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ function _pwa_add_disabled_navigation_preload_site_status_test( $tests ) {
add_filter( 'site_status_tests', '_pwa_add_disabled_navigation_preload_site_status_test' );

/**
* Print admin notice when a build has not been been performed.
* Flag navigation preload incorrectly being disabled.
*
* This is temporary measure to correct a mistake in the example for how navigation request caching strategies.
*
Expand Down
10 changes: 9 additions & 1 deletion tests/test-class-wp-web-app-manifest.php
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ public function test_get_theme_color() {
*/
public function test_get_manifest() {
$this->mock_site_icon();
$blogname = "PWA's Domain Is Here";
$blogname = 'PWA Test';
update_option( 'blogname', $blogname );
$actual_manifest = $this->instance->get_manifest();

Expand All @@ -139,6 +139,7 @@ public function test_get_manifest() {
'description' => get_bloginfo( 'description' ),
'display' => 'minimal-ui',
'name' => $blogname,
'short_name' => $blogname,
'lang' => get_bloginfo( 'language' ),
'dir' => is_rtl() ? 'rtl' : 'ltr',
'start_url' => home_url( '/' ),
Expand All @@ -147,6 +148,13 @@ public function test_get_manifest() {
);
$this->assertEquals( $expected_manifest, $actual_manifest );

// Check that long names do not automatically copy to short name.
$blogname = str_repeat( 'x', 13 );
update_option( 'blogname', $blogname );
$actual_manifest = $this->instance->get_manifest();
$this->assertEquals( $blogname, $actual_manifest['name'] );
$this->assertArrayNotHasKey( 'short_name', $actual_manifest );

// Test that the filter at the end of the method overrides the value.
add_filter( 'web_app_manifest', array( $this, 'mock_manifest' ) );
$actual_manifest = $this->instance->get_manifest();
Expand Down
114 changes: 114 additions & 0 deletions wp-includes/class-wp-web-app-manifest.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,16 @@ class WP_Web_App_Manifest {
*/
const REST_ROUTE = '/web-app-manifest';

/**
* Maximum length for short_name.
*
* @since 0.4
* @link https://developers.google.com/web/tools/lighthouse/audits/manifest-contains-short_name
* @link https://developer.chrome.com/apps/manifest/name#short_name
* @var int
*/
const SHORT_NAME_MAX_LENGTH = 12;

/**
* The default manifest icon sizes.
*
Expand All @@ -51,6 +61,7 @@ class WP_Web_App_Manifest {
public function init() {
add_action( 'wp_head', array( $this, 'manifest_link_and_meta' ) );
add_action( 'rest_api_init', array( $this, 'register_manifest_rest_route' ) );
add_filter( 'site_status_tests', array( $this, 'add_short_name_site_status_test' ) );
}

/**
Expand Down Expand Up @@ -123,6 +134,19 @@ public function get_manifest() {
'display' => 'minimal-ui',
'dir' => is_rtl() ? 'rtl' : 'ltr',
);

/*
* If the name is 12 characters or less, use it as the short_name. Lighthouse complains when the short_name
* is absent, even when the name is 12 characters or less. Chrome's max recommended short_name length is 12
* characters.
*
* https://developers.google.com/web/tools/lighthouse/audits/manifest-contains-short_name
* https://developer.chrome.com/apps/manifest/name#short_name
*/
if ( strlen( $manifest['name'] ) <= self::SHORT_NAME_MAX_LENGTH ) {
$manifest['short_name'] = $manifest['name'];
}

$language = get_bloginfo( 'language' );
if ( $language ) {
$manifest['lang'] = $language;
Expand Down Expand Up @@ -155,6 +179,96 @@ public function get_manifest() {
return apply_filters( 'web_app_manifest', $manifest );
}

/**
* Register test for lacking short_name in web app manifest.
*
* @since 0.4
*
* @param array $tests Tests.
* @return array Tests.
*/
public function add_short_name_site_status_test( $tests ) {
$tests['direct']['web_app_manifest_short_name'] = array(
'label' => __( 'Short Name in Web App Manifest', 'pwa' ),
'test' => array( $this, 'test_short_name_present_in_manifest' ),
);
return $tests;
}

/**
* Test that web app manifest contains a short_name.
*
* @since 0.4
* @todo Add test for PNG site icon.
*
* @return array Test results.
*/
public function test_short_name_present_in_manifest() {
$manifest = $this->get_manifest();

$description = sprintf(
/* translators: %1$s is `short_name`, %2$d is the max length as a number */
__( 'The %1$s is a short version of your website&#8217;s name. It is displayed when there is not enough space for the full name, for example with the site icon on a phone&#8217;s homescreen. It should be a maximum of %2$d characters long.', 'pwa' ),
'<code>short_name</code>',
self::SHORT_NAME_MAX_LENGTH
);

$actions = sprintf(
/* translators: %1$s is `web_app_manifest`, %2$s is `functions.php` */
__( 'You currently may use %1$s filter to set the short name, for example in your theme&#8217;s %2$s.', 'pwa' ),
'<code>web_app_manifest</code>',
'<code>functions.php</code>'
);

if ( empty( $manifest['short_name'] ) ) {
$result = array(
'label' => __( 'Web App Manifest lacks a short name entry', 'pwa' ),
'status' => 'recommended',
'badge' => array(
'label' => __( 'Progressive Web App', 'pwa' ),
'color' => 'orange',
),
'description' => wp_kses_post( sprintf( '<p>%s</p>', $description ) ),
'actions' => wp_kses_post( $actions ),
);
} elseif ( strlen( $manifest['short_name'] ) > self::SHORT_NAME_MAX_LENGTH ) {
$result = array(
'label' =>
sprintf(
/* translators: %1$s is the short name */
__( 'Web App Manifest has a short name (%s) that is too long', 'pwa' ),
esc_html( $manifest['short_name'] )
),
'status' => 'recommended',
'badge' => array(
'label' => __( 'Progressive Web App', 'pwa' ),
'color' => 'orange',
),
'description' => wp_kses_post( sprintf( '<p>%s</p>', $description ) ),
'actions' => wp_kses_post( $actions ),
);
} else {
$result = array(
'label' =>
sprintf(
/* translators: %1$s is the short name */
__( 'Web App Manifest has a short name (%s)', 'pwa' ),
esc_html( $manifest['short_name'] )
),
'status' => 'good',
'badge' => array(
'label' => __( 'Progressive Web App', 'pwa' ),
'color' => 'green',
),
'description' => wp_kses_post( sprintf( '<p>%s</p>', $description ) ),
);
}

$result['test'] = 'web_app_manifest_short_name';

return $result;
}

/**
* Registers the rest route to get the manifest.
*/
Expand Down