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: "making things draggable blocks touch scrolling" #134

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Raiondesu
Copy link

@Raiondesu Raiondesu commented Sep 20, 2023

This PR fixes #61 while also adding some more customizability to the touch-action property of draggable elements.

With this PR, it is now possible to simply set touch-action to another value in case there's a need.

This PR also adds a listener for dragEnd to the touchend event, as on mobile, when scrolling, pointerup may never even fire.
This, however, has the drawback of sometimes firing the dragEnd callback multiple times.
I tested various other solutions to prevent this effect, including using releasePointerCapture, without any success.

The issue is not completely gone, however, as the problem of janky dragging still persists sometimes, though much less now thanks to the touchend event.
I'll be trying multiple ways to fix it completely in the coming days. However, it's possible to almost completely fix it in userland by implementing a threshold system in the onDrag handler.

Any feedback is very welcome!

@changeset-bot
Copy link

changeset-bot bot commented Sep 20, 2023

⚠️ No Changeset found

Latest commit: 3c6c6f4

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Sep 20, 2023

@Raiondesu is attempting to deploy a commit to the Purus Projects Team on Vercel.

A member of the Team first needs to authorize it.

@Raiondesu Raiondesu changed the title fix: "making things draggable blocks touch scrolling" fix: "making things draggable blocks touch scrolling" WIP Sep 20, 2023
@Raiondesu Raiondesu changed the title fix: "making things draggable blocks touch scrolling" WIP fix: "making things draggable blocks touch scrolling" Sep 20, 2023
@PuruVJ
Copy link
Owner

PuruVJ commented Apr 9, 2024

Hi, thanks for the PR

This, however, has the drawback of sometimes firing the dragEnd callback multiple times.

Does this have a negative side-effect?

@Raiondesu
Copy link
Author

Does this have a negative side-effect?

Only in case of having side effects directly in the onDragEnd event handler - since it might be executed more than once.

I believe it's possible to implement a workaround - only trigger onDragEnd if onDragStart was triggered at least once since the last onDragEnd. This would potentially solve the issue here.

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.

Making things draggable blocks touch scrolling
2 participants