Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix touch offsets #422

Merged
merged 2 commits into from
Mar 24, 2024

Conversation

elliottkember
Copy link
Contributor

@elliottkember elliottkember commented Mar 3, 2024

I have noticed that the slider's value is not accurate when tapping. The slider jumps to a lower value whenever dragging starts. It is even more incorrect when there is a custom thumbTouchSize is used, the problem is even worse.

This happens on web, and also on mobile. Here is an example:

Before:

Screen.Recording.2024-03-03.at.10.00.11.PM.mov

This PR resolves the issue in my testing. Sliders are now much more accurate:

After:

Screen.Recording.2024-03-03.at.10.00.34.PM.mov

The one thing I'm not 100% happy with is that the dragging will still assume you want to drag the centre of the slider. We should ideally calculate the offset between the starting touch and that position, and maintain that as an offset, so that drags are relative. I may fix that in a follow-up PR if this gets merged.

@elliottkember
Copy link
Contributor Author

@miblanchard sorry to ping you in a comment, but I’m not able to request a review on this repo for some reason, but this fix really worked wonders for usability. I’m not sure how long this has been broken

Copy link
Owner

@miblanchard miblanchard left a comment

Choose a reason for hiding this comment

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

Thank you @elliottkember

@miblanchard miblanchard merged commit 42aed92 into miblanchard:main Mar 24, 2024
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants