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

report(pwa): give badges to groups with all passing audits #6504

Merged
merged 1 commit into from
Nov 8, 2018
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: 1 addition & 1 deletion lighthouse-core/config/default-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ const defaultConfig = {
{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'},
// Engaging
// PWA Optimized
{id: 'redirects-http', weight: 2, group: 'pwa-optimized'},
{id: 'splash-screen', weight: 1, group: 'pwa-optimized'},
{id: 'themed-omnibox', weight: 1, group: 'pwa-optimized'},
Expand Down
43 changes: 41 additions & 2 deletions lighthouse-core/report/html/renderer/pwa-category-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
*/
'use strict';

/* globals self, CategoryRenderer */
/* globals self, Util, CategoryRenderer */

class PwaCategoryRenderer extends CategoryRenderer {
/**
Expand All @@ -34,7 +34,7 @@ class PwaCategoryRenderer extends CategoryRenderer {
// Regular audits aren't split up into pass/fail/not-applicable clumps, they're
// all put in a top-level clump that isn't expandable/collapsable.
const regularAuditRefs = auditRefs.filter(ref => ref.result.scoreDisplayMode !== 'manual');
const auditsElem = this.renderUnexpandableClump(regularAuditRefs, groupDefinitions);
const auditsElem = this._renderAudits(regularAuditRefs, groupDefinitions);
categoryElem.appendChild(auditsElem);

// Manual audits are still in a manual clump.
Expand All @@ -45,6 +45,45 @@ class PwaCategoryRenderer extends CategoryRenderer {

return categoryElem;
}

/**
* Returns the group IDs whose audits are all considered passing.
* @param {Array<LH.ReportResult.AuditRef>} auditRefs
* @return {Set<string>}
*/
_getPassingGroupIds(auditRefs) {
const groupIds = auditRefs.map(ref => ref.group).filter(/** @return {g is string} */ g => !!g);
const uniqueGroupIds = new Set(groupIds);

// Remove any that have a failing audit.
for (const auditRef of auditRefs) {
if (!Util.showAsPassed(auditRef.result) && auditRef.group) {
uniqueGroupIds.delete(auditRef.group);
}
}

return uniqueGroupIds;
}

/**
* Render non-manual audits in groups, giving a badge to any group that has
* all passing audits.
* @param {Array<LH.ReportResult.AuditRef>} auditRefs
* @param {Object<string, LH.Result.ReportGroup>} groupDefinitions
* @return {Element}
*/
_renderAudits(auditRefs, groupDefinitions) {
const auditsElem = this.renderUnexpandableClump(auditRefs, groupDefinitions);

// Add a 'badged' class to group if all audits in that group pass.
const passsingGroupIds = this._getPassingGroupIds(auditRefs);
for (const groupId of passsingGroupIds) {
const groupElem = this.dom.find(`.lh-audit-group--${groupId}`, auditsElem);
groupElem.classList.add('lh-badged');
}

return auditsElem;
}
}

if (typeof module !== 'undefined' && module.exports) {
Expand Down
13 changes: 13 additions & 0 deletions lighthouse-core/report/html/report-styles.css
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,10 @@
--pwa-fast-reliable-gray-url: url('data:image/svg+xml;utf8,<svg xmlns="http://www.w3.org/2000/svg"><g fill="none" fill-rule="nonzero"><circle fill="#DAE0E3" cx="12" cy="12" r="12"/><path d="M12.3 4l6.3 2.8V11c0 3.88-2.69 7.52-6.3 8.4C8.69 18.52 6 14.89 6 11V6.8L12.3 4zm-.56 12.88l3.3-5.79.04-.08c.05-.1.01-.29-.26-.29h-1.96l.56-3.92h-.56L9.6 12.52c0 .03.07-.12-.03.07-.11.2-.12.37.2.37h1.97l-.56 3.92h.56z" fill="#FFF"/></g></svg>');
--pwa-installable-gray-url: url('data:image/svg+xml;utf8,<svg xmlns="http://www.w3.org/2000/svg"><g fill="none" fill-rule="nonzero"><circle fill="#DAE0E3" cx="12" cy="12" r="12"/><path d="M12 5a7 7 0 1 0 0 14 7 7 0 0 0 0-14zm3.5 7.7h-2.8v2.8h-1.4v-2.8H8.5v-1.4h2.8V8.5h1.4v2.8h2.8v1.4z" fill="#FFF"/></g></svg>');
--pwa-optimized-gray-url: url('data:image/svg+xml;utf8,<svg xmlns="http://www.w3.org/2000/svg"><g fill="none" fill-rule="evenodd"><rect fill="#DAE0E3" width="24" height="24" rx="12"/><path fill="#FFF" d="M12 15.07l3.6 2.18-.95-4.1 3.18-2.76-4.2-.36L12 6.17l-1.64 3.86-4.2.36 3.2 2.76-.96 4.1z"/><path d="M5 5h14v14H5z"/></g></svg>');

--pwa-fast-reliable-color-url: url('data:image/svg+xml;utf8,<svg xmlns="http://www.w3.org/2000/svg"><g fill-rule="nonzero" fill="none"><circle fill="#CCE3F6" cx="12" cy="12" r="12"/><path d="M12 4.3l6.3 2.8v4.2c0 3.88-2.69 7.52-6.3 8.4-3.61-.88-6.3-4.51-6.3-8.4V7.1L12 4.3zm-.56 12.88l3.3-5.79.04-.08c.05-.1.01-.29-.26-.29h-1.96l.56-3.92h-.56L9.3 12.82c0 .03.07-.12-.03.07-.11.2-.12.37.2.37h1.97l-.56 3.92h.56z" fill="#304FFE"/></g></svg>');
--pwa-installable-color-url: url('data:image/svg+xml;utf8,<svg xmlns="http://www.w3.org/2000/svg"><g fill-rule="nonzero" fill="none"><circle fill="#D4ECD5" cx="12" cy="12" r="12"/><path d="M12 5a7 7 0 1 0 0 14 7 7 0 0 0 0-14zm3.5 7.7h-2.8v2.8h-1.4v-2.8H8.5v-1.4h2.8V8.5h1.4v2.8h2.8v1.4z" fill="#009688"/></g></svg>');
--pwa-optimized-color-url: url('data:image/svg+xml;utf8,<svg xmlns="http://www.w3.org/2000/svg"><g fill="none" fill-rule="evenodd"><rect fill="#FCE4EC" width="24" height="24" rx="12"/><path d="M5 5h14v14H5z"/><path fill="#EC3F7A" d="M12 15.07l3.6 2.18-.95-4.1 3.18-2.76-4.2-.36L12 6.17l-1.64 3.86-4.2.36 3.2 2.76-.96 4.1z"/></g></svg>');
}

.lh-vars.lh-devtools {
Expand Down Expand Up @@ -636,6 +640,15 @@ details, summary {
background-image: var(--pwa-optimized-gray-url);
background-size: var(--lh-group-icon-background-size);
}
.lh-audit-group--pwa-fast-reliable.lh-badged .lh-audit-group__header::before {
background-image: var(--pwa-fast-reliable-color-url);
}
.lh-audit-group--pwa-installable.lh-badged .lh-audit-group__header::before {
background-image: var(--pwa-installable-color-url);
}
.lh-audit-group--pwa-optimized.lh-badged .lh-audit-group__header::before {
background-image: var(--pwa-optimized-color-url);
}

/* Removing too much whitespace */
.lh-audit-group--metrics {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,12 @@ describe('PwaCategoryRenderer', () => {
pwaRenderer = new PwaCategoryRenderer(dom, detailsRenderer);

sampleResults = Util.prepareReportResult(sampleResultsOrig);
category = sampleResults.reportCategories.find(cat => cat.id === 'pwa');
});

beforeEach(() => {
// Clone category to allow modifications.
const pwaCategory = sampleResults.reportCategories.find(cat => cat.id === 'pwa');
category = JSON.parse(JSON.stringify(pwaCategory));
});

afterAll(() => {
Expand Down Expand Up @@ -90,4 +95,82 @@ describe('PwaCategoryRenderer', () => {
`trouble with selector '${selector}'`);
});
});

describe('badging groups', () => {
let auditRefs;
let groupIds;

beforeEach(() => {
auditRefs = category.auditRefs
.filter(audit => audit.result.scoreDisplayMode !== 'manual');

// Expect results to all be scorable.
for (const auditRef of auditRefs) {
assert.strictEqual(auditRef.result.scoreDisplayMode, 'binary');
}

groupIds = [...new Set(auditRefs.map(ref => ref.group))];
});

it('only gives a group a badge when all the group\'s audits are passing', () => {
for (const auditRef of auditRefs) {
auditRef.result.score = 0;
}

const targetGroupId = groupIds[2];
assert.ok(targetGroupId);
const targetAuditRefs = auditRefs.filter(ref => ref.group === targetGroupId);

// Try every permutation of audit scoring.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems a bit excessive 😆 but respect 🙇

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems a bit excessive

haha, yes. I was hemming and hawing on which cases to include and said "screw it, we'll do all of them" :)

const totalPermutations = Math.pow(2, targetAuditRefs.length);
for (let i = 0; i < totalPermutations; i++) {
for (let j = 0; j < targetAuditRefs.length; j++) {
// Set as passing if jth bit in i is set.
targetAuditRefs[j].result.score = i >> j & 1;
}

const categoryElem = pwaRenderer.render(category, sampleResults.categoryGroups);
const badgedElems = categoryElem.querySelectorAll(`.lh-badged`);

// Only expect a badge on last permutation (all bits are set).
const expectedBadgeCount = i === totalPermutations - 1 ? 1 : 0;
assert.strictEqual(badgedElems.length, expectedBadgeCount);
}
});

it('renders all badges when all audits are passing', () => {
for (const auditRef of auditRefs) {
auditRef.result.score = 1;
}

const categoryElem = pwaRenderer.render(category, sampleResults.categoryGroups);
assert.strictEqual(categoryElem.querySelectorAll('.lh-badged').length, groupIds.length);
});

it('renders no badges when no audit groups are passing', () => {
for (const auditRef of auditRefs) {
auditRef.result.score = 0;
}

const categoryElem = pwaRenderer.render(category, sampleResults.categoryGroups);
assert.strictEqual(categoryElem.querySelectorAll('.lh-badged').length, 0);
});

it('renders all but one badge when all groups but one are passing', () => {
for (const auditRef of auditRefs) {
auditRef.result.score = 1;
}
auditRefs[0].result.score = 0;
const failingGroupId = auditRefs[0].group;

const categoryElem = pwaRenderer.render(category, sampleResults.categoryGroups);

for (const groupId of groupIds) {
const expectedCount = groupId === failingGroupId ? 0 : 1;

const groupElems = categoryElem.querySelectorAll(`.lh-audit-group--${groupId}.lh-badged`);
assert.strictEqual(groupElems.length, expectedCount);
}
});
});
});
15 changes: 4 additions & 11 deletions lighthouse-core/test/report/html/renderer/report-renderer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@ const DOM = require('../../../../report/html/renderer/dom.js');
const DetailsRenderer = require('../../../../report/html/renderer/details-renderer.js');
const ReportUIFeatures = require('../../../../report/html/renderer/report-ui-features.js');
const CategoryRenderer = require('../../../../report/html/renderer/category-renderer.js');
// lazy loaded because they depend on CategoryRenderer to be available globally
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these last two files cleaned up based on @patrickhulce feedback in #6486 (comment)

