added the recenter-route feature control file

[s4chinjha]

This Pr Closes #339 and adds a Recenter button to the map.

🛠️ Fixes Issue

As mentioned in issue #339, after Pr #336 disabled automatic map zooming, there was no way to return to the route once you pan or zoom away from it. So i wanted to add a simple control to recenter the map to the current route.

👨‍💻 Changes proposed

• Added the recenter-route button to the map as propose in the issue
• New button in the top-right MapLibre control group below Map Styles
• Used a crosshair icon with the tooltip "Recenter to route"
• Clicking the button fits the entire route back into view.
• Button is hidden when no route is present and reappears automatically when a route is loaded.

📄 Note to reviewers

• I have reused the bounds calculation, padding logic, and fitBounds call from the zoom effect in src/components/map/index.tsx.
• The button was not hiding after resetting waypoints (same root cause as issue Map zooms too agressively to route extents #326) which I fixed by checking successful from directionsStore instead of coordinates, since successful is set to false when clearRoutes() is called.

📷 Screenshots

A img just to show how it looks

I have used AI in assisting me

[ghost]

Preview is ready! 🚀 You can view it here: https://valhalla-app-tests.gis-ops.com/342

[nilsnolde]

before moving forward here: the current control is not a good choice:

1. it's almost indistinguishable from "current user location" control
2. what happens when we request with alternates?

the reasonable place for this zoom tool button is inside the route box in the left side panel. that's where alternates show up too.

[nilsnolde]

also, and I do seriously get tired of saying this (don't worry, you're by far not the only one), please update the PR description according to our guidelines.

[s4chinjha]

also, and I do seriously get tired of saying this (don't worry, you're by far not the only one), please update the PR description according to our guidelines.

Did it in both the Pr, also thanks for the suggestion about moving the button into the route box in the left panel.

I've put together a small design showing how the recenter button could sit inside the route card header.
Figma Design: https://www.figma.com/deck/V1QbYuCPiKC4LOHz8x9bfB

Please, give me your suggestions on this.

[nilsnolde]

personally I'd add it to the right of "show on map". the upper row is reserved for icons of "highway", "toll" etc, see e.g. https://valhalla.openstreetmap.de/directions?profile=car&wps=14.475650054441616%2C47.49714218224054%2C13.892303845869264%2C47.64852631611731&style=carto

[s4chinjha]

personally I'd add it to the right of "show on map". the upper row is reserved for icons of "highway", "toll" etc, see e.g. https://valhalla.openstreetmap.de/directions?profile=car&wps=14.475650054441616%2C47.49714218224054%2C13.892303845869264%2C47.64852631611731&style=carto

https://www.figma.com/deck/V1QbYuCPiKC4LOHz8x9bfB

Out of 1 and 2 which one do you prefer and should i keep the text "Show on Map" like it is rn or should i change that to show only when someone hovers over it.

[nilsnolde]

yeah good point, I think the text can go, the slider button gives enough context by itself. so option 1 is better IMO.

[s4chinjha]

yeah good point, I think the text can go, the slider button gives enough context by itself. so option 1 is better IMO.

Ok then we will go with option 1.

And before moving forward just to give you some context on where I am, I've already built the recenter button as a map feature with the full logic i.e bounding box, padding, fitBounds , and a spec file with 6 passing tests. So the core logic is ready, I just need to move it to the right place.
My plan for now is to remove the recenter button, move it into the route card to the right of "Show on Map", and update the tests to match.
One thing I wanted to ask before I start, should the button work for each alternate route individually when alternates are shown, or is the main route card enough for now?
Also i wanted to ask is the tooltip text okay or should i change that and the crosshair icon is fine or you would prefer something else.

And, if you have anything more to add or things i should look out for while working on this.Please do tell me.

[nilsnolde]

while I appreciate the thought process, I'd prefer if you generally:

• did most decisions by yourself after thinking about the context & impact, unless it's critical (your questions here aren't)
• then implement them
• mark the PR as ready for review

it's a more iterative process, but it shows us which decisions you did (if it's not obvious why you did those decisions, comment on your PR on the relevant lines). also it's more gentle on our time.

[s4chinjha]

while I appreciate the thought process, I'd prefer if you generally:

• did most decisions by yourself after thinking about the context & impact, unless it's critical (your questions here aren't)
• then implement them
• mark the PR as ready for review

it's a more iterative process, but it shows us which decisions you did (if it's not obvious why you did those decisions, comment on your PR on the relevant lines). also it's more gentle on our time.

Understood, will do.

[s4chinjha]

Sorry for the force push, I was cleaning up the branch history to remove the old implementation before committing the new one. I've moved the recenter button into the route card to the right of the "Show on map" toggle as suggested by @nilsnolde.

[nilsnolde]

Sorry for the force push, I was cleaning up the branch history to remove the old implementation before committing the new one.

next time, better use one commit to remove old stuff and then one commit to implement the new one. again, the default for almost all OSS projects worth their salt, is to squash merge, meaning your whole branch will only be a single commit in master/main. so your history is gone anyways, it's only there to help us reviewing. I really meant it when I said "never ever after review". it's not the most awful thing, but please do heed that.

other than that, this looks good! just a nit suggestion: could you just make the new button the same vertical height as the toggle next to it? not sure if this is directly supported by the UI lib (i.e. doesn't need hacks for mobile or so); if it's not, never mind, don't want unstable hacks.

[mustaphaturhan]

I don't think you need to get whole state to check directionsPanelOpen or settingsPanelOpen. also, you are using a hook inside a function; which is not recommended by react at all.

Instead of it, move it top and check the conditions like:

const isDirectionsPanelOpen = useCommonStore((state) => state.directionsPanelOpen)
const isSettingsPanelOpen = useCommonStore((state) => state.settingsPanelOpen)

[mustaphaturhan]

when you are not looking to here, dpOpen and spOpen doesn't give you any clue. we are compiler era now, the code will be minified on production builds. there is no need to create short constant names.

[mustaphaturhan]

You can see that this summary component only being rendered inside directions panel. if there is no panel, then there won't be this button.

the calculations of when panel is visible and not visible is unnecessary.

[mustaphaturhan]

why an array? you can directly write screen.width < 550 ? 50 : spOpen ? 420 : 50.

[mustaphaturhan]

this is always 50.

[mustaphaturhan]

this is always 50.

[mustaphaturhan]

as numbers are unnecessary.