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

Two-Finger gestures disabled when dragging disabled in samsung browser #5969

Closed
syno1997 opened this issue Dec 15, 2017 · 15 comments · Fixed by #7029
Closed

Two-Finger gestures disabled when dragging disabled in samsung browser #5969

syno1997 opened this issue Dec 15, 2017 · 15 comments · Fixed by #7029

Comments

@syno1997
Copy link

syno1997 commented Dec 15, 2017

I use the Samsung Internet Browser from my tablet and verified the issue with my Huawei smartphone by downloading the Samsung Internet Browser.
When I do "draggable: false", it also disables to pan with 2 fingers on the Samsung Browser but in none of the other browsers I tested. (Chrome, Firefox, IE, Edge, Safari)

How to reproduce

  • Leaflet version I'm using: Leaflet 1.2.0
  • Browser (with version) I'm using: Samsung Internet Browser (latest 20171215)
  • OS/Platform (with version) I'm using: Samsung/Android, other Devices Android with the specified browser installed.
    Just use any example from the leaflet docs and disable dragging.

What behaviour I'm expecting and which behaviour I'm seeing

I can not use 2 fingers to do 2 finger gestures like panning/zooming.
I expected it to do so, since all other browsers do.

Minimal example reproducing the issue

https://plnkr.co/edit/Xm59tidxPV804zsBlopu?p=preview

@perliedman
Copy link
Member

Hi, thanks for reporting.

I don't have access to the Samsung Internet Browser, so it will be hard for me to verify this, but it sounds like this might be a duplicate of, or at least similar to #5779.

Fortunately, there's #5952 which should address this. If possible, could you try out this fix and see if it works for you?

@cherniavskii
Copy link
Sponsor Collaborator

I don't think it's related to #5952, since it fixes an issue with dragging: true, touchZoom: false and doesn't change behavior of dragging: false

@syno1997
Copy link
Author

@perliedman You can download the Samsung Browser on any Android-device. I verified on my Huawei that its not just the Samsung-Devices but also on other devices using this browser.
I tested to add the CSS-changes you made in the merge you linked and unfortunate as @cherniavskii mentioned, it does not address this issue.

@ajhbh
Copy link

ajhbh commented Jul 6, 2018

I have downloaded the Samsung Browser to my HTC device and confirm the issue is there in that with dragging disabled, 2 finger interactions are also stopped.
I haven't checked myself but from the leaflet gestureHandling addon issue here:

In SamsungBrowser there's always a burst of single fingered interactions immediately before any two fingered zoom. No matter how hard you try to avoid it.
This seems to be intermittently disabling the map interactions mid pinch giving an ugly jumpy effect.

Hopefully this helps point in the right direction for a fix.

@johnd0e
Copy link
Collaborator

johnd0e commented Apr 6, 2020

Looks similar to #5425

@johnd0e
Copy link
Collaborator

johnd0e commented Apr 7, 2020

Still reproducible with latest master.

But can be fixed applying #7029:
https://gist.githack.com/johnd0e/3ad393c73b12b24af7e947711a87b7a4/raw/issue-5969.html

@johnd0e
Copy link
Collaborator

johnd0e commented Apr 7, 2020

This also could be fixed with touch-action: none; rule, but that would prevent normal page scrolling, which is no option here (as we've disabled dragging for the reason, right?)

Just add L.DomUtil.addClass(map._container, 'leaflet-touch-drag');, like dragging does:
https://gist.githack.com/johnd0e/a3647bebc3a65440d2f7ea470998816b/raw/issue-6959-ns.html

This is needed to activate this rule:

Leaflet/dist/leaflet.css

Lines 68 to 71 in d843c3b

.leaflet-container.leaflet-touch-drag.leaflet-touch-zoom {
-ms-touch-action: none;
touch-action: none;
}

@johnd0e
Copy link
Collaborator

johnd0e commented Apr 7, 2020

My explanation: when only touchZoom handler is enabled touch-action: pan-x pan-y; rule is active.

Leaflet/dist/leaflet.css

Lines 58 to 61 in d843c3b

.leaflet-container.leaflet-touch-zoom {
-ms-touch-action: pan-x pan-y;
touch-action: pan-x pan-y;
}

In theory that should prevent browser's native pinch-zoom, thus allowing us to use two-finger gesture on map.

But for unclear reason Samsung browser still applies whole page pinch-zoom.
The problem is we cannot avoid it with preventDefault(), as that seems work only for touch events, but currently all touch events are remapped with their pointer counterparts (#7077).

So I tend to think that proper way to fix this issue - apply #7029.

@johnd0e
Copy link
Collaborator

johnd0e commented Apr 9, 2020

BTW, I found why Samsung browser behaves in such way: the culprit is option Control the zoom on webpages.
Luckily, it's possible to opt out.

Immediately after turning it off touchZoom get working again.
Still, 7026 #7029 makes it working smoother.

@rhysstubbs
Copy link

rhysstubbs commented Oct 20, 2020

Are there any updates on this issue? I am experiencing the same problem in Samsung Internet.

I'm using the Leaflet.GestureHandling plugin and setting touch-action: none !imporant which fixes the issues with the map. However this obviously leaves the map stuck as the native scrolling stops working.

Does #7026 solve the issue and will it be merged anytime soon?

@johnd0e
Copy link
Collaborator

johnd0e commented Oct 21, 2020

I am experiencing the same problem in Samsung Internet.

So why not disable Control the zoom on webpages option in your browser?

@rhysstubbs
Copy link

@johnd0e

So why not disable Control the zoom on webpages option in your browser?

That may solve the issue for me but it wouldn't solve the issue for thousands of my users unfortunately. Samsung Internet makes up ~ 9% of my userbase so its not an option.

@johnd0e
Copy link
Collaborator

johnd0e commented Oct 21, 2020

Good point.

Does #7026 solve the issue and will it be merged anytime soon?

It'd be cool if you test 7026 7029 and report back if it 1) solves your issues 2) does not break anything else for you.

@rhysstubbs
Copy link

@johnd0e

I forked and merged #7026 yesterday, I don't have an Android device to test with unfortunately but one of my colleagues confirmed it didn't fix the issue in Samsung Internet.

Whether or not it breaks anything else I cannot say for certain, in our usecase with raster tiles, zoom +/- and a marker popup, everything seemed to function normally.

@johnd0e
Copy link
Collaborator

johnd0e commented Oct 21, 2020

Well, there is mistake: 7026 originated from my message, perhaps it should be #7029 instead.

Refactor [touch/pointer] events automation moved this from Issues (addressed) to Fixed issues Nov 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging a pull request may close this issue.

6 participants