From 8c3a275282f549ab706136d4df104e009e87212f Mon Sep 17 00:00:00 2001 From: William Wong Date: Tue, 12 Feb 2019 19:06:51 -0800 Subject: [PATCH 1/5] Fix Firefox scroll position locking --- CHANGELOG.md | 6 ++-- .../component/src/ScrollToBottom/Composer.js | 29 +++++++++++++++---- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0bdf881..35096b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,12 +5,14 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Changed +- `Composer`: fix #11, user scrolling in Firefox may have the scroll position locked occasionally, in PR [#12](https://github.com/compulim/react-scroll-to-bottom/issues/12) ## [1.3.0] - 2019-01-21 ### Changed - Playground: bumped to `react@16.6.0`, `react-dom@16.6.0`, and `react-scripts@2.1.6` -- Update algorithm, instead of using `componentDidUpdate`, we now use `setInterval` to check if the panel is sticky or not, this help to track content update that happen outside of React lifecycle, for example, `HTMLImageElement.onload` event -- `scrollTo()` now accepts `"100%"` instead of `"bottom"` +- `*`: Update algorithm, instead of using `componentDidUpdate`, we now use `setInterval` to check if the panel is sticky or not, this help to track content update that happen outside of React lifecycle, for example, `HTMLImageElement.onload` event +- `Composer`: `scrollTo()` now accepts `"100%"` instead of `"bottom"` ### Removed - Removed `threshold` props because the algorithm is now more robust diff --git a/packages/component/src/ScrollToBottom/Composer.js b/packages/component/src/ScrollToBottom/Composer.js index bff3278..ed8d6b3 100644 --- a/packages/component/src/ScrollToBottom/Composer.js +++ b/packages/component/src/ScrollToBottom/Composer.js @@ -9,7 +9,8 @@ import InternalContext from './InternalContext'; import SpineTo from '../SpineTo'; import StateContext from './StateContext'; -const MIN_CHECK_INTERVAL = 17; +const MIN_CHECK_INTERVAL = 17; // 1 frame +const SCROLL_DECISION_DECISION = 34; // 2 frames function setImmediateInterval(fn, ms) { fn(); @@ -85,15 +86,29 @@ export default class Composer extends React.Component { enableWorker() { clearInterval(this._stickyCheckTimeout); + let stickyButNotAtEndSince = false; + this._stickyCheckTimeout = setImmediateInterval( () => { const { state } = this; const { stateContext: { sticky }, target } = state; - if (sticky && target) { - const { atEnd } = computeViewState(state); - - !atEnd && state.functionContext.scrollToEnd(); + if (sticky && target && !computeViewState(state).atEnd) { + if (!stickyButNotAtEndSince) { + stickyButNotAtEndSince = Date.now(); + } else if (Date.now() - stickyButNotAtEndSince > SCROLL_DECISION_DECISION) { + // Quirks: In Firefox, after user scroll down, Firefox do two things: + // 1. Set to a new "scrollTop" + // 2. Fire "scroll" event + // For what we observed, #1 is fired about 20ms before #2. There is a chance that this stickyCheckTimeout is being scheduled between 1 and 2. + // That means, if we just look at #1 to decide if we should scroll, we will always scroll, in oppose to the user's intention. + // Repro: Open Firefox, set checkInterval to a lower number, and try to scroll by dragging the scroll handler. It will jump back. + + state.functionContext.scrollToEnd(); + stickyButNotAtEndSince = false; + } + } else { + stickyButNotAtEndSince = false; } }, Math.max(MIN_CHECK_INTERVAL, this.props.checkInterval) || MIN_CHECK_INTERVAL @@ -114,6 +129,8 @@ export default class Composer extends React.Component { } handleScroll({ timeStampLow }) { + console.log(`${ Date.now() }: onScroll`); + // Currently, there are no reliable way to check if the "scroll" event is trigger due to // user gesture, programmatic scrolling, or Chrome-synthesized "scroll" event to compensate size change. // Thus, we use our best-effort to guess if it is triggered by user gesture, and disable sticky if it is heading towards the start direction. @@ -207,7 +224,7 @@ export default class Composer extends React.Component { } Composer.defaultProps = { - checkInterval: 150, + checkInterval: 100, debounce: 17 }; From 4cf3320eac3c9ed53d76196a10048570c5e43497 Mon Sep 17 00:00:00 2001 From: William Wong Date: Tue, 12 Feb 2019 19:07:13 -0800 Subject: [PATCH 2/5] Remove console.log --- packages/component/src/ScrollToBottom/Composer.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/component/src/ScrollToBottom/Composer.js b/packages/component/src/ScrollToBottom/Composer.js index ed8d6b3..024dcb2 100644 --- a/packages/component/src/ScrollToBottom/Composer.js +++ b/packages/component/src/ScrollToBottom/Composer.js @@ -129,8 +129,6 @@ export default class Composer extends React.Component { } handleScroll({ timeStampLow }) { - console.log(`${ Date.now() }: onScroll`); - // Currently, there are no reliable way to check if the "scroll" event is trigger due to // user gesture, programmatic scrolling, or Chrome-synthesized "scroll" event to compensate size change. // Thus, we use our best-effort to guess if it is triggered by user gesture, and disable sticky if it is heading towards the start direction. From a5e3e4eef8fca945a0d1190ae83a7da2de5847fd Mon Sep 17 00:00:00 2001 From: William Wong Date: Tue, 12 Feb 2019 19:08:39 -0800 Subject: [PATCH 3/5] Clean up --- packages/component/src/ScrollToBottom/Composer.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/component/src/ScrollToBottom/Composer.js b/packages/component/src/ScrollToBottom/Composer.js index 024dcb2..732578f 100644 --- a/packages/component/src/ScrollToBottom/Composer.js +++ b/packages/component/src/ScrollToBottom/Composer.js @@ -93,7 +93,11 @@ export default class Composer extends React.Component { const { state } = this; const { stateContext: { sticky }, target } = state; - if (sticky && target && !computeViewState(state).atEnd) { + if ( + sticky + && target + && !computeViewState(state).atEnd + ) { if (!stickyButNotAtEndSince) { stickyButNotAtEndSince = Date.now(); } else if (Date.now() - stickyButNotAtEndSince > SCROLL_DECISION_DECISION) { From e3d60bdd73f314e3116f140f402942af79f52433 Mon Sep 17 00:00:00 2001 From: William Wong Date: Tue, 12 Feb 2019 19:53:59 -0800 Subject: [PATCH 4/5] Update PR number --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 35096b5..2d37f8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## [Unreleased] ### Changed -- `Composer`: fix #11, user scrolling in Firefox may have the scroll position locked occasionally, in PR [#12](https://github.com/compulim/react-scroll-to-bottom/issues/12) +- `Composer`: fix #13, user scrolling in Firefox may have the scroll position locked occasionally, in PR [#12](https://github.com/compulim/react-scroll-to-bottom/issues/12) ## [1.3.0] - 2019-01-21 ### Changed From 9c9fcdde7e08fc99cb81a6754f06c93deaf5b89f Mon Sep 17 00:00:00 2001 From: William Wong Date: Tue, 12 Feb 2019 20:10:26 -0800 Subject: [PATCH 5/5] Typo --- packages/component/src/ScrollToBottom/Composer.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/component/src/ScrollToBottom/Composer.js b/packages/component/src/ScrollToBottom/Composer.js index 732578f..fec8e69 100644 --- a/packages/component/src/ScrollToBottom/Composer.js +++ b/packages/component/src/ScrollToBottom/Composer.js @@ -10,7 +10,7 @@ import SpineTo from '../SpineTo'; import StateContext from './StateContext'; const MIN_CHECK_INTERVAL = 17; // 1 frame -const SCROLL_DECISION_DECISION = 34; // 2 frames +const SCROLL_DECISION_DURATION = 34; // 2 frames function setImmediateInterval(fn, ms) { fn(); @@ -100,7 +100,7 @@ export default class Composer extends React.Component { ) { if (!stickyButNotAtEndSince) { stickyButNotAtEndSince = Date.now(); - } else if (Date.now() - stickyButNotAtEndSince > SCROLL_DECISION_DECISION) { + } else if (Date.now() - stickyButNotAtEndSince > SCROLL_DECISION_DURATION) { // Quirks: In Firefox, after user scroll down, Firefox do two things: // 1. Set to a new "scrollTop" // 2. Fire "scroll" event