Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: (#1113) save map view state #1125

Merged
merged 4 commits into from
Apr 29, 2024

Conversation

svkorepanov
Copy link
Contributor

@svkorepanov svkorepanov commented Apr 26, 2024


name: Pull request
about: Create a pull request
title: ''
labels: ''
assignees: ''


What type of PR is this?(check all applicable)

  • refactor
  • feature
  • bug fix
  • documentation
  • optimization
  • other

Description

This pull request addresses #1113, where we want to create a shareable link that we can communicate zoom level and lat long center values.

Related Issues

Issue #1113

What this PR achieves

Screenshots, recordings

ezgif-7-e600230a0a

Notes

Copy link

vercel bot commented Apr 26, 2024

@svkorepanov is attempting to deploy a commit to the openbeta-dev Team on Vercel.

A member of the Team first needs to authorize it.

@musoke
Copy link
Contributor

musoke commented Apr 26, 2024

Would it be possible to format the coordinates in the URL similar to the OSM example? Just so it's easier to read and to copy and paste.

I think it's just the "/" vs "%2F" that is different. A comma might be even better.

Copy link

vercel bot commented Apr 26, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
open-tacos ✅ Ready (Inspect) Visit Preview Apr 29, 2024 4:41pm

}

const cameraInfoToQuery = ({ zoom, center }: CameraInfo): string => {
return `${Math.ceil(zoom)}/${center.lat.toPrecision(3)}/${center.lng.toPrecision(3)}`
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. Done
  2. It is not possible to do that using URLSearchParams, as it automaticaly encodes special characters.
    I probably can prevent that by manually construction the parameter. However, it takes off any benefits that URLSearchParams gives.

Therefore, I believe it will be more beneficial to use URLSearchParams and put up with percent encoded (%2f) symbols in the url.

But if you're ok with keeping '/' slash in the url, you can check out the changes that I made

@@ -121,6 +144,7 @@ export const GlobalMap: React.FC<GlobalMapProps> = ({
onDragStart={() => {
setCursor('move')
}}
onRender={onRender}
Copy link
Contributor

@vnugent vnugent Apr 26, 2024

Choose a reason for hiding this comment

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

I often see onMove() (instead of onRender) used to track current view state. Would it work here?
https://visgl.github.io/react-map-gl/docs/get-started/state-management#controlled-map

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, thank you. I was incorrectly reading documentation for mapbox and missed the fact that you're using maplibre.

Fixed ✅

@svkorepanov
Copy link
Contributor Author

Would it be possible to format the coordinates in the URL similar to the OSM example? Just so it's easier to read and to copy and paste.

I think it's just the "/" vs "%2F" that is different. A comma might be even better.

@musoke Thank you for the comment. I managed to format slash "/" without being percent encoded. Also gave my opinion on that here

@svkorepanov
Copy link
Contributor Author

Updated footage

save-map

setInitialZoom(initialStateFromUrl.zoom)
return
}

getVisitorLocation().then((visitorLocation) => {
if (visitorLocation != null) {
setInitialCenter([visitorLocation.longitude, visitorLocation.latitude])
Copy link
Contributor

Choose a reason for hiding this comment

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

Preview build: https://open-tacos-git-fork-d-k-lippert-feat-save-map-url-openbeta-dev.vercel.app/maps

In the preview build deployed on Vercel.com, visitorLocation will be non-null. I'm getting 'camera=1/0.00000/0.00000' and the map is no longer initialized to my approximate location. Perhaps adding a check 'when initialStateFromUrl == null and visitorLocation != null then set URL param to visitorLocation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed ✅

@vnugent vnugent merged commit fd5d173 into OpenBeta:develop Apr 29, 2024
4 checks passed
@vnugent
Copy link
Contributor

vnugent commented Apr 29, 2024

@all-contributors add @svkorepanov for code

Copy link
Contributor

@vnugent

I've put up a pull request to add @svkorepanov! 🎉

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.

3 participants