Allow browser context menu when possible #7369
Conversation
| e.preventDefault(); | ||
| // prevent browser context menu when necessary; we don't allow it with rotation | ||
| // because we can't discern rotation gesture start from contextmenu on Mac | ||
| if (map.dragRotate.isEnabled() || map.listens('contextmenu')) { |
There was a problem hiding this comment.
I understand the first part of this conditional but am I thinking about it correctly that map.listens('contextmenu') would be true if a contextmenu listener has been set up on the map? Why would having a listener prevent the event? It seems like odd behavior that I could disable drag rotate, set up a listener for contextmenu and then the context menu wouldn't actually open.
Also, the docs say that listens should return false if no listener is set up, but I actually get undefined.
There was a problem hiding this comment.
The original reason for having Map contextmenu event is to allow specifying custom behavior on right click, such as opening your own HTML-styled app-specific context menu, and in this case it doesn't make sense to also show the browser default one.
Leaflet does the same thing and no one raised concerns about it yet, so I think it's safe to do the same logic here.
There was a problem hiding this comment.
Ah, ok that makes sense. I tested this on Mac with Safari, Firefox and Chrome and they all worked as expected so I think this is good.
There was a problem hiding this comment.
This is not working in ios WKWebView.
Closes #2301. Enables standard browser context menu on maps where rotation is disabled and there are no
contextmenulisteners.Unfortunately I don't see a way to enable it by default because it's not compatible with rotation — on Mac,
contextmenuis fired together withmousedown(notmouseuplike in other OS), and at this point we don't know if it's a start of a rotation gesture or a simple right click. cc @jplanteLaunch Checklist
write tests for all new functionalitydocument any changes to public APIspost benchmark scores