report(flow): refine snapshot and timespan performance #13184
Conversation
| @@ -172,33 +172,35 @@ export class PerformanceCategoryRenderer extends CategoryRenderer { | |||
| } | |||
|
|
|||
| // Metrics. | |||
| const metricAuditsEl = this.renderAuditGroup(groups.metrics); | |||
| const metricAudits = category.auditRefs.filter(audit => audit.group === 'metrics'); | |||
| if (metricAudits.length) { | |||
There was a problem hiding this comment.
oof, really reveals the length of this function 😬
report/renderer/util.js
Outdated
| const auditPassed = Util.showAsPassed(auditRef.result); | ||
|
|
||
| // Don't count the audit if it's manual or isn't displayed. | ||
| if (!auditRef.group || auditRef.result.scoreDisplayMode === 'manual') { |
There was a problem hiding this comment.
FWIW I'm pretty sure we only enforce no group === not displayed in performance-category-renderer. It looks like it's the de facto rule now, but only because every non-manual, non-perf audit appears to have a group assigned in the default config since #10623
There was a problem hiding this comment.
To be safe I added a category.id === 'performance' condition
|
So informative audits marked N/A will not be completely ignored when calculating the audit fraction because they will no longer have I think this change is still worthwhile, since the number of "failing" audits is still accurately represented. Maybe we could track N/A audits separately as well? |
Seems like |
Ya that's what I'm thinking. I don't think a |
| @@ -297,7 +297,7 @@ describe('CategoryRenderer', () => { | |||
| ); | |||
|
|
|||
| const gauge = categoryDOM.querySelector('.lh-fraction__content'); | |||
| assert.equal(gauge.textContent.trim(), '39/44', 'fraction is included'); | |||
There was a problem hiding this comment.
Accessibility does have a lot of N/A audits in this sample. I don't think it's a huge deal.
A few changes here:
#11313