Issue rotate web #790
Issue rotate web #790
Conversation
| @@ -41,6 +38,10 @@ class Convert { | |||
| if (options.containsKey('zoomGesturesEnabled')) { | |||
| sink.setZoomGesturesEnabled(options['zoomGesturesEnabled']); | |||
| } | |||
| if (options.containsKey('rotateGesturesEnabled')) { | |||
| //Should not be invoked before sink.setZoomGesturesEnabled() | |||
| sink.setRotateGesturesEnabled(options['rotateGesturesEnabled']); | |||
There was a problem hiding this comment.
Could setZoomGesturesEnabled be affected by this modification?
There was a problem hiding this comment.
from what I can tell, it will impact all gestures which may use keyboard :
setZoomGesturesEnabled controls:
_map.doubleClickZoom.enable();
_map.boxZoom.enable();
_map.scrollZoom.enable();
_map.touchZoomRotate.enable();
_map.keyboard.enable();
and then setRotateGesturesEnabled controls:
_map.dragRotate.enable();
_map.touchZoomRotate.enableRotation();
_map.keyboard.enable();
The KeyboardHandler in the current implementation seems to not offer a granular option to disable/enable certain keys or combinations of keys.
I think a quick action is to simply call the methods in the Convert in the order of their likelihood:
How often do you need a map that does not zoom but which you can rotate vs. A map which can be zoomed but you cannot rotate
There was a problem hiding this comment.
just to reiterate - after this pr one can:
disable zoom
disable zoom and rotation
but cannot only disable rotation
Is this correct?
There was a problem hiding this comment.
@felix-ht yes, this is correct.
|
are the mobile fails normal due to the fact I only changed web ? |
|
@metafounder not the fails are unrelated - this is a limitation of gitlab actions. They cannot be run on branches on forks. |
|
i thought about this a bit more and using groups to enable map interaction are just a bad idea. As there is some overlap there is always unintended consequences based on the order that is complete obscured for the user |
|
hi @felix-ht , I agree yes that would be ideal. No issues if this gets rejected, it was a quick temporary fix anyway. |
|
i took a look at the native implantation there gestures are grouped as we expose them here - so it does make sense. I wanted to add the option to disable double click to zoom gesture. I could fix this properly in the same pr by removing the separate functions and adding one function that sets all these settings and once and has access to the full state of the gestures. If thats fine with you @metafounder Might have to make the settings non nullable for that, as we might set the groups to a wrong state if one of the settings is null (in an update). |
|
resolved by #851 |
this should fix: rotateGesturesEnabled seems to be ignored for web #777