Fix behavior in which releasing the CTRL-key before mouse button causes map to continue rotating

[averas]

If you rotate a map with the CTRL-key and let go of the CTRL-key before you release the left mouse button the map will keep rotating even after releasing the left mouse button. Clicking on the map will not stop the rotation. The only way to stop the rotation is "grabbing" the map again with the CTRL-key.

This behaviour is quite unintuitive.

[planemad]

Works fine for me on Chrome and Safari. @averas browser details?

[averas]

@planemad Are you on PC/Windows or PC/Linux? I realised this might be related to Mac/OSX touchpad behaviour where CTRL-touchpad click is the same as right click. Would be great if someone else with a Mac could confirm my finding.

I see this behaviour in Chrome and Safari. In Firefox CTRL-click doesn't actually allow rotation as all.

[planemad]

@averas i'm on OSX, using the touchpad

[averas]

Now that's really odd.. Anyone else on Mac (El Capitan) with touchpad?

To add a little more detail:

1. Go to https://www.mapbox.com/mapbox-gl-js/examples/
2. Hold CTRL and click with touchpad on map
3. Release the CTRL button while still holding the touchpad down
4. Release the touchpad

At this point my map continues to rotate and the only way to get it to stop is do grab it again with CTRL-click as described above.

Btw. @planemad is rotation working at all for you in Firefox?

[planemad]

@averas yes, I'm having the issue on https://www.mapbox.com/mapbox-gl-js/examples/ and other examples.

When tested on my own map this problem is not there.

[averas]

👍 Same for me!

[jfirebaugh]

Thanks, that's good intel. My hunch would be that this is only triggered if the map is in an iframe.

[averas]

@jfirebaugh Good thinking. My maps that I initially discovered this behaviour on are not in iframes, however.

[stevage]

Related to #3389?

[rgtaylor1989]

I did some digging into the code on this issue (moved from #4187 to an open bug for discussion)

Mapbox GL Code

The issue is when the user lets go of ctrl and then lets go of the left mouse click, this._ignoreEvent(e) on line 128 returns "true" (https://github.com/mapbox/mapbox-gl-js/blob/master/js/ui/handler/drag_rotate.js#L128). This causes the rest of the _onUp(e) function not to run ... which causes the drag_rotate handler to be stuck on _onMove(e).

The problem is with _ignoreEvent(e) it seems? (https://github.com/mapbox/mapbox-gl-js/blob/master/js/ui/handler/drag_rotate.js#L193).

Is there any reason to have line 128? If removed it fixes this problem. I dont know if it will break anything else though by removing this line.

Different Approach (Maybe?!?!?!?!)

Wouldnt it be better to have a function that can change between the Rotation_Pan and Pan seamlessly? See my logic below and an example code. I am certain the code will not work because I am not that familiar with Mapbox GL ... so perhaps someone who knows more could make it work (if it is even possible).

Rotate Pan Handler
onmousedown --> was ctrl held || e.which == 3 //right click --> If true then run rotate_pan_handler
onmousemove ---> still holding ctrl || e.which == 3 --> if "true" then change the bearing, etc
onmousemove ---> still holding ctrl || e.which == 3 --> if "false" then grab information you need to start changing the bearing from that point and run the function to change the bearing, etc
onmouseup ---> stop changing the map and take off eventlisteners.

Panmap
onmousedown --> was ctrl held || e.which == 3 //right click --> If false then run pan_handler
onmousemove ---> did the user hit ctrl --> if true then grab information you need to start changing the bearing from that point and run the function to change the bearing, etc
onmousemove ---> did the user hit ctrl --> if false then pan the map
onmouseup ---> stop changing the map and take off eventlisteners.

this._el.addEventListener('mousedown', this._onDown);

_onDown(e){

  if(e.ctrlKey || e.which == 3){
      window.document.addEventListener('mousemove', this._onMove_RatPan);
      window.document.addEventListener('mouseup', this._onUp_RatPan);
  } else {
      window.document.addEventListener('mousemove', this._onMove_Pan);
      window.document.addEventListener('mouseup', this._onUp_Pan);
  }


}

_onMove_RatPan(e){

  //Are they still holding the control key or right click
  if(e.ctrlKey || e.which == 3){

      //***Change the map bearing and pitch here***

  //if they let go of the of control
  }else{

      //grab any information you need for the pan here? 

      //remove the event listeners for rotation.
      window.document.removeEventListener('mousemove', this._onMove_RatPan);
      window.document.removeEventListener('mouseup', this._onUp_RatPan);

      //switch to the pan functions
      window.document.addEventListener('mousemove', this._onMove_Pan);
      window.document.addEventListener('mouseup', this._onUp_Pan);
  }

}

_onUp_RatPan(e){
    // finish the rotate
}

_onMove_Pan(){

  //Did the user hit the control key
  if(e.ctrlKey){

      //grab any information you need for the pan rotate here? 

      //remove the event listeners for pan.
      window.document.removeEventListener('mousemove', this._onMove_Pan);
      window.document.removeEventListener('mouseup', this._onUp_Pan);

      //switch to the rotate pan functions
      window.document.addEventListener('mousemove', this._onMove_RatPan);
      window.document.addEventListener('mouseup', this._onUp_RatPan);
      
  }else{

      //***Change the map bearing and pitch here***
    
  }
}

_onUp_Pan(e){
    // finish the pan
}

[jaapster]

I have the same issue in Chrome and Firefox on MasOS. Both in the mapbox examples and in my own app (where the map control is not in an iframe)

[dannybloe]

Yeah, please please fix this bugger. I drives me crazy. We have to release our app next month and I'm considering to disable this feature (with pain in my heart) because I'm sure our customers will be very unhappy due to this.

[joshmaisey]

We also have the same issue on Chrome/Firefox/Safari on Mac OS X El Capitan. Also reproducing the issue on a Windows10 WM.