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

Update leaflet to 1.9.4 #824

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

gorgobacka
Copy link
Contributor

This is a starting point to update Leaflet to the most recent version (solves #792).

I managed to fix the most obvious issue with the no-go-area. They are usable.

However, there is still one exception on the browser console that I couldn't solve so far.
leaflet-nogo-areas

Maybe someone else is able to fix it.

@mjaschen
Copy link
Contributor

I think I've figured it out and made a pull request in your repository.

It'd be nice if you can verify that it works now and update this PR.

gorgobacka#1

@gorgobacka
Copy link
Contributor Author

@mjaschen found the missing piece. 👍

This PR is ready now.

@gorgobacka gorgobacka marked this pull request as ready for review October 13, 2024 21:31
@mjaschen
Copy link
Contributor

Unfortunately there's a problem which breaks the “No Go Area” feature on touch devices.

tap and hold to create a circle doesn't work.

I've spent some time on the weekend on the search for a fix and didn't found one. But at least I've a workaround which allows placing fixed size no go areas with a single tap on touch devices, so it's usable at least.

Also, the routing library issues a call to DomEvent.fakeStop() when tapping the route, but that method no longer exists. That's another thing which has to be fixed before merging this PR. My workaround for now is to simply remove that method call: mjaschen/leaflet-routing@0690d5e

With these two workarounds, BRouter-Web works fine with Leaflet 1.9.4 – as far as I've tested it.

If I don't find any further bugs, I'll deploy it in the next days to bikerouter.de and create pull requests here. Maybe someone has an idea how to create proper bugfixes instead of workarounds.

@bagage
Copy link
Collaborator

bagage commented Oct 14, 2024

I don't have much time to test but I am more than happy to merge this PR when it's good enough to go live :) It'd be nice to fix the detected issues, but if it's really hard I think we can assess if that's really important (the nogo circle workaround seems good enough to me for instance).

If there are other testers available to give it a try and report issues, it would greatly help! Let us know @mjaschen when your instance is live with this branch so that it's easier to test it :)

And many thanks for the PR and follow-up!

@mjaschen
Copy link
Contributor

A version with Leaflet 1.9.4 was deployed to https://bikerouter.de

It contains this pull request with the following additional changes:

  • “No Go Areas“ are placed with a single tap on touch devices; behavior on devices with mouse pointer is unchanged (click + drag). The tooltip adjusts accordingly. The initial size of the circle depends on the viewport width.
    • Todo: figure out how to use tap + drag on iOS touch devices
  • Leaflet-Routing library (diff):
    • replace fakeStop() call on DomEvent as this method no longer exists
    • force showing the trailing line in draw mode after the most recent waypoint was deleted
    • Todo: check if there's an easier/cleaner way to achieve this

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

Successfully merging this pull request may close these issues.

3 participants