Conversation
Removed the static einsatzgebiet.png image and integrated an interactive map using Leaflet.js. Updated HTML and CSS to support the new map, and added a new map.js script to render the map and draw the operational area polygon.
Integrated Leaflet.markercluster for event marker clustering and added custom marker icons for active and completed events. Enhanced marker tooltips and popups, implemented address geocoding with caching, and introduced a reset map control to toggle auto-zoom. Updated CSS for marker labels and improved map user interaction restrictions.
Adds a static firehouse marker with fallback coordinates and dynamic visibility based on zoom. Enhances geocoding by normalizing addresses, bounding searches to Stuttgart, and using structured queries for better accuracy. Ensures firehouse locations are always mapped to the correct coordinates, updates map fitting logic, and improves marker visibility handling.
Moved geocoding and address normalization functions from map.js to a new geo.js module for better separation of concerns and reusability. Updated map.js to use the new Geo namespace and improved map bounds handling. Added geo.js to monitor.html. Also speed up engine and map logic
Introduces persistent caching of geocoding results using localStorage to improve performance and reduce redundant API calls. The cache is limited to 300 entries and is loaded on initialization and saved after updates. Also reduces the geocoding request timeout from 8000ms to 6000ms.
There was a problem hiding this comment.
Pull request overview
This pull request replaces the static map image with an interactive Leaflet-based map that displays real-time incident locations with geocoding capabilities. The implementation adds dynamic marker clustering, auto-zoom functionality, and integrates with OpenStreetMap's Nominatim API for address geocoding.
Changes:
- Added two new JavaScript modules (
geo.jsfor geocoding with caching,map.jsfor map rendering and marker management) - Integrated Leaflet and MarkerCluster libraries with CDN resources
- Updated HTML structure to replace static image with dynamic map container and added necessary script/style references
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 19 comments.
| File | Description |
|---|---|
| src/js/geo.js | Implements geocoding with OpenStreetMap Nominatim API, address normalization, and local caching |
| src/js/map.js | Initializes Leaflet map, manages incident/firehouse markers, handles clustering, and provides auto-zoom control |
| monitor.html | Adds Leaflet/MarkerCluster dependencies and replaces static image with dynamic map container |
| src/css/monitor.css | Updates map container styling and adds custom marker label styles |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| button.style.fontWeight = 'bold'; | ||
| button.style.fontSize = '16px'; | ||
| button.style.color = 'white'; | ||
| button.title = 'Toggle map zoom'; |
There was a problem hiding this comment.
The button title 'Toggle map zoom' is not descriptive of what the button actually does. Based on the implementation, this button toggles auto-zoom on/off. Consider changing the title to something more descriptive like 'Toggle auto-zoom' or 'Enable/disable automatic map zoom' to better communicate its functionality to users.
| button.title = 'Toggle map zoom'; | |
| button.title = 'Enable/disable automatic map zoom'; |
| function saveCache() { | ||
| try { | ||
| // Keep most recent entries up to CACHE_LIMIT | ||
| const entries = Array.from(geocodeCache.entries()).map(([k, v]) => ({ k, v })).slice(-CACHE_LIMIT); | ||
| const obj = {}; | ||
| entries.forEach(({ k, v }) => { obj[k] = v; }); | ||
| localStorage.setItem(CACHE_KEY, JSON.stringify(obj)); | ||
| } catch (e) { | ||
| // ignore write errors (e.g., storage full) | ||
| } | ||
| } |
There was a problem hiding this comment.
The cache eviction strategy uses slice(-CACHE_LIMIT) which keeps the most recent entries based on Map insertion order, but the comment says "Keep most recent entries". However, since entries are added/retrieved without updating their position in the Map, this doesn't actually keep the most recently used entries, only the most recently added ones. Consider implementing an LRU (Least Recently Used) cache by moving accessed entries to the end of the Map when they are retrieved.
| <link rel="stylesheet" href="https://unpkg.com/leaflet.markercluster@1.5.1/dist/MarkerCluster.css" /> | ||
| <link rel="stylesheet" href="https://unpkg.com/leaflet.markercluster@1.5.1/dist/MarkerCluster.Default.css" /> |
There was a problem hiding this comment.
External resources are loaded from CDNs without Subresource Integrity (SRI) hashes for the MarkerCluster CSS files. While Leaflet itself has SRI hashes, the MarkerCluster resources (lines 17-18, 137) do not. Add integrity attributes to these resources to protect against compromised or modified CDN content.
| try { | ||
| const controller = new AbortController(); | ||
| const timeoutId = setTimeout(() => controller.abort(), timeout); | ||
|
|
There was a problem hiding this comment.
The viewbox coordinates '8.85,48.70,9.55,49.05' are hard-coded without documentation. Add a comment explaining what geographic area this viewbox represents (appears to be the Stuttgart region) to improve code maintainability.
| // Viewbox bounding box (minLon,minLat,maxLon,maxLat) roughly covering the Stuttgart region, Germany. |
| @@ -0,0 +1,134 @@ | |||
| (function (window) { | |||
| const FIREHOUSE_ADDRESS = 'Stammheimerstraße 140, Stuttgart'; | |||
| const FIREHOUSE_FALLBACK_COORDS = { lat: 48.7519, lon: 9.1819 }; | |||
There was a problem hiding this comment.
The FIREHOUSE_FALLBACK_COORDS appear to be incorrect. Line 3 defines them as { lat: 48.7519, lon: 9.1819 }, but the map initializes at [48.846141, 9.157327] (line 1 of map.js), which is significantly different (approximately 10.5 km apart). These coordinates should match the actual firehouse location at "Stammheimerstraße 140, Stuttgart". Please verify and correct these coordinates to ensure the firehouse marker appears at the correct location.
| const FIREHOUSE_FALLBACK_COORDS = { lat: 48.7519, lon: 9.1819 }; | |
| const FIREHOUSE_FALLBACK_COORDS = { lat: 48.846141, lon: 9.157327 }; |
| <script src="https://unpkg.com/leaflet@1.9.4/dist/leaflet.js" | ||
| integrity="sha256-20nQCchB9co0qIjJZRGuk2/Z9VM+kNiyxNV1lvTlZBo=" | ||
| crossorigin=""></script> | ||
| <script src="https://unpkg.com/leaflet.markercluster@1.5.1/dist/leaflet.markercluster.js"></script> |
There was a problem hiding this comment.
External resources are loaded from CDNs without Subresource Integrity (SRI) hashes for the MarkerCluster JavaScript file. While Leaflet itself has SRI hashes, the MarkerCluster resource does not. Add an integrity attribute to this resource to protect against compromised or modified CDN content.
| <script src="https://unpkg.com/leaflet.markercluster@1.5.1/dist/leaflet.markercluster.js"></script> | |
| <script src="https://unpkg.com/leaflet.markercluster@1.5.1/dist/leaflet.markercluster.js" | |
| integrity="REPLACE_WITH_REAL_INTEGRITY_HASH_FOR_LEAFLET_MARKERCLUSTER_1_5_1" | |
| crossorigin=""></script> |
| container.style.backgroundColor = '#3388ff'; | ||
| container.style.border = 'none'; | ||
| container.style.borderRadius = '4px'; | ||
| container.style.overflow = 'hidden'; | ||
|
|
||
| const button = L.DomUtil.create('button', '', container); | ||
| button.innerHTML = '⊡'; | ||
| button.style.width = '32px'; | ||
| button.style.height = '32px'; | ||
| button.style.padding = '0'; | ||
| button.style.backgroundColor = '#3388ff'; | ||
| button.style.border = 'none'; | ||
| button.style.cursor = 'pointer'; | ||
| button.style.fontWeight = 'bold'; | ||
| button.style.fontSize = '16px'; | ||
| button.style.color = 'white'; | ||
| button.title = 'Toggle map zoom'; | ||
|
|
||
| button.addEventListener('click', () => { | ||
| autoZoomEnabled = !autoZoomEnabled; | ||
| if (!autoZoomEnabled) { | ||
| map.setView([48.846141, 9.157327], 13); | ||
| button.style.opacity = '0.7'; | ||
| } else { | ||
| button.style.opacity = '1'; | ||
| if (einsatzMarkers.length > 0 || firehouseCoords) { | ||
| const boundsArray = einsatzMarkers.map(m => m.getLatLng()).map(latlng => [latlng.lat, latlng.lng]); | ||
| const firehouseForBounds = firehouseCoords || FIREHOUSE_FALLBACK_COORDS; | ||
| if (firehouseForBounds) boundsArray.push([firehouseForBounds.lat, firehouseForBounds.lon]); | ||
| if (boundsArray.length > 0) { | ||
| const bounds = L.latLngBounds(boundsArray); | ||
| map.fitBounds(bounds, { padding: [38, 38], maxZoom: 19 }); | ||
| } | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| button.addEventListener('mouseover', () => { | ||
| button.style.backgroundColor = '#2b6ec7'; | ||
| }); | ||
|
|
||
| button.addEventListener('mouseout', () => { | ||
| button.style.backgroundColor = '#3388ff'; | ||
| }); |
There was a problem hiding this comment.
Inline styles are being used extensively for the button (lines 270-285, 292, 294, 308, 312) instead of using CSS classes. This violates separation of concerns and makes the code harder to maintain. Consider moving these styles to the CSS file and using class-based styling instead.
| // Simple rotation: spread tooltips in different directions | ||
| // For now, use a hash-based approach to distribute evenly | ||
| const hash = (markerLatlng.lat + markerLatlng.lng).toString().charCodeAt(0); | ||
| return directions[hash % directions.length]; |
There was a problem hiding this comment.
The tooltip direction selection uses a hash based on coordinates that will always produce the same direction for the same location, which defeats the purpose of "spreading tooltips in different directions" as mentioned in the comment. The hash calculation (markerLatlng.lat + markerLatlng.lng).toString().charCodeAt(0) will return the same value every time for the same coordinates. Consider using the marker index or a combination of factors that varies between different markers at the same location.
| // Simple rotation: spread tooltips in different directions | |
| // For now, use a hash-based approach to distribute evenly | |
| const hash = (markerLatlng.lat + markerLatlng.lng).toString().charCodeAt(0); | |
| return directions[hash % directions.length]; | |
| // Spread tooltips in different directions, including for markers at the same location. | |
| // Use a simple random selection so overlapping markers are less likely to overlap tooltips. | |
| const index = Math.floor(Math.random() * directions.length); | |
| return directions[index]; |
| function debouncedUpdateMarkers(einsaetze) { | ||
| clearTimeout(updateDebounceTimer); | ||
| updateDebounceTimer = setTimeout(() => { | ||
| updateEinsatzMarkers(einsaetze); | ||
| }, 300); | ||
| } |
There was a problem hiding this comment.
The debounce timeout of 300ms is a magic number without explanation. Consider defining this as a named constant (e.g., MARKER_UPDATE_DEBOUNCE_MS) to make the code more maintainable and self-documenting.
| }); | ||
|
|
||
| if (key !== lastBoundsKey) { | ||
| map.fitBounds(bounds, { padding: [38, 38], maxZoom: 19 }); |
There was a problem hiding this comment.
The padding [38, 38] and maxZoom 19 values are magic numbers used in multiple places (lines 223, 301) without explanation. Define these as named constants (e.g., MAP_BOUNDS_PADDING and MAX_AUTO_ZOOM) to improve code maintainability and make it easier to adjust these values consistently.
This pull request introduces a new interactive map feature to the monitoring dashboard, replacing the static image with a dynamic Leaflet-based map that displays real-time incident locations. It includes new JavaScript modules for geocoding and map management, integrates marker clustering, and enhances the UI with custom styles for map and markers.
Map Integration and UI Enhancements:
einsatzgebiet.pngimage in themonitor.htmlmap area with a dynamic map using Leaflet, and updates the HTML structure and CSS to support the new map container (#mapboxand#map). [1] [2] [3]monitor.htmlfor interactive map rendering and marker clustering. [1] [2]Geocoding and Location Handling:
src/js/geo.jsmodule that provides address normalization, geocoding with OpenStreetMap Nominatim, local caching of geocode results, and firehouse/fallback location handling.Map Functionality and Marker Management:
src/js/map.jsmodule that initializes the Leaflet map, draws the operational area polygon, manages firehouse and incident markers with clustering, handles geocoding results, and provides a custom control for toggling auto-zoom.Minor and Supporting Changes:
src/js/einsätze.jsis robust (minor whitespace change).These changes collectively provide a more informative and interactive visualization of incidents, improving usability and situational awareness for users.
closed #14