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(viewport): create ViewportMeta computed artifact #7264

Merged
merged 13 commits into from
Mar 1, 2019
11 changes: 6 additions & 5 deletions lighthouse-core/audits/seo/font-size.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
const URL = require('../../lib/url-shim');
const i18n = require('../../lib/i18n/i18n.js');
const Audit = require('../audit');
const ViewportAudit = require('../viewport');
const ComputedViewportMeta = require('../../computed/viewport-meta');
mattzeunert marked this conversation as resolved.
Show resolved Hide resolved
const MINIMAL_PERCENTAGE_OF_LEGIBLE_TEXT = 60;

const UIStrings = {
Expand Down Expand Up @@ -213,11 +213,12 @@ class FontSize extends Audit {

/**
* @param {LH.Artifacts} artifacts
* @return {LH.Audit.Product}
* @param {LH.Audit.Context} context
* @return {Promise<LH.Audit.Product>}
*/
static audit(artifacts) {
const hasViewportSet = ViewportAudit.audit(artifacts).rawValue;
if (!hasViewportSet) {
static async audit(artifacts, context) {
const viewportMeta = await ComputedViewportMeta.request(artifacts, context);
if (!viewportMeta.hasMobileViewport) {
return {
rawValue: false,
explanation: str_(UIStrings.explanationViewport),
Expand Down
11 changes: 6 additions & 5 deletions lighthouse-core/audits/seo/tap-targets.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
* no other tap target that's too close so that the user might accidentally tap on.
*/
const Audit = require('../audit');
const ViewportAudit = require('../viewport');
const ComputedViewportMeta = require('../../computed/viewport-meta');
mattzeunert marked this conversation as resolved.
Show resolved Hide resolved
const {
rectsTouchOrOverlap,
getRectOverlapArea,
Expand Down Expand Up @@ -270,11 +270,12 @@ class TapTargets extends Audit {

/**
* @param {LH.Artifacts} artifacts
* @return {LH.Audit.Product}
* @param {LH.Audit.Context} context
* @return {Promise<LH.Audit.Product>}
*/
static audit(artifacts) {
const hasViewportSet = ViewportAudit.audit(artifacts).rawValue;
if (!hasViewportSet) {
static async audit(artifacts, context) {
const viewportMeta = await ComputedViewportMeta.request(artifacts, context);
if (!viewportMeta.hasMobileViewport) {
return {
rawValue: false,
explanation: str_(UIStrings.explanationViewportMetaNotOptimized),
Expand Down
32 changes: 11 additions & 21 deletions lighthouse-core/audits/viewport.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
'use strict';

const Audit = require('./audit');
const Parser = require('metaviewport-parser');
const ComputedViewportMeta = require('../computed/viewport-meta.js');


class Viewport extends Audit {
/**
Expand All @@ -26,33 +27,22 @@ class Viewport extends Audit {

/**
* @param {LH.Artifacts} artifacts
* @return {LH.Audit.Product}
* @param {LH.Audit.Context} context
* @return {Promise<LH.Audit.Product>}
*/
static audit(artifacts) {
const viewportMeta = artifacts.MetaElements.find(meta => meta.name === 'viewport');
if (!viewportMeta) {
static async audit(artifacts, context) {
const viewportMeta = await ComputedViewportMeta.request(artifacts, context);

if (!viewportMeta.hasViewportTag) {
return {
explanation: 'No viewport meta tag found',
rawValue: false,
explanation: 'No viewport meta tag found',
};
}

const warnings = [];
const parsedProps = Parser.parseMetaViewPortContent(viewportMeta.content || '');

if (Object.keys(parsedProps.unknownProperties).length) {
warnings.push(`Invalid properties found: ${JSON.stringify(parsedProps.unknownProperties)}`);
}
if (Object.keys(parsedProps.invalidValues).length) {
warnings.push(`Invalid values found: ${JSON.stringify(parsedProps.invalidValues)}`);
}

const viewportProps = parsedProps.validProperties;
const hasMobileViewport = viewportProps.width || viewportProps['initial-scale'];

return {
rawValue: !!hasMobileViewport,
warnings,
rawValue: viewportMeta.hasMobileViewport,
warnings: viewportMeta.parserWarnings,
};
}
}
Expand Down
50 changes: 50 additions & 0 deletions lighthouse-core/computed/viewport-meta.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/**
* @license Copyright 2019 Google Inc. All Rights Reserved.
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
*/
'use strict';

const Parser = require('metaviewport-parser');

const makeComputedArtifact = require('./computed-artifact.js');

class ViewportMeta {
/**
* @param {{MetaElements: LH.GathererArtifacts['MetaElements']}} artifacts
* @return {Promise<LH.Artifacts.ViewportMeta>}
*/

mattzeunert marked this conversation as resolved.
Show resolved Hide resolved
static async compute_({MetaElements}) {
const viewportMeta = MetaElements.find(meta => meta.name === 'viewport');

if (!viewportMeta) {
return {
hasViewportTag: false,
hasMobileViewport: false,
parserWarnings: [],
};
}

const warnings = [];
const parsedProps = Parser.parseMetaViewPortContent(viewportMeta.content || '');

if (Object.keys(parsedProps.unknownProperties).length) {
warnings.push(`Invalid properties found: ${JSON.stringify(parsedProps.unknownProperties)}`);
}
if (Object.keys(parsedProps.invalidValues).length) {
warnings.push(`Invalid values found: ${JSON.stringify(parsedProps.invalidValues)}`);
}

const viewportProps = parsedProps.validProperties;
const hasMobileViewport = Boolean(viewportProps.width || viewportProps['initial-scale']);

return {
hasMobileViewport,
hasViewportTag: true,
mattzeunert marked this conversation as resolved.
Show resolved Hide resolved
parserWarnings: warnings,
};
}
}

module.exports = makeComputedArtifact(ViewportMeta);
37 changes: 19 additions & 18 deletions lighthouse-core/test/audits/seo/font-size-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,23 @@ const validViewport = 'width=device-width';

describe('SEO: Font size audit', () => {
const makeMetaElements = viewport => [{name: 'viewport', content: viewport}];
const fakeContext = {computedCache: new Map()};
mattzeunert marked this conversation as resolved.
Show resolved Hide resolved

it('fails when viewport is not set', () => {
it('fails when viewport is not set', async () => {
const artifacts = {
URL,
MetaElements: [],
FontSize: [],
};

const auditResult = FontSizeAudit.audit(artifacts);
const auditResult = await FontSizeAudit.audit(artifacts, fakeContext);
assert.equal(auditResult.rawValue, false);
expect(auditResult.explanation)
.toBeDisplayString('Text is illegible because there\'s ' +
'no viewport meta tag optimized for mobile screens.');
});

it('fails when less than 60% of text is legible', () => {
it('fails when less than 60% of text is legible', async () => {
const artifacts = {
URL,
MetaElements: makeMetaElements(validViewport),
Expand All @@ -46,13 +47,13 @@ describe('SEO: Font size audit', () => {
},
};

const auditResult = FontSizeAudit.audit(artifacts);
const auditResult = await FontSizeAudit.audit(artifacts, fakeContext);
assert.equal(auditResult.rawValue, false);
expect(auditResult.explanation).toBeDisplayString('41% of text is too small.');
expect(auditResult.displayValue).toBeDisplayString('59% legible text');
});

it('passes when there is no text', () => {
it('passes when there is no text', async () => {
const artifacts = {
URL,
MetaElements: makeMetaElements(validViewport),
Expand All @@ -67,11 +68,11 @@ describe('SEO: Font size audit', () => {
},
};

const auditResult = FontSizeAudit.audit(artifacts);
const auditResult = await FontSizeAudit.audit(artifacts, fakeContext);
assert.equal(auditResult.rawValue, true);
});

it('passes when more than 60% of text is legible', () => {
it('passes when more than 60% of text is legible', async () => {
const artifacts = {
URL,
MetaElements: makeMetaElements(validViewport),
Expand All @@ -86,12 +87,12 @@ describe('SEO: Font size audit', () => {
],
},
};
const auditResult = FontSizeAudit.audit(artifacts);
const auditResult = await FontSizeAudit.audit(artifacts, fakeContext);
assert.equal(auditResult.rawValue, true);
expect(auditResult.displayValue).toBeDisplayString('90% legible text');
});

it('groups entries with same source, sorts them by coverage', () => {
it('groups entries with same source, sorts them by coverage', async () => {
const style1 = {
styleSheetId: 1,
type: 'Regular',
Expand Down Expand Up @@ -123,15 +124,15 @@ describe('SEO: Font size audit', () => {
],
},
};
const auditResult = FontSizeAudit.audit(artifacts);
const auditResult = await FontSizeAudit.audit(artifacts, fakeContext);

assert.equal(auditResult.rawValue, false);
assert.equal(auditResult.details.items.length, 2);
assert.equal(auditResult.details.items[0].coverage, '57.14%');
expect(auditResult.displayValue).toBeDisplayString('0% legible text');
});

it('adds a category for failing text that wasn\'t analyzed', () => {
it('adds a category for failing text that wasn\'t analyzed', async () => {
const artifacts = {
URL,
MetaElements: makeMetaElements(validViewport),
Expand All @@ -145,15 +146,15 @@ describe('SEO: Font size audit', () => {
],
},
};
const auditResult = FontSizeAudit.audit(artifacts);
const auditResult = await FontSizeAudit.audit(artifacts, fakeContext);
assert.equal(auditResult.rawValue, false);
assert.equal(auditResult.details.items.length, 3);
assert.equal(auditResult.details.items[1].source, 'Add\'l illegible text');
assert.equal(auditResult.details.items[1].coverage, '40.00%');
expect(auditResult.displayValue).toBeDisplayString('50% legible text');
});

it('informs user if audit haven\'t covered all text on the page', () => {
it('informs user if audit haven\'t covered all text on the page', async () => {
const artifacts = {
URL,
MetaElements: makeMetaElements(validViewport),
Expand All @@ -167,14 +168,14 @@ describe('SEO: Font size audit', () => {
],
},
};
const auditResult = FontSizeAudit.audit(artifacts);
const auditResult = await FontSizeAudit.audit(artifacts, fakeContext);
assert.equal(auditResult.rawValue, false);
expect(auditResult.explanation)
.toBeDisplayString('100% of text is too small (based on 50% sample).');
expect(auditResult.displayValue).toBeDisplayString('0% legible text');
});

it('maintains 2 trailing decimal places', () => {
it('maintains 2 trailing decimal places', async () => {
const artifacts = {
URL,
MetaElements: makeMetaElements(validViewport),
Expand All @@ -189,11 +190,11 @@ describe('SEO: Font size audit', () => {
],
},
};
const auditResult = FontSizeAudit.audit(artifacts);
const auditResult = await FontSizeAudit.audit(artifacts, fakeContext);
expect(auditResult.displayValue).toBeDisplayString('89.78% legible text');
});

it('maintains 2 trailing decimal places with only 1 leading digit', () => {
it('maintains 2 trailing decimal places with only 1 leading digit', async () => {
const artifacts = {
URL,
MetaElements: makeMetaElements(validViewport),
Expand All @@ -208,7 +209,7 @@ describe('SEO: Font size audit', () => {
],
},
};
const auditResult = FontSizeAudit.audit(artifacts);
const auditResult = await FontSizeAudit.audit(artifacts, fakeContext);
expect(auditResult.displayValue).toBeDisplayString('2.48% legible text');
});
});
Loading