Skip to content

Conversation

@v1r0x
Copy link
Collaborator

@v1r0x v1r0x commented Dec 27, 2016

This is a work in progress PR.

Since the current version relies on an old version of leaflet and a no longer supported tilelayer, @jancborchardt and I decided to rework the whole app and update it to Leaflet 1.x.

This PR should cover the basic features of a maps app.

See #15 (comment) for todo list

cc @nextcloud/maps

@jancborchardt
Copy link
Member

Also cc @eneiluj of GpxPod and GpxEdit because we should work together on making this work directly in the one Maps app. :)

@jancborchardt
Copy link
Member

jancborchardt commented Aug 23, 2017

For the record and to not have it only as one comment in a big issue, here’s the todo list taken from #2 (comment) :)

the core scope is to provide an alternative to things like Google Maps, Apple Maps etc. This includes:

Basic core features:

  • Map display
  • Search
  • All relevant details of a place in a popover
  • Geolocation
  • Favorites, with markers shown pernanently on the map
  • List of POI categories in the sidebar for quick display like in Maps.me (Restaurants, Bars, Wifi, ATM, etc.)
  • Routing (for walking, cycling, driving)

Advanced stuff:

  • Favorites sync with Maps.me
  • Sharing a set of favorites, with another Nextcloud user but also as public link with direct map display
  • Ideally even public transport routing, but I know this is hard
  • Integration with other apps like:
    • Contacts: showing them on the map, and also suggesting them in the search
    • Contacts: inside contacts with an address, show a little map widget for their location
    • Files/Gallery: Show photos in their location based on EXIF location data
    • Calendar: providing the Calendar app with a way to show a little map widget in an event when there is a location set. The other way around, show events which take place in the future and have a location set on that place in the map.
    • Mail: make location descriptions from mails clickable, or also show a small map widget like that
  • In the future GPX track overlay, possibility to edit

@e-alfred
Copy link

Postgresql compatibility should be on the todo list too as mentioned here, if changes are necessary: #19

@v1r0x
Copy link
Collaborator Author

v1r0x commented Sep 24, 2017

Routing (for walking, cycling, driving)

@jancborchardt I tried to add the leaflet-routing-machine to the app, but the builtin routing service (osrm) is "down". The mention Mapbox as alternative, but it requires an API key. Since there is no completely free routing service we should decide which one we want to use. IMO there are three options:

  1. Mapbox (Supports at least car, bike and walking)
  2. Mapzen (Supports at least car, bike, walking and transit; Geocoding, Autocomplete Search)
  3. GraphHopper (Supports at least car, bike and walking)

I think mapzen has the most features and a good free plan. GraphHopper looks good as well. Not sure about the pricing plan of mapbox ("credit" based). Any thoughts?

@e-alfred
Copy link

@v1r0x Mapbox is also used by GNOME Maps since Mapquest shut down their free access. An API key shouldn't be a huge deal, the weather app also requires one to work (pulling data from Openweathermap).

Mapzen sounds surely better because it has more features that are useful for a map app after all. Transit data would be a good addition too.

One important issue I can see would be a good way to make transition to a new backend easier so the Mapquest debacle can be avoided if a provider closes down/cuts off free access.

@jancborchardt
Copy link
Member

@v1r0x ok, let's go for Mapzen then. :)

Btw I think it would be good to merge this as early as possible so the PR doesn't become too big and people can also add improvements on top of the visible master. :)

@jancborchardt
Copy link
Member

(As current master does not work at all anyway.)

@v1r0x
Copy link
Collaborator Author

v1r0x commented Sep 25, 2017

@v1r0x Mapbox is also used by GNOME Maps since Mapquest shut down their free access. An API key shouldn't be a huge deal, the weather app also requires one to work (pulling data from Openweathermap).

That's right. But should we add one (hardcoded) for all users or let them create one?

@jancborchardt
Copy link
Member

Let's say we add a default one for everyone for now. As soon as there's issues we can talk about customization.

@v1r0x
Copy link
Collaborator Author

v1r0x commented Oct 18, 2017

One question regarding the API key. Who should create the API Key? And are there any risks? E.g. abuse.

@e-alfred
Copy link

e-alfred commented Oct 18, 2017

