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

Blocks native touch gestures #33

Closed
jimmynotjim opened this issue Feb 2, 2021 · 8 comments · Fixed by #34
Closed

Blocks native touch gestures #33

jimmynotjim opened this issue Feb 2, 2021 · 8 comments · Fixed by #34
Assignees

Comments

@jimmynotjim
Copy link
Contributor

I'm using React RemoveScroll via React FocusOn and found an issue where the shouldPrevent function seems to be preventing native touch gestures like sliding a range input.

I tried creating a simplified Codepen example but ran into a minified error when I added the <RemoveScroll> around the input. https://codepen.io/jimmynotjim/pen/MWbweOB If you can get past the error, or recreate the pen in your online editor of choice, open the result in the Chrome mobile device emulator, iOS emulator, or an actual touch device, and observe that the drag gestures are no longer working.

@theKashey
Copy link
Owner

Sounds like doing of this library, this is what it does - cancels all movement which can lead to scroll.
Right now I am not sure how to handle your case and understand that it will not cause scroll. Probably the easiest way will be to add some logic to "allow" certain touches on certain locations.
We'll see.

@theKashey theKashey self-assigned this Feb 2, 2021
@jimmynotjim
Copy link
Contributor Author

@theKashey If you're open to contributions and can provide a preferred direction, I can submit a PR.

My first instinct was a whitelist prop similar to your FocusOn library, but I also wondered if there should be an internal whitelist of known touch related elements. Another option may be to ignore any touch events that originate within the RemoveScroll wrapper, which I'm guessing might eliminate the need for -webkit-overflow-scrolling: touch; but unsure.

Anyway, I'm happy to help in whatever way I can.

@theKashey
Copy link
Owner

I am open to any contributions but really not sure what would be the best way to address this issue.

The simplest solution is to allow horizontal scroll on some elements, as usually there is not much sense to block it, and if some undesired events will slip - nothing serious will happen.

However look like the problem can be a little more complicated, and cover for example "selection" of the text in some inputs.

So let's start from a simple step:

Sounds like a plan?

@jimmynotjim
Copy link
Contributor Author

Sorry, had a busy couple of days. Sounds good, I should have some time today/tomorrow to dig into this and report back.

@jimmynotjim
Copy link
Contributor Author

@theKashey quick update that I had some work projects come up unexpectedly but I'm starting to dig into this today

@jimmynotjim
Copy link
Contributor Author

Spent some time digging into the code this weekend, and before I proceed, I wanted to check if you've explored limiting the wheel, touchmove, and touchstart listeners to the page content outside the RemoveScroll component? It might be naive of me since I don't know the history, but I wanted to ask.

@theKashey
Copy link
Owner

First of all - please check theKashey/react-focus-on#49 - we might have an easier solution for this task. Literally in not using anything from this package and just blocking body scroll.

if you've explored limiting the wheel, touchmove, and touchstart listeners to the page content outside the RemoveScroll component?

This is the core logic of this component - it prevents offset change to leak into the parent component. Event is not originated at "parent" as Modal (or another element) covers everything, but when you "scroll" using different methods, you communicate with the browser to "scroll something".
Currently logic allows delta which components within the lock can process, even events which will cause internal scroll, not external.

In the case of radio groups it does not know that you can "scroll" radio input, and thus blocks event, thinking that it will cause unintended parent scroll.

@jimmynotjim
Copy link
Contributor Author

Hey @theKashey, I was out on PTO last week, but followed up today. Pretty sure this pr ☝️ what you are saying, but feel free to tell me I'm wrong. I'll also try the solution in the other issue, but figured this should probably be resolved either way for future users.

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 a pull request may close this issue.

2 participants