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

The mouse click for canceling a waypoint propagates to the map level and places a new waypoint #684

Open
jude opened this issue Dec 22, 2022 · 3 comments
Labels

Comments

@jude
Copy link

jude commented Dec 22, 2022

I bumped some node_modules so osrm-frontend would work with vector tiles using https://github.com/maplibre/maplibre-gl-leaflet.

Afterwards, clicking on the waypoint, the closeButton would remove the waypoint, but also propagate down to the map level and end up placing a new waypoint at the location the happened to be under the closeButton.

It may be a while before I have the time to submit a PR, but changing the block at https://github.com/perliedman/leaflet-routing-machine/blob/master/src/geocoder-element.js#L82 to the block below stopped the click propagation for me.

if (closeButton) {
	L.DomEvent.on(closeButton, 'click', function(event) {
			this.fire('delete', { waypoint: this._waypoint });
                        event.stopPropagation();
	}, this);

I didn't do any work to see if it was maplibre-gl-leaflet or one of the other module upgrades that introduced this behavior.

@curtisy1
Copy link
Collaborator

Thanks for the detailed information and suggested fix. I'll also have a look at it and try to find out if anything breaks by not propagating the event.

Do you happen to know what versions of maplibre-gl and leaflet you upgraded to?

@jude
Copy link
Author

jude commented Dec 27, 2022

I bumped the following versions from the dependencies section of https://github.com/Project-OSRM/osrm-frontend/blob/gh-pages/package.json:

leaflet: ~1.9.3
leaflet-control-geocoder: ~2.4.0
leaflet-routing-machine: ~3.2.12
leaflet.locatecontrol: ~0.79
osrm-text-instructions: ~0.14.0
maplibre-gl: ^2.4.0
@maplibre/maplibre-gl-leaflet: 0.0.18

@curtisy1 curtisy1 added the bug label Dec 28, 2022
@curtisy1
Copy link
Collaborator

Interesting. I did some further investigation and it seems like this broke with leaflet 1.9.0. I'm unsure if this is actually expected behavior within leaflet since their releases do not indicate it as a breaking change. However, since it is a DomEvent, I believe propagating it is usually the right thing to do, hence LRM does something wrong here by not explicitly stopping it.

If you can, feel free to submit a PR with the proposed change. From a quick glance, it shouldn't affect any other functionality, so stopping the event from propagating is perfectly fine

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants