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: clean up more clump/group/expandable crossover noise #6982

Merged
merged 1 commit into from
Jan 11, 2019

Conversation

brendankenny
Copy link
Member

works on top of #6930

@paulirish this is how far I think the simplification can go.

Gets rid of all expandable true/false business, groups and clumps that are <divs> and others that are <details>, etc. Clumps still use the group class names, but no longer use renderAuditGroup to make themselves.

@brendankenny brendankenny changed the title report: get rid of more clump/group/expandable crossover noise report: clean up more clump/group/expandable crossover noise Jan 11, 2019
@paulirish
Copy link
Member

what a beauty.

@paulirish paulirish added the 4.0 label Jan 11, 2019
@@ -409,12 +395,18 @@ class CategoryRenderer {
}

// Render each clump.
for (const [clumpId, clumpRefs] of clumps) {
if (clumpRefs.length === 0) continue;
for (const [clumpId, auditRefs] of clumps) {
Copy link
Member Author

Choose a reason for hiding this comment

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

using a loop when we have a fixed set of clumps is still kind of dumb, but manually unrolling the loop is just slightly worse, so still going with this

@brendankenny
Copy link
Member Author

This should also make #6974 quite a bit easier (no need for expandable or expandByDefault :)

@paulirish paulirish changed the base branch from expandpassed to master January 11, 2019 02:38
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.

nice! travis wants pwa-category-renderer updated FYI

@brendankenny
Copy link
Member Author

travis wants pwa-category-renderer updated FYI

I blame Paul's cherry picking skills :P

I'll fix.

@brendankenny
Copy link
Member Author

ok, jk, that was my fault :)

@brendankenny brendankenny merged commit d5fe65e into master Jan 11, 2019
@brendankenny brendankenny deleted the expandpassed-more branch January 11, 2019 17:40
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.

3 participants