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

Initial Map Loading #668

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
94 changes: 61 additions & 33 deletions client/src/pages/Map/Map.js
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;
Expand Down Expand Up @@ -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,
Copy link
Contributor Author

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

onSuccess: ({ lng, lat, id }) => {
mapboxManager.setCenter({
coords: { lng, lat },

Choose a reason for hiding this comment

The 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
Expand All @@ -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',
Expand Down Expand Up @@ -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 } }) => {
Expand Down Expand Up @@ -240,23 +256,20 @@ export default function Map({
popup.remove();
currentFeature.current = null;
});

mapboxMap.on('moveend', () => {
localStorage.setItem('lastVisitedUrl', window.location.href);
});

setIsMapLoaded(true);

Choose a reason for hiding this comment

The 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;
}
Expand All @@ -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
Expand All @@ -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 />
Expand All @@ -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

Choose a reason for hiding this comment

The 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,
);
}
}
}
5 changes: 4 additions & 1 deletion client/src/pages/Map/MapLayers.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import React, { useEffect } from 'react';

import { useCitiesQuery, useTreemapQuery } from '@/api/queries';
import { REGION_TYPES } from '@/util/constants';
import { treeHealthUtil } from '@/util/treeHealthUtil';

import TreeCountLayer from './TreeCountLayer';
import TreeLayer from './TreeLayer';

Expand Down Expand Up @@ -51,7 +54,7 @@ export default function MapLayers({ map, layers, handlers, currentTreeData }) {
map={map}
minzoom={2}
maxzoom={11}
flyToZoom={12}
regionType={REGION_TYPES.PLACE}
/>

{!map.getLayer('WTTV') &&
Expand Down
26 changes: 14 additions & 12 deletions client/src/pages/Map/MapLayout.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
import React, { useState, useRef, useEffect } from 'react';
import { Box, styled } from '@mui/material';
import React, { useState, useRef, useEffect } from 'react';

import { useTreeQuery } from '@/api/queries';
import { useIsMobile } from '@/pages/NewTree/utilities';
import { UserLocationProvider } from '@/pages/UserLocation/useUserLocation';
import { useNewTree, NewTreeProvider } from '@/pages/NewTree/useNewTree';
import NewTree from '@/pages/NewTree/NewTree';
import Search from '@/pages/Search/Search';
import ErrorBoundary from '@/components/ErrorBoundary/ErrorBoundary';
import PanelDrawer from '@/components/PanelDrawer/PanelDrawer';
import ScrollableDialog from '@/components/ScrollableDialog/ScrollableDialog';
import ErrorBoundary from '@/components/ErrorBoundary/ErrorBoundary';
import MapboxManagerProvider from '@/pages/Map/MapboxManagerProvider';
import NewTree from '@/pages/NewTree/NewTree';
import { useNewTree, NewTreeProvider } from '@/pages/NewTree/useNewTree';
import { useIsMobile } from '@/pages/NewTree/utilities';
import Search from '@/pages/Search/Search';
import Tree from '@/pages/Tree/Tree';
import { UserLocationProvider } from '@/pages/UserLocation/useUserLocation';

import Map from './Map';

const drawerWidth = 350;
Expand All @@ -27,7 +30,6 @@ const MapContainer = styled('main')({
function MapLayout() {
const [currentTreeId, setCurrentTreeId] = useState();
const [currentTreeDataVector, setCurrentTreeDataVector] = useState();
const [map, setMap] = useState(null);
const [mapSelectionEnabled, setMapSelectionEnabled] = useState(true);
const { newTreeState } = useNewTree();
const mapContainerRef = useRef(null);
Expand Down Expand Up @@ -72,11 +74,10 @@ function MapLayout() {
setCurrentTreeDataVector={setCurrentTreeDataVector}
setCurrentTreeId={setCurrentTreeId}
selectionEnabled={mapSelectionEnabled}
onLoad={setMap}
/>
</MapContainer>

<Search map={map} />
<Search />

{newTreeState.isPanelOpen ? (
<NewTree
Expand All @@ -87,7 +88,6 @@ function MapLayout() {
<Tree
TreeDetailsContainer={treeDetailsContainer}
drawerWidth={drawerWidth}
map={map}
currentTreeData={currentTreeData}
currentTreeId={currentTreeId}
setCurrentTreeId={setCurrentTreeId}
Expand All @@ -105,7 +105,9 @@ export default function WrappedMapLayout() {
<ErrorBoundary>
<NewTreeProvider>
<UserLocationProvider>
<MapLayout />
<MapboxManagerProvider>
<MapLayout />
</MapboxManagerProvider>
</UserLocationProvider>
</NewTreeProvider>
</ErrorBoundary>
Expand Down
96 changes: 96 additions & 0 deletions client/src/pages/Map/MapboxManager.js
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;
16 changes: 16 additions & 0 deletions client/src/pages/Map/MapboxManagerProvider.js
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;
Loading