search

gis-ops / valhalla-app

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

Closes #61 "Route summary reporting error" #111

Closed rakeshhotker closed 1 year ago

rakeshhotker commented 1 year ago

🛠️ Fixes Issue

"Route summary reporting error"

Closes #61

👨‍💻 Changes proposed

1) Created a durationFormatter.js file in utils folder 2) In the durationFormatter.js file created the formatDuration function which can be imported for use anywhere required 3) Importing the formatDuration function in Summary.jsx and Map.jsx to make sure the time displayed is same. Hence fixing the issue

📄 Note to reviewers

Please let me know of any modifications required.

📷 Screenshots

image

aaryahjolia commented 1 year ago

Good one, But I would suggest some changes:

  1. You can change the name of the file containing function. (Instead of helperfunction, you should name it something like 'durationFormatter' which makes it easy to understand what this file contains to other developers)
  2. Instead of using logic of Map.jsx, I think the logic of Summary.jsx is more accurate. See below:

image

For 20 km distance and cycling speed of 10 kph, The time taken should be approx. 2 hour (simple math, summary.jsx is accurate than 7h 38m of map.jsx logic).

  1. You can remove the commented part.

These are my suggestions only :)

rakeshhotker commented 1 year ago

Sure, I'll do the necessary changes, thanks for your inputs.

nilsnolde commented 1 year ago

Jep ok thanks