Merge pull request #273 from OneBusAway/refactor/map-state-simplification

Refactor/map-state-simplification

Author: aaronbrethorst

src/components/MapContainer.svelte

@@ -10,7 +10,7 @@
 	import { MapSource } from './../config/mapSource.js';
 
 	let apiKey = env.PUBLIC_OBA_GOOGLE_MAPS_API_KEY;
-	let { handleStopMarkerSelect, mapProvider = $bindable(), ...restProps } = $props();
+	let { handleStopMarkerSelect, mapProvider = $bindable(), stop, ...restProps } = $props();
 
 	onMount(() => {
 		if (PUBLIC_OBA_MAP_PROVIDER === MapSource.Google) {
@@ -24,7 +24,7 @@
 </script>
 
 {#if mapProvider}
-	<MapView {handleStopMarkerSelect} {mapProvider} {...restProps} />
+	<MapView {handleStopMarkerSelect} {mapProvider} {stop} {...restProps} />
 {:else}
 	<FullPageLoadingSpinner />
 {/if}

src/components/map/MapView.svelte

@@ -10,8 +10,6 @@
 	import LocationButton from '$lib/LocationButton/LocationButton.svelte';
 	import RouteMap from './RouteMap.svelte';
 
-	import { faBus } from '@fortawesome/free-solid-svg-icons';
-	import { RouteType, routePriorities, prioritizedRouteTypeForDisplay } from '$config/routeConfig';
 	import { isMapLoaded } from '$src/stores/mapStore';
 	import { userLocation } from '$src/stores/userLocationStore';
 	/**
@@ -21,16 +19,18 @@
 	 * @property {boolean} [showRoute]
 	 * @property {boolean} [showRouteMap]
 	 * @property {any} [mapProvider]
+	 * @property {any} [stop] - Currently selected stop to preserve visual context
 	 */
 
 	/** @type {Props} */
 	let {
 		handleStopMarkerSelect,
 		selectedTrip = null,
 		selectedRoute = null,
-		showRoute = false,
+		isRouteSelected = false,
 		showRouteMap = false,
-		mapProvider = null
+		mapProvider = null,
+		stop = null
 	} = $props();
 
 	let isTripPlanModeActive = $state(false);
@@ -41,6 +41,45 @@
 	let allStopsMap = new Map();
 	let stopsCache = new Map();
 
+	const Modes = {
+		NORMAL: 'normal',
+		TRIP_PLAN: 'tripPlan',
+		ROUTE: 'route'
+	};
+
+	let mapMode = $state(Modes.NORMAL);
+	let modeChangeTimeout = null;
+
+	$effect(() => {
+		let newMode;
+		if (isTripPlanModeActive) {
+			newMode = Modes.TRIP_PLAN;
+		} else if (selectedRoute || isRouteSelected || showRouteMap || selectedTrip) {
+			newMode = Modes.ROUTE;
+		} else {
+			newMode = Modes.NORMAL;
+		}
+		if (modeChangeTimeout) {
+			clearTimeout(modeChangeTimeout);
+		}
+		if (mapMode === Modes.ROUTE && newMode === Modes.NORMAL) {
+			modeChangeTimeout = setTimeout(() => {
+				mapMode = newMode;
+			}, 100);
+		} else if (mapMode !== newMode) {
+			mapMode = newMode;
+		}
+	});
+
+	$effect(() => {
+		if (!mapInstance) return;
+		if (mapMode === Modes.NORMAL) {
+			batchAddMarkers(allStops);
+		} else {
+			clearAllMarkers();
+		}
+	});
+
 	function cacheKey(zoomLevel, boundingBox) {
 		const multiplier = 100; // 2 decimal places
 		const north = Math.round(boundingBox.north * multiplier);
@@ -103,13 +142,12 @@
 			await loadStopsAndAddMarkers(initialLat, initialLng, true);
 
 			const debouncedLoadMarkers = debounce(async () => {
-				const center = mapInstance.getCenter();
-				const zoomLevel = mapInstance.map.getZoom();
-
-				// Prevent fetching stops in the background when a route is selected or trip plan mode is active, we only fetch stops when we are see other stops
-				if (selectedRoute || showRoute || isTripPlanModeActive) {
+				if (mapMode !== Modes.NORMAL) {
 					return;
 				}
+
+				const center = mapInstance.getCenter();
+				const zoomLevel = mapInstance.map.getZoom();
 				await loadStopsAndAddMarkers(center.lat, center.lng, false, zoomLevel);
 			}, 300);
 
@@ -148,17 +186,13 @@
 		}
 	}
 
-	function updateMarkers() {
-		if (!selectedRoute && !isTripPlanModeActive) {
-			batchAddMarkers(allStops);
-		}
-	}
-
 	// Batch operation to add multiple markers efficiently
 	function batchAddMarkers(stops) {
 		const stopsToAdd = stops.filter((s) => !mapInstance.hasMarker(s.id));
 
-		if (stopsToAdd.length === 0) return;
+		if (stopsToAdd.length === 0) {
+			return;
+		}
 
 		// Group DOM operations to minimize reflows/repaints
 		requestAnimationFrame(() => {
@@ -172,32 +206,17 @@
 			return;
 		}
 
-		// Delegate marker existence check to provider
 		if (mapInstance.hasMarker(s.id)) {
 			return;
 		}
 
-		let icon = faBus;
-
-		if (s.routes && s.routes.length > 0) {
-			const routeTypes = s.routes.map((r) => r.type);
-			let prioritizedType = RouteType.UNKNOWN;
-
-			// Optimized priority lookup - check highest priority first
-			for (const priority of routePriorities) {
-				if (routeTypes.includes(priority)) {
-					prioritizedType = priority;
-					break;
-				}
-			}
-
-			icon = prioritizedRouteTypeForDisplay(prioritizedType);
-		}
+		// Check if this marker should be highlighted (if it's the currently selected stop)
+		const shouldHighlight = stop && s.id === stop.id;
 
 		const markerObj = mapInstance.addMarker({
 			position: { lat: s.lat, lng: s.lon },
-			icon: icon,
 			stop: s,
+			isHighlighted: shouldHighlight,
 			onClick: () => {
 				handleStopMarkerSelect(s);
 			}
@@ -250,31 +269,22 @@
 			if (tabSwitchHandler) window.removeEventListener('tabSwitched', tabSwitchHandler);
 		}
 
+		if (modeChangeTimeout) {
+			clearTimeout(modeChangeTimeout);
+		}
+
 		clearAllMarkers();
 
 		allStopsMap.clear();
 		stopsCache.clear();
 	});
-	$effect(() => {
-		if (selectedRoute) {
-			clearAllMarkers();
-			updateMarkers();
-		} else if (!isTripPlanModeActive) {
-			batchAddMarkers(allStops);
-		}
-	});
-	$effect(() => {
-		if (isTripPlanModeActive) {
-			clearAllMarkers();
-		}
-	});
 </script>
 
 <div class="map-container">
 	<div id="map" bind:this={mapElement}></div>
 
 	{#if selectedTrip && showRouteMap}
-		<RouteMap mapProvider={mapInstance} tripId={selectedTrip.tripId} />
+		<RouteMap mapProvider={mapInstance} tripId={selectedTrip.tripId} currentSelectedStop={stop} />
 	{/if}
 </div>
 

src/components/map/RouteMap.svelte

@@ -2,9 +2,8 @@
 	import { calculateMidpoint } from '$lib/mathUtils';
 	import { clearVehicleMarkersMap, fetchAndUpdateVehicles } from '$lib/vehicleUtils';
 	import { onMount, onDestroy } from 'svelte';
-	let { mapProvider, tripId } = $props();
+	let { mapProvider, tripId, currentSelectedStop = null } = $props();
 	let shapeId = null;
-	let polyline;
 	let tripData = null;
 	let shapeData = null;
 	let isMounted = true;
@@ -23,17 +22,25 @@
 		if (loadRouteDataPromise) {
 			await loadRouteDataPromise;
 		}
+
 		await Promise.all([
-			mapProvider.removePolyline(await polyline),
+			mapProvider.clearAllPolylines(),
 			mapProvider.removeStopMarkers(),
 			mapProvider.cleanupInfoWindow(),
+			mapProvider.clearVehicleMarkers(),
 			clearInterval(currentIntervalId),
-			clearVehicleMarkersMap(mapProvider),
-			mapProvider.clearVehicleMarkers()
+			clearVehicleMarkersMap(mapProvider)
 		]);
+
+		if (currentSelectedStop) {
+			mapProvider.flyTo(currentSelectedStop.lat, currentSelectedStop.lon, 18);
+		}
 	});
 
 	async function loadRouteData() {
+		mapProvider.clearAllPolylines();
+		mapProvider.removeStopMarkers();
+
 		const tripResponse = await fetch(`/api/oba/trip-details/${tripId}`);
 		tripData = await tripResponse.json();
 
@@ -48,7 +55,7 @@
 			shapeData = await shapeResponse.json();
 			const shapePoints = shapeData?.data?.entry?.points;
 			if (shapePoints && isMounted) {
-				polyline = await mapProvider.createPolyline(shapePoints);
+				await mapProvider.createPolyline(shapePoints);
 			}
 		}
 

src/components/oba/TripDetailsPane.svelte

@@ -80,6 +80,7 @@
 
 	onDestroy(() => {
 		clearInterval(interval);
+		interval = null;
 	});
 </script>
 

src/components/search/SearchPane.svelte

@@ -15,7 +15,6 @@
 	import { removeAgencyPrefix } from '$lib/utils';
 
 	let {
-		clearPolylines,
 		handleRouteSelected,
 		handleViewAllRoutes,
 		handleStopMarkerSelect,
@@ -60,6 +59,10 @@
 	}
 
 	async function handleRouteClick(route) {
+		mapProvider.clearAllPolylines();
+		mapProvider.removeStopMarkers();
+		mapProvider.clearVehicleMarkers();
+		clearVehicleMarkersMap(mapProvider);
 		clearResults();
 		try {
 			const response = await fetch(`/api/oba/stops-for-route/${route.id}`);
@@ -108,7 +111,7 @@
 
 	function clearResults() {
 		if (polylines) {
-			clearPolylines();
+			mapProvider.clearAllPolylines();
 		}
 		routes = null;
 		stops = null;

src/components/search/__tests__/SearchPane.test.js

@@ -75,7 +75,10 @@ describe('SearchPane', () => {
 			flyTo: vi.fn(),
 			createPolyline: vi.fn().mockReturnValue('polyline-mock'),
 			addStopRouteMarker: vi.fn(),
-			clearVehicleMarkers: vi.fn()
+			clearVehicleMarkers: vi.fn(),
+			clearAllPolylines: vi.fn(),
+			removeStopMarkers: vi.fn(),
+			addMarker: vi.fn()
 		};
 
 		// Mock props