You could do it like in the Weather app that moved this setting into the admin section recently. I do not think that a normal user should be bothered with such a setting after all, only the admin should set it once and everyone can use an app then.

@jancborchardt
Copy link
Member

Yeah, API key should be an admin setting only. And would there be a default (or fair use) to ensure it works on simple install?

In any case, let's merge this branch as soon as possible @v1r0x, ok? :)

@v1r0x
Copy link
Collaborator Author

v1r0x commented Oct 19, 2017

I'm unsure about a private API key as default. I also already have a private mapzen key, which I need for some private projects. What do you think about a "official" nextcloud key which is the default? Is this even possible? @jancborchardt

@jancborchardt
Copy link
Member

How much could be used under that plan? Alternatively we could mail them about it and ask? cc @jospoortvliet

Also cc @freenerd who works at Mapbox, and @zeenix @mlundblad who work on GNOME Maps. How did you do it with the API key, and is there any way we could do it similarly with Nextcloud Maps?

Because we want to integrate Maps into Calendar, Contacts etc as well, so it makes sense to have a proper solution which works out of the box and doesn't need a technical person get an API key. :)

@v1r0x
Copy link
Collaborator Author

v1r0x commented Oct 21, 2017

How much could be used under that plan?

screenshot-2017-10-21 mapzen flex mapzen

@jancborchardt
Copy link
Member

@v1r0x mind emailing both Mapzen and Mapbox to inquire if we can partner with them in some way?

@freenerd @zeenix @mlundblad any input how you handle it with GNOME Maps is appreciated :)

@v1r0x
Copy link
Collaborator Author

v1r0x commented Oct 21, 2017

@jancborchardt No problem. I'll cc you in the mail and attach this issue. Ok?

@jancborchardt
Copy link
Member

@v1r0x yep, great! :)

@e-alfred
Copy link

I built the app and noticed a few thing that could be improved:

  • Search should show a drop down menu with matching items (if multiple results are found)
  • If geolocation is used a popover should be displayed for the location identical to an item found using the search
  • As you use leaflet now, selecting different types of maps (satellite, terrain, routes for example) would be great (like in GpxPod)

@e-alfred
Copy link

e-alfred commented Jan 3, 2018

Game over for Mapzen, it will close down at the end of January:

https://mapzen.com/blog/migration/

I hope not a lot of work was done until now because it would be wasted once again. This also shows that the dependency on external services is extremely bad after all. Striving for self reliance or at least some kind of fallback sould be seriously considered, I think.

Considering that this app uses Leaflet already, there is a routing engine available too:

http://www.liedman.net/leaflet-routing-machine/

It uses OSRM by default:

http://project-osrm.org/

@v1r0x
Copy link
Collaborator Author

v1r0x commented Jan 3, 2018

I tried to talk to mapzen devs, but didn't work out. So, not much work was done.
The leaflet-routing-machine is used (afair) in the old version and had a look at it for the rework, but afair it was very limited with plain osm. Thus, I decided to switch to something else. In general osm is very limited.
Maybe osrm is worth looking at it.

@e-alfred
Copy link

e-alfred commented Apr 1, 2019

Seems like @eneiluj is now working on this app! @eneiluj Do you have some release we can test out? That would be awesome!

@julien-nc
Copy link
Member

@e-alfred You can try rework-eneiluj branch. It's functional. I've written what's been done so far in #32. If we all agree, we (@tacruc and me) could work on rework branch and continue discussing in this PR thread instead of #32.

@jancborchardt
Copy link
Member

@eneiluj awesome! Could you open a pull request based on that branch, then we can use that to discuss and collaborate. :)

@julien-nc
Copy link
Member

@jancborchardt Should I make a PR to master or to rework ?

@jancborchardt
Copy link
Member

@eneiluj since it’s based on rework but rework has been inactive, I think making the pull request to master is best. (cc @v1r0x just FYI)

@julien-nc
Copy link
Member

@jancborchardt Here comes #44. Maps app is back on track 😉.

@julien-nc julien-nc merged commit 7083d8b into master Apr 23, 2019
@jancborchardt jancborchardt deleted the rework branch April 23, 2019 14:14
@jancborchardt jancborchardt added this to the 📜 1.0 - the basics milestone Aug 27, 2019
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.

5 participants