Skip to content

Commit

Permalink
fix: do not hide contextmenu on movestart
Browse files Browse the repository at this point in the history
If user opens a contextmenu on touch devices by long tap, in most cases
he also moves the map unintentionally. In most cases this does not fire a
`movestart` event. But if the map has `maxBounds` and the map is bounced
back, a `movestart` event is fired. In this case the contextmenu is hidden
immediately after opening, which totally breaks UX.

Hiding contextmenu on `movestart` event may not be needed at all. As far
as I researched there is no scenario in which the user can move the map
intentionally after contextmenu is opened without firing a `mousedown`
or `pointerdown` event before. Therefor contextmenu is already closed
before `movestart` event is fired.

There is only one scenario I noticed in which a `movestart` is fired
while contextmenu is visible. Using mouse as well as using touch a user
is able to move the map while opening a contextmenu. He could hold
right-click or tab and moving map while opening contextmenu. This
triggers a `movestart` only if he is using a mouse. Since this was
inconsitent between mouse and touch input, I would consider it a bug.

Fixes #94
  • Loading branch information
jelhan committed Apr 2, 2018
1 parent 2404757 commit 212a57f
Show file tree
Hide file tree
Showing 3 changed files with 1 addition and 5 deletions.
2 changes: 0 additions & 2 deletions dist/leaflet.contextmenu.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ L.Map.ContextMenu = L.Handler.extend({
this._map.on({
contextmenu: this._show,
mousedown: this._hide,
movestart: this._hide,
zoomstart: this._hide
}, this);
},
Expand All @@ -90,7 +89,6 @@ L.Map.ContextMenu = L.Handler.extend({
this._map.off({
contextmenu: this._show,
mousedown: this._hide,
movestart: this._hide,
zoomstart: this._hide
}, this);
},
Expand Down
2 changes: 1 addition & 1 deletion dist/leaflet.contextmenu.min.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions src/Map.ContextMenu.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ L.Map.ContextMenu = L.Handler.extend({
this._map.on({
contextmenu: this._show,
mousedown: this._hide,
movestart: this._hide,
zoomstart: this._hide
}, this);
},
Expand All @@ -65,7 +64,6 @@ L.Map.ContextMenu = L.Handler.extend({
this._map.off({
contextmenu: this._show,
mousedown: this._hide,
movestart: this._hide,
zoomstart: this._hide
}, this);
},
Expand Down

0 comments on commit 212a57f

Please sign in to comment.