From cf16b6c20b2bf847fabf3f6743b739b6e812eeeb Mon Sep 17 00:00:00 2001 From: Franz Liedke Date: Thu, 5 Sep 2019 02:17:09 +0200 Subject: [PATCH] Optimize ScrollListener performance Listen to "scroll" event and throttle callback executions instead of actively polling for changes to the scroll position. Fixes #1222. --- .../js/src/common/utils/ScrollListener.js | 40 ++++++++++--------- .../js/src/forum/components/PostStream.js | 3 +- .../forum/components/PostStreamScrubber.js | 3 +- 3 files changed, 24 insertions(+), 22 deletions(-) diff --git a/framework/core/js/src/common/utils/ScrollListener.js b/framework/core/js/src/common/utils/ScrollListener.js index cf71b42a3..3a35290f3 100644 --- a/framework/core/js/src/common/utils/ScrollListener.js +++ b/framework/core/js/src/common/utils/ScrollListener.js @@ -1,4 +1,4 @@ -const scroll = window.requestAnimationFrame || +const later = window.requestAnimationFrame || window.webkitRequestAnimationFrame || window.mozRequestAnimationFrame || window.msRequestAnimationFrame || @@ -17,7 +17,7 @@ export default class ScrollListener { */ constructor(callback) { this.callback = callback; - this.lastTop = -1; + this.ticking = false; } /** @@ -27,27 +27,27 @@ export default class ScrollListener { * @protected */ loop() { - if (!this.active) return; + // THROTTLE: If the callback is still running (or hasn't yet run), we ignore + // further scroll events. + if (this.ticking) return; - this.update(); + // Schedule the callback to be executed soon (TM), and stop throttling once + // the callback is done. + later(() => { + this.update(); + this.ticking = false; + }); - scroll(this.loop.bind(this)); + this.ticking = true; } /** - * Check if the scroll position has changed; if it has, run the handler. + * Run the callback, whether there was a scroll event or not. * - * @param {Boolean} [force=false] Whether or not to force the handler to be - * run, even if the scroll position hasn't changed. * @public */ - update(force) { - const top = window.pageYOffset; - - if (this.lastTop !== top || force) { - this.callback(top); - this.lastTop = top; - } + update() { + this.callback(window.pageYOffset); } /** @@ -57,8 +57,10 @@ export default class ScrollListener { */ start() { if (!this.active) { - this.active = true; - this.loop(); + window.addEventListener( + 'scroll', + this.active = this.loop.bind(this) + ); } } @@ -68,6 +70,8 @@ export default class ScrollListener { * @public */ stop() { - this.active = false; + window.removeEventListener('scroll', this.active); + + this.active = null; } } diff --git a/framework/core/js/src/forum/components/PostStream.js b/framework/core/js/src/forum/components/PostStream.js index 335d0f441..02cc2700b 100644 --- a/framework/core/js/src/forum/components/PostStream.js +++ b/framework/core/js/src/forum/components/PostStream.js @@ -2,7 +2,6 @@ import Component from '../../common/Component'; import ScrollListener from '../../common/utils/ScrollListener'; import PostLoading from './LoadingPost'; import anchorScroll from '../../common/utils/anchorScroll'; -import mixin from '../../common/utils/mixin'; import evented from '../../common/utils/evented'; import ReplyPlaceholder from './ReplyPlaceholder'; import Button from '../../common/components/Button'; @@ -586,7 +585,7 @@ class PostStream extends Component { */ unpause() { this.paused = false; - this.scrollListener.update(true); + this.scrollListener.update(); this.trigger('unpaused'); } } diff --git a/framework/core/js/src/forum/components/PostStreamScrubber.js b/framework/core/js/src/forum/components/PostStreamScrubber.js index 5d59f8fe3..8395aba3c 100644 --- a/framework/core/js/src/forum/components/PostStreamScrubber.js +++ b/framework/core/js/src/forum/components/PostStreamScrubber.js @@ -2,7 +2,6 @@ import Component from '../../common/Component'; import icon from '../../common/helpers/icon'; import ScrollListener from '../../common/utils/ScrollListener'; import SubtreeRetainer from '../../common/utils/SubtreeRetainer'; -import computed from '../../common/utils/computed'; import formatNumber from '../../common/utils/formatNumber'; /** @@ -365,7 +364,7 @@ export default class PostStreamScrubber extends Component { } onresize() { - this.scrollListener.update(true); + this.scrollListener.update(); // Adjust the height of the scrollbar so that it fills the height of // the sidebar and doesn't overlap the footer.