-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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: map metrics to audits #11732
Changes from 5 commits
faee95c
3bb719a
ed3d2c9
86db7ec
cd7a638
b3d72b0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
/** | ||
* @license Copyright 2021 The Lighthouse Authors. 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'; | ||
|
||
// go/lh-audit-metric-mapping | ||
const fcpRelevantAudits = [ | ||
'server-response-time', | ||
'render-blocking-resources', | ||
'redirects', | ||
'critical-request-chains', | ||
'uses-text-compression', | ||
'uses-rel-preconnect', | ||
'uses-rel-preload', | ||
'font-display', | ||
'unminified-javascript', | ||
'unminified-css', | ||
'unused-css-rules', | ||
]; | ||
|
||
const lcpRelevantAudits = [ | ||
...fcpRelevantAudits, | ||
'largest-contentful-paint-element', | ||
'preload-lcp-image', | ||
'unused-javascript', | ||
'efficient-animated-content', | ||
'total-byte-weight', | ||
]; | ||
|
||
const tbtRelevantAudits = [ | ||
'long-tasks', | ||
'third-party-summary', | ||
'third-party-facades', | ||
'bootup-time', | ||
'mainthread-work-breakdown', | ||
'dom-size', | ||
'duplicated-javascript', | ||
'legacy-javascript', | ||
]; | ||
|
||
const clsRelevantAudits = [ | ||
'layout-shift-elements', | ||
'non-composited-animations', | ||
'unsized-images', | ||
// 'preload-fonts', // actually in BP, rather than perf | ||
]; | ||
|
||
module.exports = { | ||
fcpRelevantAudits, | ||
lcpRelevantAudits, | ||
tbtRelevantAudits, | ||
clsRelevantAudits, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -118,18 +118,6 @@ class PerformanceCategoryRenderer extends CategoryRenderer { | |
if (fci) v5andv6metrics.push(fci); | ||
if (fmp) v5andv6metrics.push(fmp); | ||
|
||
/** @type {Record<string, string>} */ | ||
const acronymMapping = { | ||
'cumulative-layout-shift': 'CLS', | ||
'first-contentful-paint': 'FCP', | ||
'first-cpu-idle': 'FCI', | ||
'first-meaningful-paint': 'FMP', | ||
'interactive': 'TTI', | ||
'largest-contentful-paint': 'LCP', | ||
'speed-index': 'SI', | ||
'total-blocking-time': 'TBT', | ||
}; | ||
|
||
/** | ||
* Clamp figure to 2 decimal places | ||
* @param {number} val | ||
|
@@ -147,7 +135,7 @@ class PerformanceCategoryRenderer extends CategoryRenderer { | |
} else { | ||
value = 'null'; | ||
} | ||
return [acronymMapping[audit.id] || audit.id, value]; | ||
return [audit.acronym || audit.id, value]; | ||
}); | ||
const paramPairs = [...metricPairs]; | ||
|
||
|
@@ -225,6 +213,13 @@ class PerformanceCategoryRenderer extends CategoryRenderer { | |
.filter(audit => audit.group === 'load-opportunities' && !Util.showAsPassed(audit.result)) | ||
.sort((auditA, auditB) => this._getWastedMs(auditB) - this._getWastedMs(auditA)); | ||
|
||
|
||
const filterableMetrics = metricAudits.filter(a => !!a.relevantAudits); | ||
// TODO: only add if there are opportunities & diagnostics rendered. | ||
if (filterableMetrics.length) { | ||
this.renderMetricAuditFilter(filterableMetrics, element); | ||
} | ||
|
||
if (opportunityAudits.length) { | ||
// Scale the sparklines relative to savings, minimum 2s to not overstate small savings | ||
const minimumScale = 2000; | ||
|
@@ -299,6 +294,74 @@ class PerformanceCategoryRenderer extends CategoryRenderer { | |
|
||
return element; | ||
} | ||
|
||
/** | ||
* Render the control to filter the audits by metric. The filtering is done at runtime by CSS only | ||
* @param {LH.ReportResult.AuditRef[]} filterableMetrics | ||
* @param {HTMLDivElement} categoryEl | ||
*/ | ||
renderMetricAuditFilter(filterableMetrics, categoryEl) { | ||
// thx https://codepen.io/surjithctly/pen/weEJvX | ||
const metricFilterEl = this.dom.createElement('div', 'lh-metricfilter'); | ||
const textEl = this.dom.createChildOf(metricFilterEl, 'span', 'lh-metricfilter__text'); | ||
textEl.textContent = 'Show audits relevant to: '; | ||
const labelSelectors = []; | ||
const auditSelectors = []; | ||
|
||
const filterChoices = /** @type {LH.ReportResult.AuditRef[]} */ ([ | ||
({acronym: 'All'}), | ||
...filterableMetrics, | ||
]); | ||
for (const metric of filterChoices) { | ||
// The radio elements are appended into `categoryEl` to allow the sweet ~ selectors to work | ||
const elemId = `metric-${metric.acronym}`; | ||
const radioEl = this.dom.createChildOf(categoryEl, 'input', 'lh-metricfilter__radio', { | ||
type: 'radio', | ||
name: 'metricsfilter', | ||
id: elemId, | ||
hidden: 'true', | ||
}); | ||
const labelEl = this.dom.createChildOf(metricFilterEl, 'label', 'lh-metricfilter__label', { | ||
for: elemId, | ||
title: metric.result && metric.result.title, | ||
}); | ||
labelEl.textContent = metric.acronym || metric.id; | ||
if (metric.acronym === 'All') { | ||
radioEl.checked = true; | ||
} | ||
// Dynamically write some CSS, for the CSS-only filtering | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. all this dynamic CSS seems much harder to follow than the equivalent JS would be. What reasons did you rule out JS? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO the CSS's declarative state communicates all the changes pretty clearly. |
||
labelSelectors.push(`.lh-metricfilter__radio#${elemId}:checked ~ .lh-metricfilter > .lh-metricfilter__label[for="${elemId}"]`); // eslint-disable-line max-len | ||
if (metric.relevantAudits) { | ||
/* Generate some CSS selectors like this: | ||
#metric-CLS:checked ~ .lh-audit-group > #layout-shift-elements, | ||
#metric-CLS:checked ~ .lh-audit-group > #non-composited-animations, | ||
#metric-CLS:checked ~ .lh-audit-group > #unsized-images | ||
*/ | ||
auditSelectors.push(metric.relevantAudits.map(auditId => `#${elemId}:checked ~ .lh-audit-group > #${auditId}`).join(',\n')); // eslint-disable-line max-len | ||
} | ||
} | ||
|
||
const styleEl = this.dom.createChildOf(metricFilterEl, 'style'); | ||
// eslint-disable-next-line max-len | ||
styleEl.textContent = ` | ||
${labelSelectors.join(',\n')} { | ||
background: var(--color-blue-A700); | ||
color: var(--color-white); | ||
} | ||
/* If selecting non-All, hide all audits (and also the group header/description… */ | ||
.lh-metricfilter__radio:checked:not(#metric-All) ~ .lh-audit-group .lh-audit, | ||
.lh-metricfilter__radio:checked:not(#metric-All) ~ .lh-audit-group .lh-audit-group__description, | ||
.lh-metricfilter__radio:checked:not(#metric-All) ~ .lh-audit-group .lh-audit-group__itemcount { | ||
display: none; | ||
} | ||
/* …And then display:block the relevant ones */ | ||
${auditSelectors.join(',\n')} { | ||
display: block; | ||
} | ||
/*# sourceURL=metricfilter.css */ | ||
`; | ||
categoryEl.append(metricFilterEl); | ||
} | ||
} | ||
|
||
if (typeof module !== 'undefined' && module.exports) { | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
big LOL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
admittedly i should have googled before i build node 12 with
small-icu
. but yes. annoying.