Conversation
|
🚀 Deployed on https://691e26c39eb1a6864fdbd9fb--opengeos.netlify.app |
There was a problem hiding this comment.
Pull Request Overview
This PR adds OSM (OpenStreetMap) editor functionality that enables users to load transportation infrastructure data from OpenStreetMap into the Geoman editing control for visualization and editing.
Key Changes:
- Implements JavaScript helpers to fetch OSM data via Overpass API with multiple fallback endpoints for reliability
- Adds GeoJSON conversion logic with fallback support when the osmtogeojson library is unavailable
- Provides a Python API method
load_osm_transport_to_geoman()for programmatic control of OSM data loading
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 16 comments.
| File | Description |
|---|---|
| anymap/static/maplibre_widget.js | Implements OSM data fetching, conversion, UI loading overlay, and an OSM Transport button control. Includes both primary implementation and a fallback message handler. |
| anymap/maplibre.py | Adds Python API method to trigger OSM transport data loading with configurable bbox, OSM keys, and timeout parameters. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| (async () => { | ||
| try { | ||
| const b = map.getBounds(); | ||
| const bbox = Array.isArray(opts?.bbox) && opts.bbox.length === 4 | ||
| ? opts.bbox | ||
| : [b.getWest(), b.getSouth(), b.getEast(), b.getNorth()]; | ||
| const keys = Array.isArray(opts?.keys) && opts.keys.length ? opts.keys : ['highway', 'railway']; | ||
| const [w, s, e, n] = bbox; | ||
| const bboxStr = `${s},${w},${n},${e}`; | ||
| const body = [ | ||
| `[out:json][timeout:${opts?.timeout || 25}];`, | ||
| '(', | ||
| ...keys.flatMap((k) => [ | ||
| `node["${k}"](${bboxStr});`, | ||
| `way["${k}"](${bboxStr});`, | ||
| `relation["${k}"](${bboxStr});`, | ||
| ]), | ||
| ');', | ||
| 'out body geom;' | ||
| ].join(''); | ||
| const url = 'https://overpass-api.de/api/interpreter'; | ||
| const resp = await fetch(url, { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/x-www-form-urlencoded; charset=UTF-8' }, | ||
| body: new URLSearchParams({ data: body }).toString(), | ||
| }); | ||
| if (!resp.ok) throw new Error(`Overpass error ${resp.status}`); | ||
| const osmJson = await resp.json(); | ||
| // Try to load converter | ||
| if (typeof window.osmtogeojson !== 'function') { | ||
| const s = document.createElement('script'); | ||
| s.src = 'https://cdn.jsdelivr.net/npm/osmtogeojson@3.0.0/osmtogeojson.min.js'; | ||
| await new Promise((resolve, reject) => { | ||
| s.onload = resolve; | ||
| s.onerror = reject; | ||
| document.head.appendChild(s); | ||
| }); | ||
| } | ||
| const geojson = window.osmtogeojson(osmJson, { flatProperties: true }); | ||
| if (geojson && geojson.type === 'FeatureCollection') { | ||
| importGeomanData(geojson); | ||
| } | ||
| } catch (e) { | ||
| console.warn('Inline OSM transport import failed:', e); | ||
| } | ||
| })(); |
There was a problem hiding this comment.
This fallback implementation duplicates significant logic from the primary implementation (lines 4071-4296). The duplicated code includes:
- Overpass API query construction (lines 8367-8377 vs 4156-4166)
- Fetch request logic (lines 8379-8385 vs 4177-4183)
- osmtogeojson library loading (lines 8388-8394 vs 4071-4099)
Consider extracting the shared logic into reusable helper functions that both code paths can call. This would improve maintainability and ensure consistency when updates are needed.
| return false; | ||
| } | ||
|
|
||
| function convertOsmTransportToGeoJsonLite(osmJson, keys) { |
There was a problem hiding this comment.
The convertOsmTransportToGeoJsonLite function lacks documentation explaining when it's used as a fallback and what OSM data structures it handles. Consider adding a JSDoc comment that describes:
- Its purpose as a fallback converter when osmtogeojson library is unavailable
- The expected structure of the
osmJsonparameter (OSM JSON with elements array) - The
keysparameter and what transport tags it filters by - What geometry types are supported (node→Point, way→LineString, relation→MultiLineString)
| const containerEl = this._geomanInstance?.control?.container; | ||
| if (containerEl) { | ||
| const buttons = Array.from(containerEl.querySelectorAll('.gm-control-button')); | ||
| const getButtonLabel = (b) => ((b.getAttribute('title') || b.getAttribute('aria-label') || (b.textContent ? b.textContent.trim() : '')).toLowerCase()); |
There was a problem hiding this comment.
This line contains complex nested logic with multiple method calls chained together, making it difficult to read and understand. The expression ((b.getAttribute('title') || b.getAttribute('aria-label') || (b.textContent ? b.textContent.trim() : '')).toLowerCase()) could be extracted into a separate helper function or simplified. Consider breaking this into multiple lines or extracting the button label retrieval logic.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
No description provided.