Skip to content

Conversation

@Claudio-Chies
Copy link
Contributor

@Claudio-Chies Claudio-Chies commented Aug 21, 2025

Summary

Upgrades MapLibre GL to v5.6.2 and adds automatic country code and elevation detection for map markers.

Key Changes

🗺️ Map Enhancements

  • Updated MapLibre GL: v1.15.1 → v5.6.2
  • Auto-detect country code and elevation when placing/moving markers
  • Improved map style loading with dynamic layer management
  • auto-population of location form

User Impact

  • Better UX: Drop a marker → country code and elevation auto-fill
  • Transparency: Clear privacy disclosure for map services
  • Performance: Updated MapLibre for better rendering
  • This enhances user experience while maintaining privacy transparency and improving the technical foundation.

@socket-security
Copy link

socket-security bot commented Aug 21, 2025

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Updatednpm/​maplibre-gl@​1.15.3 ⏵ 5.6.298100100 +1198 +170

View full report

@Claudio-Chies
Copy link
Contributor Author

This PR fixes #55 and #197

@Claudio-Chies
Copy link
Contributor Author

@dbrgn would be ready for review. i closed the other PR i had as this is a much better implementation.

@Claudio-Chies
Copy link
Contributor Author

@dbrgn
Copy link
Owner

dbrgn commented Aug 21, 2025

Nice, thanks!

Can you rebase against main? I've just merged the branch with many updates.

Without having tested your implementation yet (just based on the video): If an elevation was manually entered, we should probably not override it when moving the marker... What's your view on this?

We could initialize a manuallyModifiedElevation flag when loading the form. When modifying the elevation manually, it will be set to true. Equally, if a number is set when loading the form, set it to true as well. Then only modify the elevation when moving the marker if it's set to false.

@dbrgn
Copy link
Owner

dbrgn commented Aug 22, 2025

Another thing: You used AI tools quite a lot for your PR, right? That is fine in general (I use LLM based tools as well for some tasks), but if you do so, please make sure that it has the same resulting quality as if you yourself wrote the change.

Two examples where I'm unsure about this:

  • In the updated privacy policy, the bulletpoint lists did not actually render as bulletpoints, because the <ul> needs to be a child element of the class="content" element. It seems you did not actually verify the resulting layout? Otherwise it would immediately become obvious that it looks weird.
  • The link in the privacy policy links to the "swisstopo.admin.ch data protection information", but the link (https://www.geo.admin.ch/en/general-terms-of-use-fsdi) does not actually contain such information. The link points to the "General Terms of Use and Operating Conditions of the Federal Spatial Data Infrastructure FSDI", which is relevant for me as an integrator, but not for the end user.

Other examples include a code comment that claims to "Remove existing layers safely". I'm not sure what aspect of that code should be safe, and what would make it non-safe? This looks like a typical LLM-generated slightly nonsensical comment.

PRs with LLM-generated code that was reviewed, verified and tested by you is welcome and appreciated. If not thoroughly tested and line-by-line-reviewed by you, then only with a clear disclaimer (and such PRs might get slower attention by me, because it means more work for me when reviewing).

(In this case, you don't need to change anything about the privacy policy, I'll cherry-pick the commit to a separate PR and will do the necessary adjustments. In general, if you make separate PRs for separate topics, that helps me reviewing and will also speed up the merge process.)

Copy link
Owner

@dbrgn dbrgn left a comment

Choose a reason for hiding this comment

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

Oops, seems I never submitted my review comments...

@Claudio-Chies
Copy link
Contributor Author

Another thing: You used AI tools quite a lot for your PR, right? That is fine in general (I use LLM based tools as well for some tasks), but if you do so, please make sure that it has the same resulting quality as if you yourself wrote the change.

Yea, my background is more in Robotics and coding with C++, with some minimal HTML and CSS quite a while ago. which is why the whole Svelte and co is pretty new to me.
i thought i catched all empty code, but i'll go though it once again.
thanks for the feedback, will implement it in the next few days

@Claudio-Chies Claudio-Chies requested a review from dbrgn September 16, 2025 19:41
@Claudio-Chies
Copy link
Contributor Author

Claudio-Chies commented Sep 16, 2025

@dbrgn took a while to implement the changes, as the exaggeration is linked with the returned elevation. and didn't want to go down the pixel color reading and manual calculation route.
with the new approach it works well with minimal additional code.

@Claudio-Chies
Copy link
Contributor Author

@dbrgn gave it another go, and tried to follow the existing implementation as closely as possible.
what would be needed for you to be able to get this through?

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.

2 participants