From debcafd7606ecf569192faa84fd93c45b1bf3416 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Wed, 26 Jun 2019 18:37:59 +0100 Subject: [PATCH 1/5] if on trackpad, don't mess with horizontal scrolling. trackpad heuristic is 'if 15 minutes of no horizontal scrollwheel events, assume user may have switched to mousewheel' --- .../structures/IndicatorScrollbar.js | 36 +++++++++++++++---- 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/src/components/structures/IndicatorScrollbar.js b/src/components/structures/IndicatorScrollbar.js index b11e655f0d2..52e0c9b8ac0 100644 --- a/src/components/structures/IndicatorScrollbar.js +++ b/src/components/structures/IndicatorScrollbar.js @@ -131,17 +131,41 @@ export default class IndicatorScrollbar extends React.Component { // the harshness of the scroll behaviour. Should be a value between 0 and 1. const yRetention = 1.0; - // Check for trackpad users every so often to avoid boosting their scroll. + // whenever see horizontal scrolling, assume the user is on a trackpad + // for at least the next 15 minutes. + const now = new Date().getTime(); + if (Math.abs(e.deltaX) > 0) { + this._likelyTrackpadUser = true; + this._checkAgainForTrackpad = now + (15 * 60 * 1000); // 15min + } + else { + // if we haven't seen any horizontal scrolling for >15 minutes, assume + // the user might have plugged in a mousewheel + if (this._likelyTrackpadUser && now >= this._checkAgainForTrackpad) { + this._likelyTrackpadUser = false; + } + } + + // don't mess with the horizontal scroll for trackpad users + // See https://github.com/vector-im/riot-web/issues/10005 + if (this._likelyTrackpadUser) { + return; + } + + // Every 15 minutes, start checking to Check for trackpad users every so often to messing with their scroll // See https://github.com/vector-im/riot-web/issues/10005 const now = new Date().getTime(); - if (now >= this._checkAgainForTrackpad) { - this._likelyTrackpadUser = Math.abs(e.deltaX) > 0; + if (this._likelyTrackpadUser && now >= this._checkAgainForTrackpad) { + if (Math.abs(e.deltaX) > 0) { + this._likelyTrackpadUser = true; + } this._checkAgainForTrackpad = now + (15 * 60 * 1000); // 15min } - const safeToBoost = !this._likelyTrackpadUser; + if (this._likelyTrackpadUser) return; + + if (Math.abs(e.deltaX) <= xyThreshold) { // we are vertically scrolling. - if (Math.abs(e.deltaX) <= xyThreshold) { // HACK: We increase the amount of scroll to counteract smooth scrolling browsers. // Smooth scrolling browsers (Firefox) use the relative area to determine the scroll // amount, which means the likely small area of content results in a small amount of @@ -152,7 +176,7 @@ export default class IndicatorScrollbar extends React.Component { const additionalScroll = e.deltaY < 0 ? -50 : 50; // noinspection JSSuspiciousNameCombination - const val = Math.abs(e.deltaY) < 25 && safeToBoost ? (e.deltaY + additionalScroll) : e.deltaY; + const val = Math.abs(e.deltaY) < 25 ? (e.deltaY + additionalScroll) : 0; this._scrollElement.scrollLeft += val * yRetention; } } From 3d11eb430b59c77a53a925506f1a22cb34c061c7 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Wed, 26 Jun 2019 18:38:46 +0100 Subject: [PATCH 2/5] oops, remove old code --- src/components/structures/IndicatorScrollbar.js | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/components/structures/IndicatorScrollbar.js b/src/components/structures/IndicatorScrollbar.js index 52e0c9b8ac0..762a626c023 100644 --- a/src/components/structures/IndicatorScrollbar.js +++ b/src/components/structures/IndicatorScrollbar.js @@ -152,18 +152,6 @@ export default class IndicatorScrollbar extends React.Component { return; } - // Every 15 minutes, start checking to Check for trackpad users every so often to messing with their scroll - // See https://github.com/vector-im/riot-web/issues/10005 - const now = new Date().getTime(); - if (this._likelyTrackpadUser && now >= this._checkAgainForTrackpad) { - if (Math.abs(e.deltaX) > 0) { - this._likelyTrackpadUser = true; - } - this._checkAgainForTrackpad = now + (15 * 60 * 1000); // 15min - } - - if (this._likelyTrackpadUser) return; - if (Math.abs(e.deltaX) <= xyThreshold) { // we are vertically scrolling. // HACK: We increase the amount of scroll to counteract smooth scrolling browsers. From 7fc5d229d64823c142b74074b537760ea5324106 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Wed, 26 Jun 2019 21:13:17 +0100 Subject: [PATCH 3/5] fix stuff as per review --- src/components/structures/IndicatorScrollbar.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/components/structures/IndicatorScrollbar.js b/src/components/structures/IndicatorScrollbar.js index 762a626c023..21a23e5670b 100644 --- a/src/components/structures/IndicatorScrollbar.js +++ b/src/components/structures/IndicatorScrollbar.js @@ -131,15 +131,15 @@ export default class IndicatorScrollbar extends React.Component { // the harshness of the scroll behaviour. Should be a value between 0 and 1. const yRetention = 1.0; - // whenever see horizontal scrolling, assume the user is on a trackpad - // for at least the next 15 minutes. + // whenever we see horizontal scrolling, assume the user is on a trackpad + // for at least the next 1 minute. const now = new Date().getTime(); if (Math.abs(e.deltaX) > 0) { this._likelyTrackpadUser = true; - this._checkAgainForTrackpad = now + (15 * 60 * 1000); // 15min + this._checkAgainForTrackpad = now + (1 * 60 * 1000); } else { - // if we haven't seen any horizontal scrolling for >15 minutes, assume + // if we haven't seen any horizontal scrolling for a while, assume // the user might have plugged in a mousewheel if (this._likelyTrackpadUser && now >= this._checkAgainForTrackpad) { this._likelyTrackpadUser = false; @@ -164,7 +164,7 @@ export default class IndicatorScrollbar extends React.Component { const additionalScroll = e.deltaY < 0 ? -50 : 50; // noinspection JSSuspiciousNameCombination - const val = Math.abs(e.deltaY) < 25 ? (e.deltaY + additionalScroll) : 0; + const val = Math.abs(e.deltaY) < 25 ? (e.deltaY + additionalScroll) : e.deltaY; this._scrollElement.scrollLeft += val * yRetention; } } From 3873dc724aaa4072d367e0980293d11e33239b2a Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Wed, 26 Jun 2019 21:47:55 +0100 Subject: [PATCH 4/5] stupid linter >:( --- src/components/structures/IndicatorScrollbar.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/components/structures/IndicatorScrollbar.js b/src/components/structures/IndicatorScrollbar.js index 21a23e5670b..1895d2089dc 100644 --- a/src/components/structures/IndicatorScrollbar.js +++ b/src/components/structures/IndicatorScrollbar.js @@ -137,8 +137,7 @@ export default class IndicatorScrollbar extends React.Component { if (Math.abs(e.deltaX) > 0) { this._likelyTrackpadUser = true; this._checkAgainForTrackpad = now + (1 * 60 * 1000); - } - else { + } else { // if we haven't seen any horizontal scrolling for a while, assume // the user might have plugged in a mousewheel if (this._likelyTrackpadUser && now >= this._checkAgainForTrackpad) { From 00dfdfe7f171fad3822efde3a9bdf2851f1cd910 Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Fri, 28 Jun 2019 15:16:44 +0100 Subject: [PATCH 5/5] Fix linter warning --- src/components/structures/IndicatorScrollbar.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/structures/IndicatorScrollbar.js b/src/components/structures/IndicatorScrollbar.js index 1895d2089dc..d6efe8bee2c 100644 --- a/src/components/structures/IndicatorScrollbar.js +++ b/src/components/structures/IndicatorScrollbar.js @@ -152,7 +152,6 @@ export default class IndicatorScrollbar extends React.Component { } if (Math.abs(e.deltaX) <= xyThreshold) { // we are vertically scrolling. - // HACK: We increase the amount of scroll to counteract smooth scrolling browsers. // Smooth scrolling browsers (Firefox) use the relative area to determine the scroll // amount, which means the likely small area of content results in a small amount of