report(redesign): add sticky scores header #8524
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -157,6 +157,39 @@ class ReportRenderer { | |
| return container; | ||
| } | ||
|
|
||
| /** | ||
| * @param {LH.ReportResult} report | ||
| * @param {CategoryRenderer} categoryRenderer | ||
| * @param {Record<string, CategoryRenderer>} specificCategoryRenderers | ||
| * @return {DocumentFragment[]} | ||
| */ | ||
| _renderScoreGauges(report, categoryRenderer, specificCategoryRenderers) { | ||
| // Group gauges in this order: default, pwa, plugins. | ||
| const defaultGauges = []; | ||
| const customGauges = []; // PWA. | ||
| const pluginGauges = []; | ||
|
|
||
| for (const category of report.reportCategories) { | ||
| const renderer = specificCategoryRenderers[category.id] || categoryRenderer; | ||
| const categoryGauge = renderer.renderScoreGauge(category, report.categoryGroups || {}); | ||
|
|
||
| if (Util.isPluginCategory(category.id)) { | ||
| pluginGauges.push(categoryGauge); | ||
| } else if (renderer.renderScoreGauge === categoryRenderer.renderScoreGauge) { | ||
| // The renderer for default categories is just the default CategoryRenderer. | ||
| // If the functions are equal, then renderer is an instance of CategoryRenderer. | ||
| // For example, the PWA category uses PwaCategoryRenderer, which overrides | ||
| // CategoryRenderer.renderScoreGauge, so it would fail this check and be placed | ||
| // in the customGauges bucket. | ||
| defaultGauges.push(categoryGauge); | ||
| } else { | ||
| customGauges.push(categoryGauge); | ||
| } | ||
| } | ||
|
|
||
| return [...defaultGauges, ...customGauges, ...pluginGauges]; | ||
| } | ||
|
|
||
| /** | ||
| * @param {LH.ReportResult} report | ||
| * @return {DocumentFragment} | ||
|
|
@@ -223,29 +256,8 @@ class ReportRenderer { | |
| // } | ||
|
|
||
| if (scoreHeader) { | ||
| scoreHeader.append( | ||
| ...this._renderScoreGauges(report, categoryRenderer, specificCategoryRenderers)); | ||
|
There was a problem hiding this comment. kind of hard to read the spread after a line break like this. Maybe save to a local var first? |
||
| const scoreScale = this._dom.cloneTemplate('#tmpl-lh-scorescale', this._templateContext); | ||
| const scoresContainer = this._dom.find('.lh-scores-container', headerContainer); | ||
| scoresContainer.appendChild(scoreHeader); | ||
|
|
@@ -255,10 +267,24 @@ class ReportRenderer { | |
| reportSection.appendChild(this._renderReportFooter(report)); | ||
|
|
||
| const reportFragment = this._dom.createFragment(); | ||
|
|
||
| if (!this._dom.isDevTools()) { | ||
| const topbarDocumentFragment = this._renderReportTopbar(report); | ||
| reportFragment.appendChild(topbarDocumentFragment); | ||
| } | ||
|
|
||
| if (scoreHeader && !this._dom.isDevTools()) { | ||
| const stickyHeader = this._dom.createElement('div', 'lh-sticky-header'); | ||
|
There was a problem hiding this comment. we need a test for this, though jsdom (AFAIK) doesn't support scrolling, so it will have to be a limited one. Since we have a nice complete "renders score gauges in this order" test, maybe just a test that Or something like that There was a problem hiding this comment.
imo that doesn't seem like a useful test, since these gauges are created with the same function.
jsdom/jsdom#1422 (comment) this makes me think it should be possible to fake it There was a problem hiding this comment. ok it'd be way too much faking, basically all the |
||
| this._dom.createChildOf(stickyHeader, 'div', 'lh-highlighter'); | ||
|
|
||
| // The sticky header is just the score gauges, but styled to be smaller. Just | ||
| // clone the gauges from the score header. | ||
|
|
||
| stickyHeader.append( | ||
| ...this._renderScoreGauges(report, categoryRenderer, specificCategoryRenderers)); | ||
|
There was a problem hiding this comment. same thing with the line break and spread as above |
||
|
|
||
| reportFragment.appendChild(stickyHeader); | ||
| } | ||
|
|
||
| reportFragment.appendChild(headerContainer); | ||
| reportFragment.appendChild(container); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,6 +40,14 @@ class ReportUIFeatures { | |
| this._copyAttempt = false; | ||
| /** @type {HTMLElement} */ | ||
| this.exportButton; // eslint-disable-line no-unused-expressions | ||
| /** @type {HTMLElement} */ | ||
| this.topbarEl; // eslint-disable-line no-unused-expressions | ||
| /** @type {HTMLElement} */ | ||
| this.scoreScaleEl; // eslint-disable-line no-unused-expressions | ||
| /** @type {HTMLElement} */ | ||
| this.stickyHeaderEl; // eslint-disable-line no-unused-expressions | ||
| /** @type {HTMLElement} */ | ||
| this.highlightEl; // eslint-disable-line no-unused-expressions | ||
|
|
||
| this.onMediaQueryChange = this.onMediaQueryChange.bind(this); | ||
| this.onCopy = this.onCopy.bind(this); | ||
|
|
@@ -48,6 +56,7 @@ class ReportUIFeatures { | |
| this.onKeyDown = this.onKeyDown.bind(this); | ||
| this.printShortCutDetect = this.printShortCutDetect.bind(this); | ||
| this.onChevronClick = this.onChevronClick.bind(this); | ||
| this._updateStickyHeaderOnScroll = this._updateStickyHeaderOnScroll.bind(this); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -61,10 +70,13 @@ class ReportUIFeatures { | |
| this.json = report; | ||
| this._setupMediaQueryListeners(); | ||
| this._setupExportButton(); | ||
| this._setupStickyHeaderElements(); | ||
| this._setUpCollapseDetailsAfterPrinting(); | ||
| this._resetUIState(); | ||
| this._document.addEventListener('keydown', this.printShortCutDetect); | ||
| this._document.addEventListener('copy', this.onCopy); | ||
| this._document.addEventListener('scroll', this._updateStickyHeaderOnScroll); | ||
| window.addEventListener('resize', this._updateStickyHeaderOnScroll); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -102,6 +114,13 @@ class ReportUIFeatures { | |
| dropdown.addEventListener('click', this.onExport); | ||
| } | ||
|
|
||
| _setupStickyHeaderElements() { | ||
| this.topbarEl = this._dom.find('.lh-topbar', this._document); | ||
| this.scoreScaleEl = this._dom.find('.lh-scorescale', this._document); | ||
| this.stickyHeaderEl = this._dom.find('.lh-sticky-header', this._document); | ||
| this.highlightEl = this._dom.find('.lh-highlighter', this._document); | ||
| } | ||
|
|
||
| /** | ||
| * Handle copy events. | ||
| * @param {ClipboardEvent} e | ||
|
|
@@ -380,6 +399,29 @@ class ReportUIFeatures { | |
| this._document.body.removeChild(a); | ||
| setTimeout(_ => URL.revokeObjectURL(href), 500); | ||
| } | ||
|
|
||
| _updateStickyHeaderOnScroll() { | ||
|
There was a problem hiding this comment. does this need to be called once on startup? There was a problem hiding this comment.
I guess not since it starts out hidden There was a problem hiding this comment. TIL - https://stackoverflow.com/a/34095654
So fresh view (top of page), state should be hidden, and the default is just that. Refresh after scrolling a bit, or click on a link w/ an anchor to the report, and a scroll event fires. |
||
| // Show sticky header when the score scale begins to go underneath the topbar. | ||
| const topbarBottom = this.topbarEl.getBoundingClientRect().bottom; | ||
| const scoreScaleTop = this.scoreScaleEl.getBoundingClientRect().top; | ||
| const showStickyHeader = topbarBottom >= scoreScaleTop; | ||
| this.stickyHeaderEl.classList.toggle('lh-sticky-header--visible', showStickyHeader); | ||
|
There was a problem hiding this comment. we'll want to change this at the bottom. otherwise we force a layout within this function.
There was a problem hiding this comment. Interesting... The order is the same, so is the issue that code in between these two changes allows for the browser to jump in and do an extra layout? Idk how layouts work There was a problem hiding this comment. There was a problem hiding this comment. got it, makes sense. AFAICT from the performance tab w/ this code, there are no extra layouts. One change I saw for when the sticky header toggles - "Recalculate Styles" was within the update function, but w/ your changes it ran just after on the main thread. Would like to know if I'm reading the performance timeline wrong. Even if there technically isn't a layout cost, moving the DOM mutation to the bottom SGTM b/c it makes it obvious there is no layout thrashing. |
||
|
|
||
| // Highlight mini gauge when section is in view. | ||
|
There was a problem hiding this comment. early return if |
||
| // In view = the last category that starts above the middle of the window. | ||
| const categoryEls = Array.from(this._document.querySelectorAll('.lh-category')); | ||
| const categoriesAboveTheMiddle = | ||
| categoryEls.filter(el => el.getBoundingClientRect().top - window.innerHeight / 2 < 0); | ||
| const highlightIndex = | ||
| categoriesAboveTheMiddle.length > 0 ? categoriesAboveTheMiddle.length - 1 : 0; | ||
|
|
||
| // Category order matches gauge order in sticky header. | ||
| // TODO(hoten): not 100% true yet, need to order gauges like: core, pwa, plugins. Remove | ||
| // this comment when that is done. | ||
| const gaugeWrapperEls = this.stickyHeaderEl.querySelectorAll('.lh-gauge__wrapper'); | ||
| const gaugeToHighlight = gaugeWrapperEls[highlightIndex]; | ||
| this.highlightEl.style.left = gaugeToHighlight.getBoundingClientRect().left + 'px'; | ||
| } | ||
| } | ||
|
|
||
| if (typeof module !== 'undefined' && module.exports) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
categoryRendererandspecificCategoryRenderersprobably need docstrings (sorry for that second name :)