From 662ca552c456d4d41296f15b96aa90b66428bfc0 Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Thu, 3 May 2018 14:45:56 -0700 Subject: [PATCH] core(category): add manualDescription (#5100) --- lighthouse-core/config/config.js | 19 +++--- lighthouse-core/config/default-config.js | 52 +++++++--------- .../report/html/renderer/category-renderer.js | 21 +++---- .../report/html/renderer/report-renderer.js | 1 + lighthouse-core/test/results/sample_v2.json | 60 ++++++------------- typings/lhr.d.ts | 2 + 6 files changed, 61 insertions(+), 94 deletions(-) diff --git a/lighthouse-core/config/config.js b/lighthouse-core/config/config.js index b25233db8254..1f74f00970c5 100644 --- a/lighthouse-core/config/config.js +++ b/lighthouse-core/config/config.js @@ -51,23 +51,24 @@ function validateCategories(categories, audits, groups) { return; } - const auditIds = audits.map(audit => audit.implementation.meta.name); Object.keys(categories).forEach(categoryId => { - categories[categoryId].audits.forEach((audit, index) => { - if (!audit.id) { + categories[categoryId].audits.forEach((auditRef, index) => { + if (!auditRef.id) { throw new Error(`missing an audit id at ${categoryId}[${index}]`); } - if (!auditIds.includes(audit.id)) { - throw new Error(`could not find ${audit.id} audit for category ${categoryId}`); + const audit = audits.find(a => a.implementation.meta.name === auditRef.id); + if (!audit) { + throw new Error(`could not find ${auditRef.id} audit for category ${categoryId}`); } - if (categoryId === 'accessibility' && !audit.group) { - throw new Error(`${audit.id} accessibility audit does not have a group`); + const auditImpl = audit.implementation; + if (categoryId === 'accessibility' && !auditRef.group && !auditImpl.meta.manual) { + throw new Error(`${auditRef.id} accessibility audit does not have a group`); } - if (audit.group && !groups[audit.group]) { - throw new Error(`${audit.id} references unknown group ${audit.group}`); + if (auditRef.group && !groups[auditRef.group]) { + throw new Error(`${auditRef.id} references unknown group ${auditRef.group}`); } }); }); diff --git a/lighthouse-core/config/default-config.js b/lighthouse-core/config/default-config.js index 649920c65542..af641459d462 100644 --- a/lighthouse-core/config/default-config.js +++ b/lighthouse-core/config/default-config.js @@ -232,16 +232,6 @@ module.exports = { title: 'Meta Tags Used Properly', description: 'These are opportunities to improve the user experience of your site.', }, - 'manual-a11y-checks': { - title: 'Additional items to manually check', - description: 'These items address areas which an automated testing tool cannot cover. Learn more in our guide on [conducting an accessibility review](https://developers.google.com/web/fundamentals/accessibility/how-to-review).', - }, - 'manual-pwa-checks': { - title: 'Additional items to manually check', - description: 'These checks are required by the baseline ' + - '[PWA Checklist](https://developers.google.com/web/progressive-web-apps/checklist) but are ' + - 'not automatically checked by Lighthouse. They do not affect your score but it\'s important that you verify them manually.', - }, 'seo-mobile': { title: 'Mobile Friendly', description: 'Make sure your pages are mobile friendly so users don’t have to pinch or zoom ' + @@ -255,10 +245,6 @@ module.exports = { title: 'Crawling and Indexing', description: 'To appear in search results, crawlers need access to your app.', }, - 'manual-seo-checks': { - title: 'Additional items to manually check', - description: 'Run these additional validators on your site to check additional SEO best practices.', - }, }, categories: { 'performance': { @@ -302,6 +288,9 @@ module.exports = { 'pwa': { name: 'Progressive Web App', description: 'These checks validate the aspects of a Progressive Web App, as specified by the baseline [PWA Checklist](https://developers.google.com/web/progressive-web-apps/checklist).', + manualDescription: 'These checks are required by the baseline ' + + '[PWA Checklist](https://developers.google.com/web/progressive-web-apps/checklist) but are ' + + 'not automatically checked by Lighthouse. They do not affect your score but it\'s important that you verify them manually.', audits: [ {id: 'service-worker', weight: 1}, {id: 'works-offline', weight: 1}, @@ -314,14 +303,16 @@ module.exports = { {id: 'themed-omnibox', weight: 1}, {id: 'viewport', weight: 1}, {id: 'content-width', weight: 1}, - {id: 'pwa-cross-browser', weight: 0, group: 'manual-pwa-checks'}, - {id: 'pwa-page-transitions', weight: 0, group: 'manual-pwa-checks'}, - {id: 'pwa-each-page-has-url', weight: 0, group: 'manual-pwa-checks'}, + // Manual audits + {id: 'pwa-cross-browser', weight: 0}, + {id: 'pwa-page-transitions', weight: 0}, + {id: 'pwa-each-page-has-url', weight: 0}, ], }, 'accessibility': { name: 'Accessibility', description: 'These checks highlight opportunities to [improve the accessibility of your web app](https://developers.google.com/web/fundamentals/accessibility). Only a subset of accessibility issues can be automatically detected so manual testing is also encouraged.', + manualDescription: 'These items address areas which an automated testing tool cannot cover. Learn more in our guide on [conducting an accessibility review](https://developers.google.com/web/fundamentals/accessibility/how-to-review).', audits: [ {id: 'accesskeys', weight: 1, group: 'a11y-correct-attributes'}, {id: 'aria-allowed-attr', weight: 3, group: 'a11y-aria'}, @@ -358,16 +349,17 @@ module.exports = { {id: 'valid-lang', weight: 1, group: 'a11y-language'}, {id: 'video-caption', weight: 4, group: 'a11y-describe-contents'}, {id: 'video-description', weight: 3, group: 'a11y-describe-contents'}, - {id: 'logical-tab-order', weight: 0, group: 'manual-a11y-checks'}, - {id: 'focusable-controls', weight: 0, group: 'manual-a11y-checks'}, - {id: 'managed-focus', weight: 0, group: 'manual-a11y-checks'}, - {id: 'focus-traps', weight: 0, group: 'manual-a11y-checks'}, - {id: 'custom-controls-labels', weight: 0, group: 'manual-a11y-checks'}, - {id: 'custom-controls-roles', weight: 0, group: 'manual-a11y-checks'}, - {id: 'visual-order-follows-dom', weight: 0, group: 'manual-a11y-checks'}, - {id: 'offscreen-content-hidden', weight: 0, group: 'manual-a11y-checks'}, - {id: 'heading-levels', weight: 0, group: 'manual-a11y-checks'}, - {id: 'use-landmarks', weight: 0, group: 'manual-a11y-checks'}, + // Manual audits + {id: 'logical-tab-order', weight: 0}, + {id: 'focusable-controls', weight: 0}, + {id: 'managed-focus', weight: 0}, + {id: 'focus-traps', weight: 0}, + {id: 'custom-controls-labels', weight: 0}, + {id: 'custom-controls-roles', weight: 0}, + {id: 'visual-order-follows-dom', weight: 0}, + {id: 'offscreen-content-hidden', weight: 0}, + {id: 'heading-levels', weight: 0}, + {id: 'use-landmarks', weight: 0}, ], }, 'best-practices': { @@ -397,6 +389,7 @@ module.exports = { description: 'These checks ensure that your page is optimized for search engine results ranking. ' + 'There are additional factors Lighthouse does not check that may affect your search ranking. ' + '[Learn more](https://support.google.com/webmasters/answer/35769).', + manualDescription: 'Run these additional validators on your site to check additional SEO best practices.', audits: [ {id: 'viewport', weight: 1, group: 'seo-mobile'}, {id: 'document-title', weight: 1, group: 'seo-content'}, @@ -409,8 +402,9 @@ module.exports = { {id: 'canonical', weight: 1, group: 'seo-content'}, {id: 'font-size', weight: 1, group: 'seo-mobile'}, {id: 'plugins', weight: 1, group: 'seo-content'}, - {id: 'mobile-friendly', weight: 0, group: 'manual-seo-checks'}, - {id: 'structured-data', weight: 0, group: 'manual-seo-checks'}, + // Manual audits + {id: 'mobile-friendly', weight: 0}, + {id: 'structured-data', weight: 0}, ], }, }, diff --git a/lighthouse-core/report/html/renderer/category-renderer.js b/lighthouse-core/report/html/renderer/category-renderer.js index 9bd35754aa40..c4de249b17f0 100644 --- a/lighthouse-core/report/html/renderer/category-renderer.js +++ b/lighthouse-core/report/html/renderer/category-renderer.js @@ -196,21 +196,14 @@ class CategoryRenderer { /** * @param {!Array} manualAudits - * @param {!Object} groupDefinitions + * @param {string} manualDescription * @param {!Element} element Parent container to add the manual audits to. */ - _renderManualAudits(manualAudits, groupDefinitions, element) { - // While we could support rendering multiple groups of manual audits, it doesn't - // seem desirable for UX or renderer complexity. So we'll throw. - const groupsIds = new Set(manualAudits.map(a => a.group)); - /* eslint-disable no-console */ - console.assert(groupsIds.size <= 1, 'More than 1 manual audit group found.'); - console.assert(!groupsIds.has(undefined), 'Some manual audits don\'t belong to a group'); - /* eslint-enable no-console */ - if (!groupsIds.size) return; - - const groupId = /** @type {string} */ (Array.from(groupsIds)[0]); - const auditGroupElem = this.renderAuditGroup(groupDefinitions[groupId], {expandable: true}); + _renderManualAudits(manualAudits, manualDescription, element) { + if (!manualAudits.length) return; + + const group = {title: 'Additional items to manually check', description: manualDescription}; + const auditGroupElem = this.renderAuditGroup(group, {expandable: true}); auditGroupElem.classList.add('lh-audit-group--manual'); manualAudits.forEach(audit => { @@ -354,7 +347,7 @@ class CategoryRenderer { } // Render manual audits after passing. - this._renderManualAudits(manualAudits, groupDefinitions, element); + this._renderManualAudits(manualAudits, category.manualDescription, element); return element; } diff --git a/lighthouse-core/report/html/renderer/report-renderer.js b/lighthouse-core/report/html/renderer/report-renderer.js index e92723bc475a..ef233f429d7c 100644 --- a/lighthouse-core/report/html/renderer/report-renderer.js +++ b/lighthouse-core/report/html/renderer/report-renderer.js @@ -239,6 +239,7 @@ ReportRenderer.AuditJSON; // eslint-disable-line no-unused-expressions * id: string, * score: number, * description: string, + * manualDescription: string, * audits: !Array * }} */ diff --git a/lighthouse-core/test/results/sample_v2.json b/lighthouse-core/test/results/sample_v2.json index 063967abb8d8..6b26d02fe8a7 100644 --- a/lighthouse-core/test/results/sample_v2.json +++ b/lighthouse-core/test/results/sample_v2.json @@ -4728,6 +4728,7 @@ { "name": "Progressive Web App", "description": "These checks validate the aspects of a Progressive Web App, as specified by the baseline [PWA Checklist](https://developers.google.com/web/progressive-web-apps/checklist).", + "manualDescription": "These checks are required by the baseline [PWA Checklist](https://developers.google.com/web/progressive-web-apps/checklist) but are not automatically checked by Lighthouse. They do not affect your score but it's important that you verify them manually.", "audits": [ { "id": "service-worker", @@ -4775,18 +4776,15 @@ }, { "id": "pwa-cross-browser", - "weight": 0, - "group": "manual-pwa-checks" + "weight": 0 }, { "id": "pwa-page-transitions", - "weight": 0, - "group": "manual-pwa-checks" + "weight": 0 }, { "id": "pwa-each-page-has-url", - "weight": 0, - "group": "manual-pwa-checks" + "weight": 0 } ], "id": "pwa", @@ -4795,6 +4793,7 @@ { "name": "Accessibility", "description": "These checks highlight opportunities to [improve the accessibility of your web app](https://developers.google.com/web/fundamentals/accessibility). Only a subset of accessibility issues can be automatically detected so manual testing is also encouraged.", + "manualDescription": "These items address areas which an automated testing tool cannot cover. Learn more in our guide on [conducting an accessibility review](https://developers.google.com/web/fundamentals/accessibility/how-to-review).", "audits": [ { "id": "accesskeys", @@ -4973,53 +4972,43 @@ }, { "id": "logical-tab-order", - "weight": 0, - "group": "manual-a11y-checks" + "weight": 0 }, { "id": "focusable-controls", - "weight": 0, - "group": "manual-a11y-checks" + "weight": 0 }, { "id": "managed-focus", - "weight": 0, - "group": "manual-a11y-checks" + "weight": 0 }, { "id": "focus-traps", - "weight": 0, - "group": "manual-a11y-checks" + "weight": 0 }, { "id": "custom-controls-labels", - "weight": 0, - "group": "manual-a11y-checks" + "weight": 0 }, { "id": "custom-controls-roles", - "weight": 0, - "group": "manual-a11y-checks" + "weight": 0 }, { "id": "visual-order-follows-dom", - "weight": 0, - "group": "manual-a11y-checks" + "weight": 0 }, { "id": "offscreen-content-hidden", - "weight": 0, - "group": "manual-a11y-checks" + "weight": 0 }, { "id": "heading-levels", - "weight": 0, - "group": "manual-a11y-checks" + "weight": 0 }, { "id": "use-landmarks", - "weight": 0, - "group": "manual-a11y-checks" + "weight": 0 } ], "id": "accessibility", @@ -5100,6 +5089,7 @@ { "name": "SEO", "description": "These checks ensure that your page is optimized for search engine results ranking. There are additional factors Lighthouse does not check that may affect your search ranking. [Learn more](https://support.google.com/webmasters/answer/35769).", + "manualDescription": "Run these additional validators on your site to check additional SEO best practices.", "audits": [ { "id": "viewport", @@ -5158,13 +5148,11 @@ }, { "id": "mobile-friendly", - "weight": 0, - "group": "manual-seo-checks" + "weight": 0 }, { "id": "structured-data", - "weight": 0, - "group": "manual-seo-checks" + "weight": 0 } ], "id": "seo", @@ -5216,14 +5204,6 @@ "title": "Meta Tags Used Properly", "description": "These are opportunities to improve the user experience of your site." }, - "manual-a11y-checks": { - "title": "Additional items to manually check", - "description": "These items address areas which an automated testing tool cannot cover. Learn more in our guide on [conducting an accessibility review](https://developers.google.com/web/fundamentals/accessibility/how-to-review)." - }, - "manual-pwa-checks": { - "title": "Additional items to manually check", - "description": "These checks are required by the baseline [PWA Checklist](https://developers.google.com/web/progressive-web-apps/checklist) but are not automatically checked by Lighthouse. They do not affect your score but it's important that you verify them manually." - }, "seo-mobile": { "title": "Mobile Friendly", "description": "Make sure your pages are mobile friendly so users don’t have to pinch or zoom in order to read the content pages. [Learn more](https://developers.google.com/search/mobile-sites/)." @@ -5235,10 +5215,6 @@ "seo-crawl": { "title": "Crawling and Indexing", "description": "To appear in search results, crawlers need access to your app." - }, - "manual-seo-checks": { - "title": "Additional items to manually check", - "description": "Run these additional validators on your site to check additional SEO best practices." } } } \ No newline at end of file diff --git a/typings/lhr.d.ts b/typings/lhr.d.ts index 8d8ef7b04684..986f65f3779d 100644 --- a/typings/lhr.d.ts +++ b/typings/lhr.d.ts @@ -46,6 +46,8 @@ declare global { name: string; /** A more detailed description of the category and its importance. */ description: string; + /** A description for the manual audits in the category. */ + manualDescription?: string; /** The overall score of the category, the weighted average of all its audits. */ score: number; /** An array of references to all the audit members of this category. */