Skip to content

fix: increase fitBounds padding to keep route markers inside viewport#360

Merged
nilsnolde merged 4 commits intovalhalla:masterfrom
Pbhacks:fix/issue-265-map-bounds-padding
Mar 11, 2026
Merged

fix: increase fitBounds padding to keep route markers inside viewport#360
nilsnolde merged 4 commits intovalhalla:masterfrom
Pbhacks:fix/issue-265-map-bounds-padding

Conversation

@Pbhacks
Copy link
Contributor

@Pbhacks Pbhacks commented Mar 8, 2026

Fixes #265

The fitBounds padding was only 50px, which was insufficient for the 45px-tall pin markers, causing end-of-route markers to fall halfway off the screen. Increased padding to 80px on all sides (and 450px when side panels are open) to ensure markers remain fully visible.

🛠️ Fixes Issue

'Closes #265'

📷 Screenshots

image

Fixes valhalla#265

The fitBounds padding was only 50px, which was insufficient for the
45px-tall pin markers, causing end-of-route markers to fall halfway
off the screen. Increased padding to 80px on all sides (and 450px
when side panels are open) to ensure markers remain fully visible.
Copilot AI review requested due to automatic review settings March 8, 2026 13:12
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates map fitBounds padding so route endpoint markers stay fully visible within the viewport, addressing the “final marker clipped at edge” behavior reported in #265.

Changes:

  • Increased default fitBounds padding from 50px to 80px.
  • Increased side-panel padding from 420px to 450px when directions/settings panels are open.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Pbhacks added a commit to Pbhacks/web-app that referenced this pull request Mar 8, 2026
…ding

Addresses Copilot review feedback on PR valhalla#360.
screen.width reflects the physical device screen, not the browser
viewport, so it doesn't respond to window resizes or split-screen.
Using window.innerWidth ensures padding matches the actual viewport.
@ghost
Copy link

ghost commented Mar 8, 2026

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

@nilsnolde
Copy link
Member

nilsnolde commented Mar 8, 2026

looks good, thanks. do note, that if this is gsoc, then this PR won't count unless you follow the gsoc guidelines. also tests are failing.

…ding

Addresses Copilot review feedback on PR valhalla#360.
screen.width reflects the physical device screen, not the browser
viewport, so it doesn't respond to window resizes or split-screen.
Using window.innerWidth ensures padding matches the actual viewport.
@Pbhacks Pbhacks force-pushed the fix/issue-265-map-bounds-padding branch from 7234628 to a18884d Compare March 9, 2026 01:15
@Pbhacks
Copy link
Contributor Author

Pbhacks commented Mar 9, 2026

Yes sir I am eager to participate in your GSOC idea project "
Valhalla - Automated Routing QA: Build a regression testing framework for routing & guidance quality" and would complete all necessary to dos to make sure proper participation

@nilsnolde
Copy link
Member

did you understand that you have to follow the gsoc guidelines on this PR? I won't tell you explicitly what I want to see, that document is too easy to read and understand.

@Pbhacks
Copy link
Contributor Author

Pbhacks commented Mar 11, 2026

Yes sir I'll make the changes on the pr and also make sure the information is accurate and in depth. Also for now the tests have passed. 3/3 checks passed

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.

I'll give you the benefit of the doubt and assume no AI was used here. I won't believe that for your other PR though

@nilsnolde nilsnolde merged commit c4b7140 into valhalla:master Mar 11, 2026
3 checks passed
@Pbhacks
Copy link
Contributor Author

Pbhacks commented Mar 11, 2026

Respected sir @nilsnolde , I appreciate your acceptance on this PR. But I want to clearly state that for this small issue no AI is required when myself I am a strong researcher and been attributed multiple times at top conferences for my research works. Secondly, being a supporter of the good side of Artificial Intelligence, i believe that using it responsibly and with caution can help all of us to reduce a lot of our regular similar works. E.g. drafting the same email for different people again and again and sending each day can be done by an AI agent once template is supplied, similarly if there are some issues in compilation of an android folder into apk for example in react native then we can use AI to make us understand what library or what function we are missing so I have always been advocating for this but nevertheless human brain and pure works are unbeatable. Therefore I appeal for your trust and if needed you can test me.

Looking for YOUR GUIDANCE, which is more precious.

Then no AI would be needed.

@nilsnolde
Copy link
Member

That didn't really help you as much as you might think..

If you were Linux Torvalds himself, it wouldn't matter, the guidelines are crystal clear. You really wanna tell me that your PR description or commit messages weren't AI generated?

fix: increase fitBounds padding to keep route markers inside viewport
72a9798
Fixes #265

The fitBounds padding was only 50px, which was insufficient for the
45px-tall pin markers, causing end-of-route markers to fall halfway
off the screen. Increased padding to 80px on all sides (and 450px
when side panels are open) to ensure markers remain fully visible.

Please..

I can see your first commits on both of those PRs. They're roughly 15 mins apart. Funnily enough the more complex PR was done last, so within around 15 mins max. And that's not AI either then right? Sorry, I pass.

@Pbhacks
Copy link
Contributor Author

Pbhacks commented Mar 12, 2026

That didn't really help you as much as you might think..

If you were Linux Torvalds himself, it wouldn't matter, the guidelines are crystal clear. You really wanna tell me that your PR description or commit messages weren't AI generated?

fix: increase fitBounds padding to keep route markers inside viewport
72a9798
Fixes #265

The fitBounds padding was only 50px, which was insufficient for the
45px-tall pin markers, causing end-of-route markers to fall halfway
off the screen. Increased padding to 80px on all sides (and 450px
when side panels are open) to ensure markers remain fully visible.

Please..

I can see your first commits on both of those PRs. They're roughly 15 mins apart. Funnily enough the more complex PR was done last, so within around 15 mins max. And that's not AI either then right? Sorry, I pass.

Respected sir,

I firmly want to say that I do not disagree that I didn't use AI in the other PR as far as file handling was concerned. As already stated by me in the last comment before yours, as far as the PR is concerned it was written by myself and I take accountability for not following the guideline properly related to the PR.

I hope you are not disappointed by my earlier statement and would rather guide me with your experience. At the end of the day I want to apply my knowledge but with the guidance of the experienced like yourself. And I take all their comments as a learning opportunity. If I make any errors please teach me.

The best part about you was unlike anyone else you rapidly provided your honest statements which I truly give my appreciations.

Regards

@nilsnolde
Copy link
Member

I do not disagree to disagree that you didn't use AI though!

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.

Final point of route ends up on edge of map

3 participants