stadiamaps / ferrostar

A FOSS navigation SDK built from the ground up for the future
https://stadiamaps.github.io/ferrostar/
Other
179 stars 25 forks source link

Support specifying the waypoints and (optionally) origin upfront in the web component properties #211

Open ianthetechie opened 2 months ago

ianthetechie commented 2 months ago

Many use cases for the web component will be for a prefilled destination or even full route upfront (ex: for a walking tour or navigation within a festival). Allowing the specification of these upfront will make the DX much better.

cu8code commented 1 month ago

@ianthetechie would like to work on this! I am thinking of adding a parameter which will allow the developer to specify a point

<ferrostar-map
      id="ferrostar"
      valhallaEndpointUrl="https://api.stadiamaps.com/route/v1"
      styleUrl="https://tiles.stadiamaps.com/styles/outdoors.json"
      profile="bicycle"
      destination=[1000.00, 24.00]
    ></ferrostar-map>
ianthetechie commented 1 month ago

That would be great @cu8code! And thanks for the high-level example of the interface to brainstorm on! This makes things so much easier as a project maintainer 😍

A few comments:

First, I think the waypoints will need to be specified as an object rather than just a coordinate pair. Most routers that I'm aware of support different kinds of waypoints. The most common ones (which Ferrostar supports today) are leg "break points" and points which are simply passed through with no special instruction. We should support this, so I think destination will need to be renamed waypoints and accept a list of Waypoint objects.

Second, I like the decision to not require an explicit user location for the web component! I think this probably matches what most frontend devs will expect. I think it might make sense to optionally specify the origin location as well, but I haven't thought through what the other expectations are around that.

As I reflect a bit more, maybe this should be a separate task. I think the use cases for specifying the origin are maybe a bit different and are not so designed for interactive navigation but as a overview with an instruction list. But we don't have that yet ;) I'm curious if you have any thoughts on this. In any case, we don't need to figure this out before adding a waypoints parameter.

Finally, let's think a little bit about the ways this could break ;) In the ideal case, the user either already gave permission to access their location. I think the most logical behavior for the component is to wait for a location (there is an async method for this already): https://github.com/stadiamaps/ferrostar/blob/9b445619a5fa1fe8aca7f85093b40be05298b6fa/web/src/location.ts#L121.

I think that will "just work" even when the user grants location access after a delay. But what if they reject? In this case, we don't have quite the sophisticated error handling system as Rust or mobile languages. I think for an initial version we could use JavaScript alert, but a better long-term solution is probably an overlay on top of the map letting the user know that they have to enable geolocation.

cu8code commented 1 month ago

Thanks for explaining! My initial idea was quite different—I realize now I was thinking of something else entirely, which feels a bit off in hindsight

We should support this, so I think destination will need to be renamed waypoints and accept a list of Waypoint objects.

If the developer provides the waypoints, the goal would be to immediately start navigation, showing a path from the user's current location to the nearest waypoint. Does that sound like the right approach

are not so designed for interactive navigation but as a overview with an instruction list

Not sure about the user case. An example would be nice!

I think for an initial version we could use JavaScript alert, but a better long-term solution is probably an overlay on top of the map letting the user know that they have to enable geolocation

We could use an alert, but adding an overlay doesn't seem too complicated. It might be worth creating a separate PR for that.

My current Plan about the PR!

Is there any better solution, would love to know about it!

ianthetechie commented 1 month ago

Thanks for explaining! My initial idea was quite different—I realize now I was thinking of something else entirely, which feels a bit off in hindsight

No worries! And I apologize for not providing a very clearly specified issue description 😅 We tend to write a lot of issues quickly to get them out of our heads in the middle of a discussion, but should be more thorough!

If the developer provides the waypoints, the goal would be to immediately start navigation, showing a path from the user's current location to the nearest waypoint. Does that sound like the right approach

I think so.... maybe there is a boolean toggle to control the auto-start, but that's minor stuff that can be added later. I expect most people will want to auto-start.

are not so designed for interactive navigation but as a overview with an instruction list

Not sure about the user case. An example would be nice!

Sure! Here are a few...

Actually, now that I think about it, with the exception of a classic mapquest style directions page, none of those actually need the instructions list :P Let's table this for now... apologies for any mental wheel spinning on ideas ;) I need to think through this a bit better.

We could use an alert, but adding an overlay doesn't seem too complicated. It might be worth creating a separate PR for that.

👍

I think your plan sounds good! Though I'd say it needs to start navigation, not just a simulation ;) Our demo page is indeed primarily as simulation, but this is something that you really can use live on your phone too! I've probably got 50km of test cycle riding already!