gis-ops / valhalla-app

This is the demo web app running on https://valhalla.openstreetmap.de
https://valhalla.openstreetmap.de
MIT License
164 stars 90 forks source link

add date-time option for easy testing conditional-turn-restriction #200

Closed nvcnvn closed 8 months ago

nvcnvn commented 11 months ago

👨‍💻 Changes proposed

📷 Screenshots

Screenshot 2023-10-25 at 15 37 01

nilsnolde commented 11 months ago

Thanks, that'd be a nice addition which would make the app much more useful. A couple of comments:

nvcnvn commented 11 months ago
  • seems like it's now mandatory to give a date/time? Would be better if it was opt-in, e.g. the default item in the dropdown could be "Anytime" or so and a greyed out date/time field. WDYT?

I just want to clarify the behavior of Valhalla in this case, should "Anytime" (empty date_time) different with "Current" (date_time.type = 0)? Because most of the time we: A don't care about time, B want to test departure/arrival.

nilsnolde commented 11 months ago

Yes it will be different. If you say datetime.type = 0, then it’ll use the current time, ie time will be respected (incl restrictions, but also traffic if it’s loaded). When you don’t want to respect time, then you don’t send the date_time object in the request at all. And I agree, the latter will be the vast majority of requests.

nilsnolde commented 11 months ago

Thanks, I'll try it out soon. I'd also like to have @chrstnbwnkl opinion, who's much closer to react code than myself.

nvcnvn commented 10 months ago

Thanks @nvcnvn , just a couple of comments!

Finally I found some time to update the PR.

nvcnvn commented 9 months ago

Sorry for not getting back to this earlier... Looks like you forgot to check in the renamed datepicker component file?

ah you are right