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

Allow pinpointing of locations on Explore map #583

Merged
merged 1 commit into from
Jul 17, 2024
Merged

Conversation

zarino
Copy link
Member

@zarino zarino commented Jul 16, 2024

Uses the Leaflet GeoSearch plugin, and OSM Nominatim geocoder, to allow users to place a marker on the map, at the requested location.

Jul-16-2024 08-37-31

Handy when you know the name of a place, but not which constituency it’s in!

Part of this work required me to have access to this.map inside a callback function, which revealed the same Vue.js proxying issue we’ve encountered in other projects. The fix is to ensure we “un-proxy” all references this.map with toRaw(this.map).

@zarino zarino requested a review from struan July 16, 2024 07:40
Uses the Leaflet GeoSearch plugin, and OSM Nominatim geocoder, to allow
users to place a marker on the map, at the requested location.

Handy when you know the name of a place, but not which constituency
it’s in!

Once the marker is plotted onto the map, we zoom to it. This revealed
the same Vue.js proxying issue we’ve encountered in other projects.
Namely: when you store a `Leaflet.Map` instance in the Vue.js state
(making it accessible inside your Vue methods as, say, `this.map`),
Vue _actually_ stores a wrapped version of the original instance,
to enable its clever reactive features. This “proxy” object, though,
breaks some internal Leaflet stuff, including `animateZoom` (see
https://stackoverflow.com/questions/65981712), which results in
broken zooming/panning. The recommendation from the Vue.js docs
is to “un-wrap” that proxy object when you need to work  with it,
replacing `this.map` with `toRaw(this.map)`.

I didn’t _need_ to use `toRaw()` in every place that I’ve used it here
(just inside the `geosearch/showlocation` callback would have done,
because that’s the only place we _currently_ zoom to bounds), but I
figure it’s simpler to use it _everywhere_ we refer to `this.map` than
to use it inconsitently and force future maintainers to keep making the
decision as to whether it’s needed or not.
@zarino zarino force-pushed the in-map-location-search branch from 01ae69a to 04656f9 Compare July 16, 2024 13:19
@zarino zarino merged commit 04656f9 into main Jul 17, 2024
5 checks passed
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