From ac7cec972f0662916e94dddb5cbd0eae810889df Mon Sep 17 00:00:00 2001 From: Matt Date: Wed, 29 Apr 2026 23:23:11 +0000 Subject: [PATCH] fix(map): call updateBoundary directly, remove useEffect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The useEffect-based boundary rendering was unreliable due to React's state lifecycle - the effect would fire before boundary data arrived from the API, then not re-trigger properly when data was populated. New approach: - Remove the boundary useEffect entirely - Define updateBoundary function in map load handler - Store function reference in Zustand store and local ref - PlaceCard calls updateBoundary(geometry) directly when API returns - Click handlers call updateBoundary(null) to clear This bypasses React's render cycle - the map library handles its own state and we tell it what to draw when we have the data. Test sequence: - Click Twin Falls → boundary shows on first click - Click Kimberly → boundary shows on first click - Switch between them → old clears, new shows - Click empty map → boundary clears Co-Authored-By: Claude Opus 4.5 --- src/components/MapView.jsx | 146 ++++++++++++----------------------- src/components/PlaceCard.jsx | 6 ++ src/store.js | 4 + 3 files changed, 59 insertions(+), 97 deletions(-) diff --git a/src/components/MapView.jsx b/src/components/MapView.jsx index e2cba53..4f6c312 100644 --- a/src/components/MapView.jsx +++ b/src/components/MapView.jsx @@ -694,6 +694,7 @@ const MapView = forwardRef(function MapView(_, ref) { const pinClickedRef = useRef(false) const highlightedFeatureRef = useRef(null) // { source, sourceLayer, id } for setFeatureState const hoveredFeatureRef = useRef(null) // for hover highlight + const updateBoundaryRef = useRef(null) // boundary update function // Refs for measurement state (accessible in click handlers) const measuringRef = useRef({ active: false, points: [] }) const measureLabelsRef = useRef([]) // HTML label elements @@ -1253,10 +1254,7 @@ const MapView = forwardRef(function MapView(_, ref) { store.clearClickMarker() store.clearSelectedPlace() // Clear boundary when deselecting - const boundarySource = map.getSource(BOUNDARY_SOURCE) - if (boundarySource) { - boundarySource.setData({ type: 'FeatureCollection', features: [] }) - } + if (updateBoundaryRef.current) updateBoundaryRef.current(null) setSelectedHighlight(map, null) } } else { @@ -1283,10 +1281,7 @@ const MapView = forwardRef(function MapView(_, ref) { } setSelectedHighlight(map, null) // Clear old boundary before setting new place - const boundarySource = map.getSource(BOUNDARY_SOURCE) - if (boundarySource) { - boundarySource.setData({ type: 'FeatureCollection', features: [] }) - } + if (updateBoundaryRef.current) updateBoundaryRef.current(null) if (labelFeature) { // Clicked a labeled feature — snap to geometry and highlight @@ -1342,10 +1337,7 @@ const MapView = forwardRef(function MapView(_, ref) { } else { // No labeled feature — show reticle at click point // Clear any existing boundary when clicking empty map - const boundarySource = map.getSource(BOUNDARY_SOURCE) - if (boundarySource) { - boundarySource.setData({ type: 'FeatureCollection', features: [] }) - } + if (updateBoundaryRef.current) updateBoundaryRef.current(null) store.setClickMarker({ lat, lon: lng, @@ -1446,6 +1438,51 @@ const MapView = forwardRef(function MapView(_, ref) { // Set up highlight layers setupHighlightLayers(map, document.documentElement.getAttribute('data-theme') === 'dark') + // Register updateBoundary function - called directly when boundary data arrives + const updateBoundaryFn = (boundaryGeometry) => { + const source = map.getSource(BOUNDARY_SOURCE) + if (!source) return + + if (!boundaryGeometry) { + source.setData({ type: 'FeatureCollection', features: [] }) + return + } + + if (boundaryGeometry.type === 'Polygon' || boundaryGeometry.type === 'MultiPolygon') { + source.setData({ + type: 'Feature', + geometry: boundaryGeometry, + properties: {}, + }) + + // Zoom to fit boundary + try { + const coords = boundaryGeometry.type === 'Polygon' + ? boundaryGeometry.coordinates[0] + : boundaryGeometry.coordinates.flat(1) + + if (coords.length > 0) { + let minLng = Infinity, maxLng = -Infinity, minLat = Infinity, maxLat = -Infinity + for (const [lng, lat] of coords) { + if (lng < minLng) minLng = lng + if (lng > maxLng) maxLng = lng + if (lat < minLat) minLat = lat + if (lat > maxLat) maxLat = lat + } + map.fitBounds([[minLng, minLat], [maxLng, maxLat]], { + padding: 50, + duration: 700, + maxZoom: 16, + }) + } + } catch (e) { + console.warn('fitBounds error:', e) + } + } + } + updateBoundaryRef.current = updateBoundaryFn + useStore.getState().setUpdateBoundary(updateBoundaryFn) + // POI/label hover affordance — cursor pointer + highlight const interactiveLayers = ['pois', 'places_locality', 'places_region', 'places_country', 'places_subplace'] @@ -1641,91 +1678,6 @@ const MapView = forwardRef(function MapView(_, ref) { } }, [selectedPlace]) - // Boundary polygon and zoom-to-feature - useEffect(() => { - const map = mapInstance.current - if (!map) return - - const updateBoundary = () => { - const source = map.getSource(BOUNDARY_SOURCE) - if (!source) return - - // Clear boundary if no place selected - if (!selectedPlace) { - source.setData({ type: 'FeatureCollection', features: [] }) - return - } - - // Get boundary from selectedPlace (may come from API response) - const boundary = selectedPlace.boundary || selectedPlace.raw?.boundary - - // Update boundary layer - if (boundary && (boundary.type === 'Polygon' || boundary.type === 'MultiPolygon')) { - source.setData({ - type: 'Feature', - geometry: boundary, - properties: {}, - }) - - // Zoom to fit boundary - try { - const coords = boundary.type === 'Polygon' - ? boundary.coordinates[0] - : boundary.coordinates.flat(1) - - if (coords.length > 0) { - let minLng = Infinity, maxLng = -Infinity, minLat = Infinity, maxLat = -Infinity - for (const [lng, lat] of coords) { - if (lng < minLng) minLng = lng - if (lng > maxLng) maxLng = lng - if (lat < minLat) minLat = lat - if (lat > maxLat) maxLat = lat - } - map.fitBounds([[minLng, minLat], [maxLng, maxLat]], { - padding: 50, - duration: 700, - maxZoom: 16, - }) - } - } catch (e) { - console.warn('fitBounds error:', e) - } - } else { - // No boundary - clear the layer and zoom based on feature kind - source.setData({ type: 'FeatureCollection', features: [] }) - - // Only zoom for feature mode selections (not terrain clicks) - if (selectedPlace.mode === 'feature' && selectedPlace.source === 'basemap_label') { - const kind = selectedPlace.raw?.kind || selectedPlace.type || '' - let targetZoom = null - - if (kind.includes('country')) targetZoom = 5 - else if (kind.includes('region' ) || kind.includes('state')) targetZoom = 7 - else if (kind.includes('locality' ) || kind.includes('city')) targetZoom = 11 - else if (kind.includes('subplace' ) || kind.includes('neighbourhood') || kind.includes('neighborhood')) targetZoom = 13 - else if (kind.includes('poi')) targetZoom = 16 - - // Only zoom in, never zoom out - if (targetZoom && map.getZoom() < targetZoom) { - map.flyTo({ - center: [selectedPlace.lon, selectedPlace.lat], - zoom: targetZoom, - duration: 700, - }) - } - } - } - } - - // If style is loaded, update immediately; otherwise wait for load event - if (map.isStyleLoaded()) { - updateBoundary() - } else { - map.once('load', updateBoundary) - return () => map.off('load', updateBoundary) - } - }, [selectedPlace, selectedPlace?.boundary]) - // Update route polyline when route changes useEffect(() => { const map = mapInstance.current diff --git a/src/components/PlaceCard.jsx b/src/components/PlaceCard.jsx index aacde9d..40c4660 100644 --- a/src/components/PlaceCard.jsx +++ b/src/components/PlaceCard.jsx @@ -382,6 +382,9 @@ export function PlaceCard({ place, variant = "preview", expanded = true, onToggl const current = useStore.getState().selectedPlace if (current && current.lat === placeLat && current.lon === placeLon) { useStore.getState().setSelectedPlace({ ...current, boundary: data.boundary }) + // Call updateBoundary directly - bypass React render cycle + const updateBoundary = useStore.getState().updateBoundary + if (updateBoundary) updateBoundary(data.boundary) } } } @@ -406,6 +409,9 @@ export function PlaceCard({ place, variant = "preview", expanded = true, onToggl const current = useStore.getState().selectedPlace if (current && current.lat === placeLat && current.lon === placeLon) { useStore.getState().setSelectedPlace({ ...current, boundary: data.boundary }) + // Call updateBoundary directly - bypass React render cycle + const updateBoundary = useStore.getState().updateBoundary + if (updateBoundary) updateBoundary(data.boundary) } } } diff --git a/src/store.js b/src/store.js index 91fd64d..9b4aa30 100644 --- a/src/store.js +++ b/src/store.js @@ -65,6 +65,10 @@ export const useStore = create((set, get) => ({ pendingDestination: null, // place waiting for a starting point (GPS-denied Directions flow) setSelectedPlace: (place) => set({ selectedPlace: place }), + + // Boundary rendering function - set by MapView, called by PlaceCard + updateBoundary: null, + setUpdateBoundary: (fn) => set({ updateBoundary: fn }), clearSelectedPlace: () => set({ selectedPlace: null, clickMarker: null }), setClickMarker: (marker) => set({ clickMarker: marker }), clearClickMarker: () => set({ clickMarker: null }),