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

core: rename webapp-install-banner audit to installable-manifest #6630

Merged
merged 2 commits into from
Nov 27, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 2 additions & 2 deletions lighthouse-cli/test/cli/__snapshots__/index-test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ Object {
"path": "redirects",
},
Object {
"path": "webapp-install-banner",
"path": "installable-manifest",
},
Object {
"path": "splash-screen",
Expand Down Expand Up @@ -840,7 +840,7 @@ Object {
},
Object {
"group": "pwa-installable",
"id": "webapp-install-banner",
"id": "installable-manifest",
"weight": 2,
},
Object {
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-cli/test/smokehouse/offline-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ module.exports = {
'user-timings',
'critical-request-chains',
'render-blocking-resources',
'webapp-install-banner',
'installable-manifest',
'splash-screen',
'themed-omnibox',
'aria-valid-attr',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,10 @@ module.exports = [
'critical-request-chains': {
scoreDisplayMode: 'not-applicable',
},
'webapp-install-banner': {
'installable-manifest': {
score: 0,
explanation: 'Failures: No manifest was fetched.',
details: {items: [{isParseFailure: true}]},
},
'splash-screen': {
score: 0,
Expand Down Expand Up @@ -124,7 +126,7 @@ module.exports = [
'critical-request-chains': {
scoreDisplayMode: 'not-applicable',
},
'webapp-install-banner': {
'installable-manifest': {
score: 1,
},
'splash-screen': {
Expand Down
4 changes: 2 additions & 2 deletions lighthouse-cli/test/smokehouse/pwa-expectations.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ module.exports = [
'load-fast-enough-for-pwa': {
// Ignore speed test; just verify that it ran.
},
'webapp-install-banner': {
'installable-manifest': {
score: 1,
details: {items: [pwaDetailsExpectations]},
},
Expand Down Expand Up @@ -110,7 +110,7 @@ module.exports = [
'load-fast-enough-for-pwa': {
// Ignore speed test; just verify that it ran.
},
'webapp-install-banner': {
'installable-manifest': {
score: 1,
details: {items: [pwaDetailsExpectations]},
},
Expand Down
5 changes: 3 additions & 2 deletions lighthouse-cli/test/smokehouse/pwa2-expectations.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,10 @@ module.exports = [
'load-fast-enough-for-pwa': {
// Ignore speed test; just verify that it ran.
},
'webapp-install-banner': {
'installable-manifest': {
score: 0,
details: {items: [jakeExpectations]},
explanation: /^Failures: .*short_name/,
},
'splash-screen': {
score: 1,
Expand Down Expand Up @@ -104,7 +105,7 @@ module.exports = [
'load-fast-enough-for-pwa': {
// Ignore speed test; just verify that it ran.
},
'webapp-install-banner': {
'installable-manifest': {
score: 1,
details: {items: [pwaDetailsExpectations]},
},
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-cli/test/smokehouse/pwa3-expectations.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ module.exports = [
'load-fast-enough-for-pwa': {
// Ignore speed test; just verify that it ran .
},
'webapp-install-banner': {
'installable-manifest': {
score: 1,
details: {items: [pwaRocksExpectations]},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
*/
'use strict';

const MultiCheckAudit = require('./multi-check-audit');
const MultiCheckAudit = require('./multi-check-audit.js');
const ManifestValues = require('../computed/manifest-values.js');

/**
* @fileoverview
* Audits if a page is configured to prompt users with the webapp install banner.
* Audits if the page's web app manifest qualifies for triggering a beforeinstallprompt event.
* https://github.com/GoogleChrome/lighthouse/issues/23#issuecomment-270453303
*
* Requirements:
Expand All @@ -20,27 +20,21 @@ const ManifestValues = require('../computed/manifest-values.js');
* * manifest has a valid shortname
* * manifest display property is standalone, minimal-ui, or fullscreen
* * manifest contains icon that's a png and size >= 192px
* * SW is registered, and it owns this page and the manifest's start url
* * Site engagement score of 2 or higher

* This audit covers these requirements with the following exceptions:
* * it doesn't consider SW controlling the starturl
* * it doesn't consider the site engagement score (naturally)
*/

class WebappInstallBanner extends MultiCheckAudit {
class InstallableManifest extends MultiCheckAudit {
/**
* @return {LH.Audit.Meta}
*/
static get meta() {
return {
id: 'webapp-install-banner',
title: 'User can be prompted to Install the Web App',
failureTitle: 'User will not be prompted to Install the Web App',
id: 'installable-manifest',
title: 'Web app manifest meets the installability requirements',
failureTitle: 'Web app manifest does not meet the installability requirements',
description: 'Browsers can proactively prompt users to add your app to their homescreen, ' +
'which can lead to higher engagement. ' +
'[Learn more](https://developers.google.com/web/tools/lighthouse/audits/install-prompt).',
requiredArtifacts: ['URL', 'ServiceWorker', 'Manifest'],
requiredArtifacts: ['URL', 'Manifest'],
};
}

Expand Down Expand Up @@ -86,7 +80,7 @@ class WebappInstallBanner extends MultiCheckAudit {
*/
static async audit_(artifacts, context) {
const manifestValues = await ManifestValues.request(artifacts.Manifest, context);
const manifestFailures = WebappInstallBanner.assessManifest(manifestValues);
const manifestFailures = InstallableManifest.assessManifest(manifestValues);

return {
failures: [
Expand All @@ -97,4 +91,4 @@ class WebappInstallBanner extends MultiCheckAudit {
}
}

module.exports = WebappInstallBanner;
module.exports = InstallableManifest;
1 change: 0 additions & 1 deletion lighthouse-core/audits/multi-check-audit.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ class MultiCheckAudit extends Audit {
...result,
...result.manifestValues,
manifestValues: undefined,
warnings: undefined,
allChecks: undefined,
};

Expand Down
7 changes: 0 additions & 7 deletions lighthouse-core/computed/manifest-values.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,6 @@ const PWA_DISPLAY_VALUES = ['minimal-ui', 'fullscreen', 'standalone'];
const SUGGESTED_SHORTNAME_LENGTH = 12;

class ManifestValues {
static get validityIds() {
return ['hasManifest', 'hasParseableManifest'];
}

/** @typedef {(val: NonNullable<LH.Artifacts.Manifest['value']>) => boolean} Validator */

/**
Expand Down Expand Up @@ -86,8 +82,6 @@ class ManifestValues {
*/
static async compute_(manifest) {
// if the manifest isn't there or is invalid json, we report that and bail
let parseFailureReason;

if (manifest === null) {
return {
isParseFailure: true,
Expand Down Expand Up @@ -115,7 +109,6 @@ class ManifestValues {

return {
isParseFailure: false,
parseFailureReason,
allChecks: remainingChecks,
};
}
Expand Down
4 changes: 2 additions & 2 deletions lighthouse-core/config/default-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ const defaultConfig = {
'user-timings',
'critical-request-chains',
'redirects',
'webapp-install-banner',
'installable-manifest',
'splash-screen',
'themed-omnibox',
'content-width',
Expand Down Expand Up @@ -371,7 +371,7 @@ const defaultConfig = {
// Installable
{id: 'is-on-https', weight: 2, group: 'pwa-installable'},
{id: 'service-worker', weight: 1, group: 'pwa-installable'},
{id: 'webapp-install-banner', weight: 2, group: 'pwa-installable'},
{id: 'installable-manifest', weight: 2, group: 'pwa-installable'},
// PWA Optimized
{id: 'redirects-http', weight: 2, group: 'pwa-optimized'},
{id: 'splash-screen', weight: 1, group: 'pwa-optimized'},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/
'use strict';

const WebappInstallBannerAudit = require('../../audits/webapp-install-banner');
const InstallableManifestAudit = require('../../audits/installable-manifest.js');
const assert = require('assert');
const manifestParser = require('../../lib/manifest-parser');

Expand Down Expand Up @@ -37,7 +37,7 @@ describe('PWA: webapp install banner audit', () => {
artifacts.Manifest = null;
const context = generateMockAuditContext();

return WebappInstallBannerAudit.audit(artifacts, context).then(result => {
return InstallableManifestAudit.audit(artifacts, context).then(result => {
assert.strictEqual(result.rawValue, false);
assert.ok(result.explanation.includes('No manifest was fetched'), result.explanation);
});
Expand All @@ -47,7 +47,7 @@ describe('PWA: webapp install banner audit', () => {
const artifacts = generateMockArtifacts();
artifacts.Manifest = manifestParser('{,:}', EXAMPLE_MANIFEST_URL, EXAMPLE_DOC_URL);
const context = generateMockAuditContext();
return WebappInstallBannerAudit.audit(artifacts, context).then(result => {
return InstallableManifestAudit.audit(artifacts, context).then(result => {
assert.strictEqual(result.rawValue, false);
assert.ok(result.explanation.includes('failed to parse as valid JSON'));
});
Expand All @@ -57,7 +57,7 @@ describe('PWA: webapp install banner audit', () => {
const artifacts = generateMockArtifacts();
artifacts.Manifest = manifestParser('{}', EXAMPLE_MANIFEST_URL, EXAMPLE_DOC_URL);
const context = generateMockAuditContext();
return WebappInstallBannerAudit.audit(artifacts, context).then(result => {
return InstallableManifestAudit.audit(artifacts, context).then(result => {
assert.strictEqual(result.rawValue, false);
assert.ok(result.explanation);
assert.strictEqual(result.details.items[0].failures.length, 4);
Expand All @@ -66,39 +66,43 @@ describe('PWA: webapp install banner audit', () => {

it('passes with complete manifest and SW', () => {
const context = generateMockAuditContext();
return WebappInstallBannerAudit.audit(generateMockArtifacts(), context).then(result => {
return InstallableManifestAudit.audit(generateMockArtifacts(), context).then(result => {
assert.strictEqual(result.rawValue, true, result.explanation);
assert.strictEqual(result.explanation, undefined, result.explanation);
});
});
});

describe('one-off-failures', () => {
/* eslint-disable camelcase */ // because start_url
it('fails when a manifest contains no start_url', () => {
const artifacts = generateMockArtifacts();
artifacts.Manifest.value.start_url.value = undefined;
const context = generateMockAuditContext();

return WebappInstallBannerAudit.audit(artifacts, context).then(result => {
return InstallableManifestAudit.audit(artifacts, context).then(result => {
assert.strictEqual(result.rawValue, false);
assert.ok(result.explanation.includes('start_url'), result.explanation);
const failures = result.details.items[0].failures;
assert.strictEqual(failures.length, 1, failures);

const details = result.details.items[0];
assert.strictEqual(details.failures.length, 1, details.failures);
assert.strictEqual(details.hasStartUrl, false);
assert.strictEqual(details.hasShortName, true);
});
});

/* eslint-disable camelcase */ // because short_name
it('fails when a manifest contains no short_name', () => {
const artifacts = generateMockArtifacts();
artifacts.Manifest.value.short_name.value = undefined;
const context = generateMockAuditContext();

return WebappInstallBannerAudit.audit(artifacts, context).then(result => {
return InstallableManifestAudit.audit(artifacts, context).then(result => {
assert.strictEqual(result.rawValue, false);
assert.ok(result.explanation.includes('short_name'), result.explanation);
const failures = result.details.items[0].failures;
assert.strictEqual(failures.length, 1, failures);

const details = result.details.items[0];
assert.strictEqual(details.failures.length, 1, details.failures);
assert.strictEqual(details.hasStartUrl, true);
assert.strictEqual(details.hasShortName, false);
});
});

Expand All @@ -107,11 +111,14 @@ describe('PWA: webapp install banner audit', () => {
artifacts.Manifest.value.name.value = undefined;
const context = generateMockAuditContext();

return WebappInstallBannerAudit.audit(artifacts, context).then(result => {
return InstallableManifestAudit.audit(artifacts, context).then(result => {
assert.strictEqual(result.rawValue, false);
assert.ok(result.explanation.includes('name'), result.explanation);
const failures = result.details.items[0].failures;
assert.strictEqual(failures.length, 1, failures);

const details = result.details.items[0];
assert.strictEqual(details.failures.length, 1, details.failures);
assert.strictEqual(details.hasStartUrl, true);
assert.strictEqual(details.hasName, false);
});
});

Expand All @@ -120,11 +127,14 @@ describe('PWA: webapp install banner audit', () => {
artifacts.Manifest.value.icons.value = [];
const context = generateMockAuditContext();

return WebappInstallBannerAudit.audit(artifacts, context).then(result => {
return InstallableManifestAudit.audit(artifacts, context).then(result => {
assert.strictEqual(result.rawValue, false);
assert.ok(result.explanation.includes('PNG icon'), result.explanation);
const failures = result.details.items[0].failures;
assert.strictEqual(failures.length, 1, failures);

const details = result.details.items[0];
assert.strictEqual(details.failures.length, 1, details.failures);
assert.strictEqual(details.hasStartUrl, true);
assert.strictEqual(details.hasIconsAtLeast192px, false);
});
});
});
Expand All @@ -133,11 +143,14 @@ describe('PWA: webapp install banner audit', () => {
const artifacts = generateMockArtifacts(manifestDirtyJpgSrc);
const context = generateMockAuditContext();

return WebappInstallBannerAudit.audit(artifacts, context).then(result => {
return InstallableManifestAudit.audit(artifacts, context).then(result => {
assert.strictEqual(result.rawValue, false);
assert.ok(result.explanation.includes('PNG icon'), result.explanation);
const failures = result.details.items[0].failures;
assert.strictEqual(failures.length, 1, failures);

const details = result.details.items[0];
assert.strictEqual(details.failures.length, 1, details.failures);
assert.strictEqual(details.hasStartUrl, true);
assert.strictEqual(details.hasIconsAtLeast192px, false);
});
});
});
10 changes: 5 additions & 5 deletions lighthouse-core/test/results/sample_v2.json
Original file line number Diff line number Diff line change
Expand Up @@ -460,9 +460,9 @@
"overallSavingsMs": 0
}
},
"webapp-install-banner": {
"id": "webapp-install-banner",
"title": "User will not be prompted to Install the Web App",
"installable-manifest": {
"id": "installable-manifest",
"title": "Web app manifest does not meet the installability requirements",
"description": "Browsers can proactively prompt users to add your app to their homescreen, which can lead to higher engagement. [Learn more](https://developers.google.com/web/tools/lighthouse/audits/install-prompt).",
"score": 0,
"scoreDisplayMode": "binary",
Expand Down Expand Up @@ -2951,7 +2951,7 @@
"group": "pwa-installable"
},
{
"id": "webapp-install-banner",
"id": "installable-manifest",
"weight": 2,
"group": "pwa-installable"
},
Expand Down Expand Up @@ -3697,7 +3697,7 @@
},
{
"startTime": 0,
"name": "lh:audit:webapp-install-banner",
"name": "lh:audit:installable-manifest",
"duration": 100,
"entryType": "measure"
},
Expand Down
Loading