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

Add some tolerances to breadcrumb scrolling #2892

Merged
merged 1 commit into from
Apr 9, 2019

Conversation

turt2live
Copy link
Member

See element-hq/element-web#9400
See element-hq/element-web#9394

Tolerances are defined as a device-only setting to give advanced users an option to override the values. No UI is exposed for this.

The default values are picked for assumptions on comfort, however as people change the tolerances themselves the defaults may need to change.

@turt2live turt2live requested a review from a team April 8, 2019 16:54
See element-hq/element-web#9400
See element-hq/element-web#9394

Tolerances are defined as a device-only setting to give advanced users an option to override the values. No UI is exposed for this. 

The default values are picked for assumptions on comfort, however as people change the tolerances themselves the defaults may need to change.
@turt2live turt2live force-pushed the travis/breadcrumbs/scrolling branch from 46f9d68 to aa96fd2 Compare April 8, 2019 16:56
@dbkr dbkr self-assigned this Apr 9, 2019
Copy link
Member

@dbkr dbkr left a comment

Choose a reason for hiding this comment

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

Hmm - do we know how common a problem this is? If it's something that's going to affect a lot of people, telling them to tweak some values in their config doesn't really seem like a good solution. Also, presumably tis behaviour differs between trackpad / scroll wheel? In which case you'd tweak the values for one and then plug in / unplug a mouse and have to undo the tweaking again?

@dbkr dbkr removed their assignment Apr 9, 2019
@turt2live
Copy link
Member Author

It's a common problem for at least 2 people. The two variables are meant to solve the two separate problems: xyThreshold is meant to solve the trackpad problem whereas yReduction is meant to solve the scroll wheel problem. Plugging in a mouse would probably cause the tolerances to be outdated for the user, however I don't think the impact will be severe enough for them to notice. Can we even detect which device the user is using or when it changes?

The tolerances setting is more meant for us to try and work out sensible defaults. For instance, if people complain that the horizontal scroll is too fast after this then we can recommend they play with the settings to come up with numbers they like and we can use that to influence the defaults. To me this is preferable because it means less PRs with titles of "change yReduction to 0.65". The eventual plan is to either give the setting proper UI (if we want that) or remove it (if we don't want the UI).

@turt2live turt2live requested a review from dbkr April 9, 2019 15:19
Copy link
Member

@dbkr dbkr left a comment

Choose a reason for hiding this comment

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

Fair enough if it's so we can experiment & get the defaults right. Mostly my surprise is that a horizontal scroller needs special parameters tweaked rather than just being a native browser scrolling element.

@turt2live
Copy link
Member Author

It is a native scrolling element, but the this._scrollElement.scrollLeft += e.deltaY * yReduction; is what I think is throwing off the calculations. Humans aren't perfect (sadly), so there's probably some vertical component to their scroll which is causing excessive scrolling.

@turt2live turt2live merged commit 44e33ba into develop Apr 9, 2019
@turt2live turt2live deleted the travis/breadcrumbs/scrolling branch April 9, 2019 15:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants