report: show fireworks only if all core categories score 100 #9073
Conversation
| @@ -98,7 +99,9 @@ class ReportUIFeatures { | |||
|
|
|||
| // Fireworks. | |||
| const scoresAll100 = Object.values(report.categories).every(cat => cat.score === 1); | |||
| if (!this._dom.isDevTools() && scoresAll100) { | |||
| const hasAllCoreCategories = | |||
There was a problem hiding this comment.
hm, I guess this would still get fireworks for custom config categories but if the user is going all that way to get some fireworks they probably deserve 'em 😆
There was a problem hiding this comment.
I kind of feel like it should be at least perf + any other categories need to be 100. So solo perf 100 would be ok.
But I also don't care that much and this seems fine :)
There was a problem hiding this comment.
solo + fireworks would require a refactor, since the fireworks were designed to work with the scores container header.
| if (!this._dom.isDevTools() && scoresAll100) { | ||
| const hasAllCoreCategories = | ||
| Object.keys(report.categories).filter(id => !Util.isPluginCategory(id)).length >= 5; | ||
| if (!this._dom.isDevTools() && scoresAll100 && hasAllCoreCategories) { |
There was a problem hiding this comment.
brave soul creating merge conflicts for yourself 😉
| @@ -26,6 +26,7 @@ | |||
| /* globals getFilenamePrefix Util */ | |||
|
|
|||
| /** @typedef {import('./dom.js')} DOM */ | |||
| /** @typedef {import('./util.js')} Util */ | |||
There was a problem hiding this comment.
doesn't the html-renderer.d.ts take care of this? (I don't think either typedef is needed)
| @@ -98,7 +99,9 @@ class ReportUIFeatures { | |||
|
|
|||
| // Fireworks. | |||
| const scoresAll100 = Object.values(report.categories).every(cat => cat.score === 1); | |||
| if (!this._dom.isDevTools() && scoresAll100) { | |||
| const hasAllCoreCategories = | |||
There was a problem hiding this comment.
I kind of feel like it should be at least perf + any other categories need to be 100. So solo perf 100 would be ok.
But I also don't care that much and this seems fine :)
connorjclark
changed the title
Fixes #9058