-
Notifications
You must be signed in to change notification settings - Fork 9.6k
report(performance): use metric savings for metric filter #15540
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
Conversation
| const labelEl = this.dom.createChildOf(metricFilterEl, 'label', 'lh-metricfilter__label'); | ||
| labelEl.htmlFor = elemId; | ||
| labelEl.title = metric.result?.title; | ||
| labelEl.title = 'result' in metric ? metric.result.title : ''; |
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.
This fixes a bug where the title of the "All" filter button is "undefined"
| /** Gather mode used to collect artifacts. */ | ||
| type GatherMode = 'navigation'|'timespan'|'snapshot'; | ||
|
|
||
| type MetricAcronym = 'FCP' | 'LCP' | 'TBT' | 'CLS' | 'INP' | 'SI' | 'TTI' | 'FMP'; |
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.
|
ping @connorjclark |
adrianaixba
left a comment
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.
lgtm!
| renderAudit(audit) { | ||
| const component = this.dom.createComponent('audit'); | ||
| return this.populateAuditValues(audit, component); | ||
| return /** @type {HTMLElement} */ (this.populateAuditValues(audit, component)); |
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.
which part required this change to return type?
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.
hidden is not a property on Element but it is on HTMLElement
#15445 (comment)
Metric filter was still using
relevantAuditswhich is just a manually curated list that we haven't touched in a while. This PR usesmetricSavingsinstead to determine filter applicability and sorts the audits on the filtered metric.