From 934ca6908fb0bd1c1fff5ea0507583628e229b96 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 11 Apr 2019 11:07:31 -0600 Subject: [PATCH] Remove breadcrumb scroll tolerances and use sensible defaults Fixes https://github.com/vector-im/riot-web/issues/9394 Fixes https://github.com/vector-im/riot-web/issues/9400 Numbers chosen based on user feedback. The setting has also been removed because it isn't really needed anymore. --- .../structures/IndicatorScrollbar.js | 30 +++++++------------ src/components/views/rooms/RoomBreadcrumbs.js | 10 ++----- src/settings/Settings.js | 7 ----- 3 files changed, 13 insertions(+), 34 deletions(-) diff --git a/src/components/structures/IndicatorScrollbar.js b/src/components/structures/IndicatorScrollbar.js index 2470f9fa11..a615717104 100644 --- a/src/components/structures/IndicatorScrollbar.js +++ b/src/components/structures/IndicatorScrollbar.js @@ -29,13 +29,6 @@ export default class IndicatorScrollbar extends React.Component { // scroll horizontally rather than vertically. This should only be used on components // with no vertical scroll opportunity. verticalScrollsHorizontally: PropTypes.bool, - - // An object containing 2 numbers: xyThreshold and yReduction. xyThreshold is the amount - // of horizontal movement required in order to ignore any vertical changes in scroll, and - // only applies when verticalScrollsHorizontally is true. yReduction is the factor to - // multiply the vertical delta by when verticalScrollsHorizontally is true. The default - // behaviour is to have an xyThreshold of infinity and a yReduction of 0.8 - scrollTolerances: PropTypes.object, }; constructor(props) { @@ -127,20 +120,19 @@ export default class IndicatorScrollbar extends React.Component { onMouseWheel = (e) => { if (this.props.verticalScrollsHorizontally && this._scrollElement) { - const xyThreshold = this.props.scrollTolerances - ? this.props.scrollTolerances.xyThreshold - : Number.MAX_SAFE_INTEGER; + // xyThreshold is the amount of horizontal motion required for the component to + // ignore the vertical delta in a scroll. Used to stop trackpads from acting in + // strange ways. Should be positive. + const xyThreshold = 0; - const yReduction = this.props.scrollTolerances - ? this.props.scrollTolerances.yReduction - : 0.8; + // yRetention is the factor multiplied by the vertical delta to try and reduce + // the harshness of the scroll behaviour. Should be a value between 0 and 1. + const yRetention = 1.0; - // Don't apply vertical motion to horizontal scrolls. This is meant to eliminate - // trackpads causing excessive scroll motion. - if (e.deltaX >= xyThreshold) return; - - // noinspection JSSuspiciousNameCombination - this._scrollElement.scrollLeft += e.deltaY * yReduction; + if (Math.abs(e.deltaX) < xyThreshold) { + // noinspection JSSuspiciousNameCombination + this._scrollElement.scrollLeft += e.deltaY * yRetention; + } } }; diff --git a/src/components/views/rooms/RoomBreadcrumbs.js b/src/components/views/rooms/RoomBreadcrumbs.js index 7c5947e049..fff1fa7f3c 100644 --- a/src/components/views/rooms/RoomBreadcrumbs.js +++ b/src/components/views/rooms/RoomBreadcrumbs.js @@ -33,12 +33,7 @@ const MAX_ROOMS = 20; export default class RoomBreadcrumbs extends React.Component { constructor(props) { super(props); - - const tolerances = SettingsStore.getValue("breadcrumb_scroll_tolerances"); - this.state = {rooms: [], scrollTolerances: tolerances}; - - // Record this for debugging purposes - console.log("Breadcrumbs scroll tolerances:", tolerances); + this.state = {rooms: []}; this.onAction = this.onAction.bind(this); this._dispatcherRef = null; @@ -343,8 +338,7 @@ export default class RoomBreadcrumbs extends React.Component { }); return ( + trackHorizontalOverflow={true} verticalScrollsHorizontally={true}> { avatars } ); diff --git a/src/settings/Settings.js b/src/settings/Settings.js index dcd6f91aef..35baa718b9 100644 --- a/src/settings/Settings.js +++ b/src/settings/Settings.js @@ -262,13 +262,6 @@ export const SETTINGS = { supportedLevels: ['account'], default: [], }, - "breadcrumb_scroll_tolerances": { - supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS, - default: { - xyThreshold: 10, - yReduction: 0.8, - }, - }, "analyticsOptIn": { supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG, displayName: _td('Send analytics data'),