Skip to content

fix-378: Add proper maneuver icons#379

Open
nandalalshukla wants to merge 1 commit intovalhalla:masterfrom
nandalalshukla:fix/add-manuever-icons
Open

fix-378: Add proper maneuver icons#379
nandalalshukla wants to merge 1 commit intovalhalla:masterfrom
nandalalshukla:fix/add-manuever-icons

Conversation

@nandalalshukla
Copy link

🛠️ Fixes Issue

Closes #378

👨‍💻 Changes proposed

Created a function in the get-direction-icon.ts file that return the appropriate icons for a given manuever, handling different edgecases.
Also added the tests for it - all tests passes!

📄 Note to reviewers

Please review the changes in these files,
get-direction-icon.ts
get-direction-icon.spec.ts
maneuvers.tsx

and suggest any improvements if required, i tried my best to match the practices followed in the codebase.

📷 Screenshots

You can see now it show different icons based on the maneuvers :)
image

Copy link
Member

@nilsnolde nilsnolde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm I don't know, this seems very hacky all in all. we should not ever rely on text matching to derive anything for maneuvery. there's maneuver types and they're all we should need in the valhalla format. but as I mentioned in #378 we probably want to look at the osrm output format for guidance instructions.

@nandalalshukla
Copy link
Author

I tried to show icons based on the types, but i found some inconsistencies when showing icons based on types.
Can you please refer me any source when i can learn about valhalla types and then again work on this issue.
I also saw the osrm api output format, here is how it look,
"maneuver": {
"bearing_after": 132,
"bearing_before": 140,
"location": [
85.566025,
27.03034
],
"modifier": "straight",
"type": "new name"
},
i think they show icons based on the modifier, type and bearing angle calculations.
On the other hand here is our api output,
{
"type": 15,
"instruction": "Turn left.",
"verbal_transition_alert_instruction": "Turn left.",
"verbal_succinct_transition_instruction": "Turn left. Then Turn right onto NH37.",
"verbal_pre_transition_instruction": "Turn left. Then Turn right onto NH37.",
"verbal_post_transition_instruction": "Continue for 20 meters.",
"bearing_before": 133,
"bearing_after": 2,
"time": 3.079,
"length": 0.023,
"cost": 12.097,
"begin_shape_index": 2764,
"end_shape_index": 2765,
"verbal_multi_cue": true,
"travel_mode": "bicycle",
"travel_type": "hybrid"
},
if you wish me show the icons based on the type and bearing angle, i can do it. Can you please attach any source where i can learn about valhalla types?

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.

Add proper maneuver icons

2 participants