let PerformanceCategoryRenderer = null;
let PwaCategoryRenderer = null;
const CriticalRequestChainRenderer = require(
'../../../../report/html/renderer/crc-details-renderer.js');
const ReportRenderer = require('../../../../report/html/renderer/report-renderer.js');
Expand All @@ -39,16 +36,12 @@ describe('ReportRenderer', () => {
global.CriticalRequestChainRenderer = CriticalRequestChainRenderer;
global.DetailsRenderer = DetailsRenderer;
global.CategoryRenderer = CategoryRenderer;
if (!PerformanceCategoryRenderer) {
PerformanceCategoryRenderer =

// lazy loaded because they depend on CategoryRenderer to be available globally
global.PerformanceCategoryRenderer =
require('../../../../report/html/renderer/performance-category-renderer.js');
}
global.PerformanceCategoryRenderer = PerformanceCategoryRenderer;
if (!PwaCategoryRenderer) {
PwaCategoryRenderer =
global.PwaCategoryRenderer =
require('../../../../report/html/renderer/pwa-category-renderer.js');
}
global.PwaCategoryRenderer = PwaCategoryRenderer;

// Stub out matchMedia for Node.
global.matchMedia = function() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@ const DOM = require('../../../../report/html/renderer/dom.js');
const DetailsRenderer = require('../../../../report/html/renderer/details-renderer.js');
const ReportUIFeatures = require('../../../../report/html/renderer/report-ui-features.js');
const CategoryRenderer = require('../../../../report/html/renderer/category-renderer.js');
// lazy loaded because they depend on CategoryRenderer to be available globally
let PerformanceCategoryRenderer = null;
let PwaCategoryRenderer = null;
const CriticalRequestChainRenderer = require(
'../../../../report/html/renderer/crc-details-renderer.js');
const ReportRenderer = require('../../../../report/html/renderer/report-renderer.js');
Expand All @@ -41,16 +38,12 @@ describe('ReportUIFeatures', () => {
global.CriticalRequestChainRenderer = CriticalRequestChainRenderer;
global.DetailsRenderer = DetailsRenderer;
global.CategoryRenderer = CategoryRenderer;
if (!PerformanceCategoryRenderer) {
PerformanceCategoryRenderer =

// lazy loaded because they depend on CategoryRenderer to be available globally
global.PerformanceCategoryRenderer =
require('../../../../report/html/renderer/performance-category-renderer.js');
}
global.PerformanceCategoryRenderer = PerformanceCategoryRenderer;
if (!PwaCategoryRenderer) {
PwaCategoryRenderer =
global.PwaCategoryRenderer =
require('../../../../report/html/renderer/pwa-category-renderer.js');
}
global.PwaCategoryRenderer = PwaCategoryRenderer;

// Stub out matchMedia for Node.
global.matchMedia = function() {
Expand Down