Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Remove breadcrumb scroll tolerances and use sensible defaults #2913

Merged
merged 1 commit into from
Apr 11, 2019
Merged

Remove breadcrumb scroll tolerances and use sensible defaults #2913

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 13 additions & 21 deletions src/components/structures/IndicatorScrollbar.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;

const yReduction = this.props.scrollTolerances
? this.props.scrollTolerances.yReduction
: 0.8;

// 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;
// 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;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting choice to keep the variables around when they're both effectively causing no change... I guess it's in case we want to adjust down the road? Would it be simpler to just remove for now?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's more to keep the documentation for the behaviour, and to make it slightly easier to tweak down the road if needed.


// 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;

if (Math.abs(e.deltaX) < xyThreshold) {
// noinspection JSSuspiciousNameCombination
this._scrollElement.scrollLeft += e.deltaY * yRetention;
}
}
};

Expand Down
10 changes: 2 additions & 8 deletions src/components/views/rooms/RoomBreadcrumbs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -343,8 +338,7 @@ export default class RoomBreadcrumbs extends React.Component {
});
return (
<IndicatorScrollbar ref="scroller" className="mx_RoomBreadcrumbs"
trackHorizontalOverflow={true} verticalScrollsHorizontally={true}
scrollTolerances={this.state.scrollTolerances}>
trackHorizontalOverflow={true} verticalScrollsHorizontally={true}>
{ avatars }
</IndicatorScrollbar>
);
Expand Down
7 changes: 0 additions & 7 deletions src/settings/Settings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down