-
Notifications
You must be signed in to change notification settings - Fork 24
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
Initial Map Loading #668
base: main
Are you sure you want to change the base?
Initial Map Loading #668
Changes from all commits
e8cab83
c04d800
a2cf808
094ade0
9a9d14c
a037d6a
194a570
c4cdfde
87d1ef2
a1ad871
dca3293
dc21b9a
73bd52a
fe8662e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,16 +1,21 @@ | ||
import React, { useState, useEffect, useRef } from 'react'; | ||
import mapboxgl from 'mapbox-gl'; | ||
import { Alert, Box, Typography } from '@mui/material'; | ||
import mapboxgl from 'mapbox-gl'; | ||
import React, { useContext, useEffect, useRef, useState } from 'react'; | ||
|
||
import { useTreeQuery } from '@/api/queries'; | ||
import { mapboxAccessToken } from '@/util/config'; | ||
import Adopt from '@/pages/Adopt/Adopt'; | ||
import DonateBtn from '../../components/Button/DonateBtn/DonateBtn'; | ||
import GeolocateControl from '@/pages/UserLocation/GeolocateControl'; | ||
import { MapboxManagerContext } from '@/pages/Map/MapboxManagerProvider'; | ||
import NewTreeButton from '@/pages/NewTree/NewTreeButton'; | ||
import GeolocateControl from '@/pages/UserLocation/GeolocateControl'; | ||
import { mapboxAccessToken } from '@/util/config'; | ||
import { REGION_TYPES } from '@/util/constants'; | ||
import { treeHealthUtil } from '@/util/treeHealthUtil'; | ||
import MapboxControlPortal from './MapboxControlPortal'; | ||
|
||
import DonateBtn from '../../components/Button/DonateBtn/DonateBtn'; | ||
import MapLayers from './MapLayers'; | ||
import MapboxControlPortal from './MapboxControlPortal'; | ||
import TreeLayerLegend from './TreeLayerLegend'; | ||
|
||
import './Map.css'; | ||
|
||
mapboxgl.accessToken = mapboxAccessToken; | ||
|
@@ -79,22 +84,34 @@ export default function Map({ | |
currentTreeDb, | ||
setCurrentTreeId, | ||
selectionEnabled, | ||
onLoad, | ||
}) { | ||
const [map, setMap] = useState(null); | ||
const mapboxManager = useContext(MapboxManagerContext); | ||
const [isMapLoaded, setIsMapLoaded] = useState(false); | ||
const selectionEnabledRef = useRef(selectionEnabled); | ||
const currentFeature = useRef(null); | ||
const hasInitialFlyToFired = useRef(false); | ||
|
||
const initialLoadTreeId = useRef( | ||
new URLSearchParams(window.location.hash.slice(1)).get('id'), | ||
); | ||
|
||
// This tree query is intentionally separate from the one in `MapLayout`, | ||
// since it kicks off only once on load! | ||
const { data: initialLoadTreeData } = useTreeQuery( | ||
{ id: initialLoadTreeId.current }, | ||
{ retry: 0, enabled: isMapLoaded && !!initialLoadTreeId }, | ||
useTreeQuery( | ||
{ | ||
id: initialLoadTreeId.current, | ||
}, | ||
{ | ||
retry: 0, | ||
enabled: initialLoadTreeId.current != null, | ||
onSuccess: ({ lng, lat, id }) => { | ||
mapboxManager.setCenter({ | ||
coords: { lng, lat }, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The TODO comment should be addressed or removed if it's no longer relevant. |
||
regionType: REGION_TYPES.LATLONG, | ||
}); | ||
setCurrentTreeId(id); | ||
}, | ||
}, | ||
); | ||
|
||
// TODO: maybe use a class for Map so that event handlers bound to the instance can look at | ||
|
@@ -112,6 +129,8 @@ export default function Map({ | |
|
||
useEffect(() => { | ||
if (isMapboxSupported && !map && containerRef.current) { | ||
setInitialMapViewport(initialLoadTreeId); | ||
|
||
const mapboxMap = new mapboxgl.Map({ | ||
container: containerRef.current, | ||
projection: 'globe', | ||
|
@@ -153,9 +172,6 @@ export default function Map({ | |
url: 'mapbox://waterthetrees.open-trees', | ||
}); | ||
|
||
onLoad(mapboxMap); | ||
setIsMapLoaded(true); | ||
|
||
// Wait until the map is loaded to add mouse event handlers so that we don't try to query | ||
// layers when the mouse moves before they've been added and loaded. | ||
mapboxMap.on('click', ({ point: { x, y } }) => { | ||
|
@@ -240,23 +256,20 @@ export default function Map({ | |
popup.remove(); | ||
currentFeature.current = null; | ||
}); | ||
|
||
mapboxMap.on('moveend', () => { | ||
localStorage.setItem('lastVisitedUrl', window.location.href); | ||
}); | ||
|
||
setIsMapLoaded(true); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider wrapping setIsMapLoaded(true) in a separate useEffect with proper dependencies to avoid potential issues with the component lifecycle. |
||
}); | ||
|
||
// Eventually, we only want to set the mapboxManager's map and not save the mapbox map itself in state | ||
setMap(mapboxMap); | ||
mapboxManager.setMap(mapboxMap); | ||
} | ||
}, [map, containerRef]); | ||
|
||
useEffect(() => { | ||
if (!isMapLoaded || hasInitialFlyToFired.current) { | ||
return; | ||
} | ||
if (initialLoadTreeData) { | ||
const { lng, lat, id } = initialLoadTreeData; | ||
flyTo({ map, lng, lat, hasInitialFlyToFired }); | ||
setCurrentTreeId(id); | ||
} | ||
}, [isMapLoaded, map, initialLoadTreeData, setCurrentTreeId]); | ||
|
||
if (!isMapboxSupported) { | ||
return unsupportedError; | ||
} | ||
|
@@ -270,7 +283,7 @@ export default function Map({ | |
currentTreeData={currentTreeData} | ||
/> | ||
<MapboxControlPortal map={map} position="bottom-right"> | ||
<NewTreeButton map={map} /> | ||
<NewTreeButton /> | ||
</MapboxControlPortal> | ||
<MapboxControlPortal map={map} position="bottom-right"> | ||
<TreeLayerLegend | ||
|
@@ -281,7 +294,7 @@ export default function Map({ | |
/> | ||
</MapboxControlPortal> | ||
<MapboxControlPortal map={map} position="top-right"> | ||
<GeolocateControl map={map} /> | ||
<GeolocateControl /> | ||
</MapboxControlPortal> | ||
<MapboxControlPortal map={map} position="bottom-right"> | ||
<Adopt /> | ||
|
@@ -294,10 +307,25 @@ export default function Map({ | |
); | ||
} | ||
|
||
function flyTo({ map, lng, lat, hasInitialFlyToFired }) { | ||
map.flyTo({ | ||
center: [lng, lat], | ||
zoom: 15, | ||
}); | ||
hasInitialFlyToFired.current = true; | ||
// 1. If URL contains a valid id hash param, map ignores the pos in favor of coordinates of tree with id | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider checking if 'lastVisitedUrl' is a valid URL before assigning it to 'window.location'. |
||
// 2. Else if URL contains valid pos hash param, map loads in with passed in LatLng and zoom | ||
// 3. Otherwise, center map around the user’s last map url params if available | ||
// See https://docs.google.com/document/d/1MSSbkCo77VCE9tHZdWKAr8lfrBpRF_1AEMptInFjUBI/edit# | ||
function setInitialMapViewport(initialLoadTreeId) { | ||
const mapPos = new URLSearchParams(window.location.hash.slice(1)).get('pos'); | ||
|
||
if ( | ||
initialLoadTreeId.current == null && | ||
mapPos == null && | ||
localStorage.getItem('lastVisitedUrl') | ||
) { | ||
try { | ||
window.location = localStorage.getItem('lastVisitedUrl'); | ||
} catch (e) { | ||
console.error( | ||
'Error when attempting to retrieve item from local storage', | ||
e, | ||
); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,96 @@ | ||
import { REGION_TYPES } from '../../util/constants'; | ||
|
||
class MapboxManager { | ||
// @param {mapboxgl.Map} markerRef: a mapboxgl.Map() object | ||
// See https://docs.mapbox.com/mapbox-gl-js/api/map/ | ||
// map is expected to be nullable initially while map.js creates the mapboxgl.Map() object. | ||
// Mostly serves as a way to pass in a mock in tests | ||
constructor(map) { | ||
this.map = map; | ||
this.mapLoadPromise = new Promise((resolve) => { | ||
this.onMapLoad = resolve; | ||
}); | ||
} | ||
|
||
setMap(map) { | ||
this.map = map; | ||
this.onMapLoad(); | ||
} | ||
|
||
getCenter() { | ||
if (!this.map) { | ||
return; | ||
} | ||
|
||
return this.map.getCenter(); | ||
} | ||
|
||
// @param {LngLatLike} coords: See https://docs.mapbox.com/mapbox-gl-js/api/geography/#lnglatlike | ||
// (LngLat | {lng: number, lat: number} | {lon: number, lat: number} | [number, number]) | ||
async setCenter({ coords, regionType }) { | ||
if (!coords) { | ||
throw new Error('No coordinates specified in setCenter'); | ||
} | ||
|
||
await this.await_map_loaded(); | ||
|
||
// We currently just take the first element in place_type if array | ||
// FIXME: We should choose either the smallest or largest region type instead | ||
if (Array.isArray(regionType)) { | ||
regionType = regionType[0]; | ||
} | ||
|
||
const zoomLevel = this._getZoom(regionType); | ||
if (zoomLevel) { | ||
this.map.flyTo({ | ||
center: coords, | ||
zoom: zoomLevel, | ||
}); | ||
} else { | ||
this.map.flyTo({ | ||
center: coords, | ||
}); | ||
} | ||
} | ||
|
||
_getZoom(regionType) { | ||
switch (regionType) { | ||
case REGION_TYPES.COUNTRY: | ||
return 4.5; | ||
case REGION_TYPES.REGION: | ||
return 6; | ||
case REGION_TYPES.PLACE: | ||
case REGION_TYPES.DISTRICT: | ||
case REGION_TYPES.POSTAL_CODE: | ||
return 12; | ||
case REGION_TYPES.NEIGHBORHOOD: | ||
case REGION_TYPES.LOCALITY: | ||
return 14; | ||
case REGION_TYPES.ADDRESS: | ||
case REGION_TYPES.LATLONG: | ||
case REGION_TYPES.POI: | ||
return 17; | ||
default: | ||
return null; | ||
} | ||
} | ||
|
||
// @param {RefObject} markerRef: ref to a mapboxgl.Marker() object | ||
// See https://docs.mapbox.com/mapbox-gl-js/api/markers/#marker#addto | ||
async addMarkerRefToMap(markerRef) { | ||
await this.await_map_loaded(); | ||
markerRef.current.addTo(this.map); | ||
} | ||
|
||
async await_map_loaded() { | ||
if (!this.map) { | ||
await this.mapLoadPromise; | ||
} | ||
|
||
if (!this.map) { | ||
throw new Error("Something went wrong. Couldn't load map."); | ||
} | ||
} | ||
} | ||
|
||
export default MapboxManager; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
import MapboxManager from './MapboxManager'; | ||
import React, { createContext } from 'react'; | ||
|
||
export const MapboxManagerContext = createContext(undefined); | ||
|
||
const mapboxManager = new MapboxManager(); | ||
|
||
const MapboxManagerProvider = ({ children }) => { | ||
return ( | ||
<MapboxManagerContext.Provider value={mapboxManager}> | ||
{children} | ||
</MapboxManagerContext.Provider> | ||
); | ||
}; | ||
|
||
export default MapboxManagerProvider; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah crap, just realized there's a bug in here in that this query will be repeated after the default cache/stale time runs out. If that happens, the map will be recentered at seemingly random times.
Maybe an alternative is having a hook for doing something just once after all conditions pass lol. Seems hacky but better than having a bunch of
isInitiated
flags laying around