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

Conversation

brendankenny
Copy link
Member

part of #6395

When all the audits in a PWA category group are passing, the icon goes from grayscale to color to indicate its badge (score gauge is still unstyled).

@@ -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)

Copy link
Collaborator

@patrickhulce patrickhulce left a comment

Choose a reason for hiding this comment

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

LGTM!

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" :)

@brendankenny brendankenny merged commit 29c5751 into master Nov 8, 2018
@brendankenny brendankenny deleted the pwabadges branch November 8, 2018 00:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants