report: New audit group clump for audits with warnings #6974
Conversation
connorjclark
requested review from
brendankenny,
exterkamp and
paulirish
as code owners
| const expandable = clumpInfo.expandable; | ||
| const expandByDefault = clumpInfo.expandByDefault; | ||
|
|
||
| if (!expandable) { | ||
| // Failed audit clump is always expanded and not nested in an lh-audit-group. |
There was a problem hiding this comment.
update comment and names now then?
| const elements = this._renderGroupedAudits(auditRefs, groupDefinitions, {expandable: false}); | ||
| const elements = this._renderGroupedAudits(auditRefs, groupDefinitions, { | ||
| expandable: false, | ||
| expandByDefault: false, |
There was a problem hiding this comment.
this one is a bit confusing since won't it be expanded by default? how else would you expand it? maybe this value shouldn't matter when expandable is false
| @@ -196,6 +211,9 @@ class CategoryRenderer { | |||
| if (expandable) { | |||
| const chevronEl = summaryInnerEl.appendChild(this._createChevron()); | |||
| chevronEl.title = Util.UIStrings.auditGroupExpandTooltip; | |||
| if (opts.expandByDefault) { | |||
There was a problem hiding this comment.
seems like this is ripe for some cleanup like our boolean traps over in page load.
WDYT about an enum instead of the two booleans type ExpansionBehavior = 'open-permanent' | 'closed-expandable' | 'open-expandable' or something like that?
| className: 'lh-clump--failed', | ||
| expandable: false, | ||
| expandByDefault: false, |
There was a problem hiding this comment.
arg, sorry, I was hoping you'd wait for #6930. I think a lot of this file can be deleted. I'm not sure it even makes sense to have clumps anymore since groups and clumps won't be at all alike anymore.
There was a problem hiding this comment.
ok, i'll wait for that. third time's a charm
|
Abandoning for #6989 |
connorjclark
deleted the
issue-5327-expand-passed-audits-group-with-warnings-pass-2
branch
fixes #5327