Skip to content

Commit 68ba77a

Browse files
authored
report: fix sticky header toggling too late (#13279)
1 parent 5d8ea1a commit 68ba77a

File tree

3 files changed

+49
-32
lines changed

3 files changed

+49
-32
lines changed

report/renderer/topbar-features.js

+38-32
Original file line numberDiff line numberDiff line change
@@ -29,16 +29,15 @@ export class TopbarFeatures {
2929
/** @type {HTMLElement} */
3030
this.topbarEl; // eslint-disable-line no-unused-expressions
3131
/** @type {HTMLElement} */
32-
this.scoreScaleEl; // eslint-disable-line no-unused-expressions
33-
/** @type {HTMLElement} */
32+
this.categoriesEl; // eslint-disable-line no-unused-expressions
33+
/** @type {HTMLElement?} */
3434
this.stickyHeaderEl; // eslint-disable-line no-unused-expressions
3535
/** @type {HTMLElement} */
3636
this.highlightEl; // eslint-disable-line no-unused-expressions
3737
this.onDropDownMenuClick = this.onDropDownMenuClick.bind(this);
3838
this.onKeyUp = this.onKeyUp.bind(this);
3939
this.onCopy = this.onCopy.bind(this);
4040
this.collapseAllDetails = this.collapseAllDetails.bind(this);
41-
this._updateStickyHeaderOnScroll = this._updateStickyHeaderOnScroll.bind(this);
4241
}
4342

4443
/**
@@ -54,25 +53,7 @@ export class TopbarFeatures {
5453
const topbarLogo = this._dom.find('.lh-topbar__logo', this._dom.rootEl);
5554
topbarLogo.addEventListener('click', () => toggleDarkTheme(this._dom));
5655

57-
// There is only a sticky header when at least 2 categories are present.
58-
if (Object.keys(this.lhr.categories).length >= 2) {
59-
this._setupStickyHeaderElements();
60-
const reportRootEl = this._dom.rootEl;
61-
const elToAddScrollListener = this._getScrollParent(reportRootEl);
62-
elToAddScrollListener.addEventListener('scroll', this._updateStickyHeaderOnScroll);
63-
64-
// Use ResizeObserver where available.
65-
// TODO: there is an issue with incorrect position numbers and, as a result, performance
66-
// issues due to layout thrashing.
67-
// See https://github.com/GoogleChrome/lighthouse/pull/9023/files#r288822287 for details.
68-
// For now, limit to DevTools.
69-
if (this._dom.isDevTools()) {
70-
const resizeObserver = new window.ResizeObserver(this._updateStickyHeaderOnScroll);
71-
resizeObserver.observe(reportRootEl);
72-
} else {
73-
window.addEventListener('resize', this._updateStickyHeaderOnScroll);
74-
}
75-
}
56+
this._setupStickyHeader();
7657
}
7758

7859
/**
@@ -233,7 +214,7 @@ export class TopbarFeatures {
233214
/**
234215
* Finds the first scrollable ancestor of `element`. Falls back to the document.
235216
* @param {Element} element
236-
* @return {Node}
217+
* @return {Element | Document}
237218
*/
238219
_getScrollParent(element) {
239220
const {overflowY} = window.getComputedStyle(element);
@@ -271,20 +252,45 @@ export class TopbarFeatures {
271252
}
272253
}
273254

274-
_setupStickyHeaderElements() {
255+
_setupStickyHeader() {
256+
// Cache these elements to avoid qSA on each onscroll.
275257
this.topbarEl = this._dom.find('div.lh-topbar', this._dom.rootEl);
276-
this.scoreScaleEl = this._dom.find('div.lh-scorescale', this._dom.rootEl);
277-
this.stickyHeaderEl = this._dom.find('div.lh-sticky-header', this._dom.rootEl);
258+
this.categoriesEl = this._dom.find('div.lh-categories', this._dom.rootEl);
259+
260+
// Defer behind rAF to avoid forcing layout.
261+
window.requestAnimationFrame(() => window.requestAnimationFrame(() => {
262+
// Only present in the DOM if it'll be used (>=2 categories)
263+
try {
264+
this.stickyHeaderEl = this._dom.find('div.lh-sticky-header', this._dom.rootEl);
265+
} catch {
266+
return;
267+
}
278268

279-
// Highlighter will be absolutely positioned at first gauge, then transformed on scroll.
280-
this.highlightEl = this._dom.createChildOf(this.stickyHeaderEl, 'div', 'lh-highlighter');
269+
// Highlighter will be absolutely positioned at first gauge, then transformed on scroll.
270+
this.highlightEl = this._dom.createChildOf(this.stickyHeaderEl, 'div', 'lh-highlighter');
271+
272+
// Update sticky header visibility and highlight when page scrolls/resizes.
273+
const scrollParent = this._getScrollParent(this._dom.rootEl);
274+
// The 'scroll' handler must be should be on {Element | Document}...
275+
scrollParent.addEventListener('scroll', () => this._updateStickyHeader());
276+
// However resizeObserver needs an element, *not* the document.
277+
const resizeTarget = scrollParent instanceof window.Document
278+
? document.documentElement
279+
: scrollParent;
280+
new window.ResizeObserver(() => this._updateStickyHeader()).observe(resizeTarget);
281+
}));
281282
}
282283

283-
_updateStickyHeaderOnScroll() {
284-
// Show sticky header when the score scale begins to go underneath the topbar.
284+
/**
285+
* Toggle visibility and update highlighter position
286+
*/
287+
_updateStickyHeader() {
288+
if (!this.stickyHeaderEl) return;
289+
290+
// Show sticky header when the main 5 gauges clear the topbar.
285291
const topbarBottom = this.topbarEl.getBoundingClientRect().bottom;
286-
const scoreScaleTop = this.scoreScaleEl.getBoundingClientRect().top;
287-
const showStickyHeader = topbarBottom >= scoreScaleTop;
292+
const categoriesTop = this.categoriesEl.getBoundingClientRect().top;
293+
const showStickyHeader = topbarBottom >= categoriesTop;
288294

289295
// Highlight mini gauge when section is in view.
290296
// In view = the last category that starts above the middle of the window.

report/test/clients/bundle-test.js

+5
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,17 @@ describe('lighthouseRenderer bundle', () => {
2929
document = window.document;
3030

3131
global.window = global.self = window;
32+
global.window.requestAnimationFrame = fn => fn();
3233
// Stub out matchMedia for Node.
3334
global.self.matchMedia = function() {
3435
return {
3536
addListener: function() {},
3637
};
3738
};
39+
global.window.ResizeObserver = class ResizeObserver {
40+
observe() { }
41+
unobserve() { }
42+
};
3843
});
3944

4045
afterAll(() => {

report/test/renderer/report-ui-features-test.js

+6
Original file line numberDiff line numberDiff line change
@@ -60,14 +60,20 @@ describe('ReportUIFeatures', () => {
6060

6161
global.HTMLElement = document.window.HTMLElement;
6262
global.HTMLInputElement = document.window.HTMLInputElement;
63+
global.HTMLInputElement = document.window.HTMLInputElement;
6364

6465
global.window = document.window;
66+
global.window.requestAnimationFrame = fn => fn();
6567
global.window.getComputedStyle = function() {
6668
return {
6769
marginTop: '10px',
6870
height: '10px',
6971
};
7072
};
73+
global.window.ResizeObserver = class ResizeObserver {
74+
observe() { }
75+
unobserve() { }
76+
};
7177

7278
dom = new DOM(document.window.document);
7379
sampleResults = Util.prepareReportResult(sampleResultsOrig);

0 commit comments

Comments
 (0)