Replace map component

[kellyi]

Overview

This PR replaces the existing Google Map component with a React Leaflet map using a Google Basemap (via GoogleMutant, via some React wrappers). I believe it has feature parity with the Google Map component version on staging, although the cluster marker sizes may be slightly different.

Connects #706

Notes

Leaflet.markercluster wants a peer dependency of Leaflet 1.3.something. There's an open PR to fix this by adjusting how it declares its peer dependencies, but the yarn install output here will currently report an incorrect peer dependency. Here's the PR:

Leaflet/Leaflet.markercluster#946

The library seems to work regardless of the peer dependency mismatch, however.

Testing Instructions

• get this branch, then ./scripts/update and ./scripts/resetdb and ./scripts/server
• use the map and verify that searching for facilities works as before and the clusters work as before
• using shell_plus, set two facilities to live at Point(0, 0). I did this just to make them easily searchable:

>>> from django.contrib.gis.geos import Point
>>> f = Facility.objects.first()
>>> f.location = Point(0, 0)
>>> f.name = "Hello"
>>> f.save()
>>> f = Facility.objects.last()
>>> f.location = Point(0, 0)
>>> f.name = "Hello world"
>>> f.save()

• search for Hello in the app and verify that you see two facilities at the same point and that the disambiguation thing works the same as it does on staging

Checklist

• fixup! commits have been squashed
• CI passes after rebase
• CHANGELOG.md updated with summary of features or fixes, following Keep a Changelog guidelines

[jwalgran]

Sorry. I was trying to click on the details of the failed PR build and the content moved. I accidentally clicked "ready for review"

[kellyi]

No worries!

[kellyi]

The diff here isn't really meaningful -- this is an entirely new file, but I deleted the prior one and replaced it with the React-Leaflet implementation.

[jwalgran]

Great work switching over to Leaflet. All the interactions are in tact and seem to perform a bit better. I just have a few minor suggestions related to style and zoom behavior.

[jwalgran]

Our popup styling has regressed a bit. Consider adjusting the styles to

• Have the name be left-aligned rather than centered
• Restore the small drop shadow

Before

After

[kellyi]

Changed this in 6ebbd43

[jwalgran]

There is some non-intuitive behavior when selecting a cluster representing overlapping markers. Clicking on the cluster zooms in really far, then clicking a popup item zooms back out.

Consider harmonizing the max zoom level to which we zoom when clicking on a cluster and clicking on a marker.

[kellyi]

I think this is taken care of by b9d1915 as well: we no longer adjust the zoom level on moving toward the details page, so it'll just keep the existing zoom level.

[kellyi]

Ended up solving this in a different way: viz., we only set the map view for a details page if it is an initial page load, navigating directly to the details page.

[jwalgran]

The current zoom behavior makes it a little difficult to click around several nearby points

Consider panning only instead of zoom+pan.

[kellyi]

Handled this (and also continued adjusting the minZoom setup) in b9d1915

[jwalgran]

Consider using the application font and tweaking the span style so that the numbers appear more centered.

diff --git a/src/app/src/components/FacilitiesMap.jsx b/src/app/src/components/FacilitiesMap.jsx
index 8669cc1..ca794f1 100644
--- a/src/app/src/components/FacilitiesMap.jsx
+++ b/src/app/src/components/FacilitiesMap.jsx
@@ -214,9 +214,9 @@ function FacilitiesMap({
                     })();
 
                     return L.divIcon({
-                        className: iconClassName,
+                        className: `cluster-icon ${iconClassName}`,
                         iconSize,
-                        html: `<span>${clusterCount}</span>`,
+                        html: `<span style="margin: -0.1rem; display: block;">${clusterCount}</span>`,
                     });
                 }}
                 onClusterClick={({
diff --git a/src/app/src/styles/css/leafletMap.css b/src/app/src/styles/css/leafletMap.css
index 71c6d98..aea2022 100644
--- a/src/app/src/styles/css/leafletMap.css
+++ b/src/app/src/styles/css/leafletMap.css
@@ -1,3 +1,7 @@
+.cluster-icon {
+    font-family: ff-tisa-sans-web-pro, sans-serif;
+}
+
 .cluster-icon-one {
     width: 53px;
     height: 53px;

Before

After

[kellyi]

Cool -- I added this in b615822

[jwalgran]

Consider 2 as an initial zoom rather than 3. It shows more of the world.

Zoom 3

Zoom 2

[kellyi]

Changed this in fcf2a55

[jwalgran]

Consider setting a minimum zoom of 1

[kellyi]

I set the minZoom to 1 in dd37625

[kellyi]

fd63ec1 adjusts the map panning behavior:

Previously we were setting the view anew each time a new facility details page got pushed onto the stack. Now I've adjusted things so that we only set the view around the facility if user has navigated directly to the details screen without traversing the main facilities search.

We discussed this behavior in Slack and determined that it was the preferred behavior.

[kellyi]

@jwalgran I think this is ready for another look! Apologies for the fixup! fixup! fixup! etc stuff.

[jwalgran]

This is really excellent. A big upgrade in map interactions. I think there is only one thing we need to fix up. Now that we are not zooming on every select, the selection can get out of sync with the map if you select an item from the list that is not in the viewport. I think Google Maps shows the best way of handling this situation, where you don't move the map if you click a result on the map, but do move the map if you click an item in the sidebar.

[kellyi]

where you don't move the map if you click a result on the map, but do move the map if you click an item in the sidebar

That makes sense to me -- I'll add this interaction before merging.

[kellyi]

Going to squash this momentarily but 5e69460 has the changes to make the map panning work like:

• if the selected facility is in the viewport's map bounds when it is selected (technically: when its data returns from the API), do not pan the map
• if the selected facility is not in the viewport's map bounds when it is selected (technically: when its data returns from the API), do pan the map to center it on the facility

I also commented the useEffects so that we'll have an easier time understanding them if we ever, for instance, switch to another mapping library.