report: metric toggle without JS #8844
Conversation
| const toggleTmpl = this.dom.cloneTemplate('#tmpl-lh-metrics-toggle', this.templateContext); | ||
| const toggleEl = this.dom.find('.lh-metrics-toggle', toggleTmpl); | ||
| auditGroupHeader.appendChild(toggleEl); | ||
| metricAuditsEl.prepend(...toggleEl.childNodes); |
There was a problem hiding this comment.
did we lose a little padding by removing this nesting layer? feels a bit close now
| this.metricDescriptionToggleEl.addEventListener('input', this._toggleMetricDescription); | ||
| this.metricAuditGroup.addEventListener('click', e => { | ||
| const el = /** @type {HTMLElement} */ (e.target); | ||
| if (el.closest('.lh-metric__title')) this.metricDescriptionToggleEl.click(); |
There was a problem hiding this comment.
we lose this click behavior now
IMO, it was a little wonky anyhoo so not the biggest loss but something to consider 🤷♂
There was a problem hiding this comment.
we agreed to remove this thing separately. connor's PR also nukes it
https://github.com/GoogleChrome/lighthouse/pull/8785/files#diff-10f7ab3e3a96491edf29819a09f5b45fL271
|
oh and tests need a bit of love :) |
|
|
||
| .lh-metric__value { | ||
| white-space: nowrap; /* No wrapping between metric value and the icon */ | ||
| font-weight: 500; | ||
| } | ||
|
|
||
| .lh-metrics-toggle { | ||
| position: relative; | ||
| /* No-JS toggle switch |
There was a problem hiding this comment.
is the indent weird here or just in the review UI?
| @@ -163,40 +163,43 @@ describe('ReportUIFeatures', () => { | |||
| }); | |||
| }); | |||
|
|
|||
| // This is done all in CSS, but tested here. | |||
There was a problem hiding this comment.
since this is self contained, cut and paste over to performance-category-renderer.js. Slightly better there?
| const showClass = 'lh-audit-group--metrics__show-descriptions'; | ||
| const metricsSelector = '.lh-audit-group--metrics'; | ||
| const toggleSelector = '.lh-metrics-toggle__input'; | ||
| const magicSelector = '.lh-metrics-toggle__input:checked ~ .lh-columns .lh-metric__description'; |
|
Merged 35 seconds b4 my LGTM! |
Because importing report-features into PSI is a pain
and because we don't need JS :P