From 42bf8455e5b5b008c03fc5af5f24e98d3589c029 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Tue, 20 Jan 2026 16:03:51 +0700 Subject: [PATCH] Improve place routing and loading * Normalize OSM POIs and always use and store the OSM type and tags * Pass place objects to place route, do not load from API if passed * Construct place URLs with osm prefix including the type * Load specific type from API when given --- app/components/map.gjs | 24 ++++++++---------- app/components/places-sidebar.gjs | 17 +++++++------ app/routes/place.js | 42 ++++++++++++++++++++----------- app/services/osm.js | 41 ++++++++++++++++++++++-------- app/templates/application.gjs | 14 ++++------- 5 files changed, 83 insertions(+), 55 deletions(-) diff --git a/app/components/map.gjs b/app/components/map.gjs index 03439c5..e393541 100644 --- a/app/components/map.gjs +++ b/app/components/map.gjs @@ -147,7 +147,7 @@ export default class MapComponent extends Component { clearTimeout(locateTimeout); locateTimeout = null; } - + // Remove listener try { if (locateListenerKey) { @@ -181,7 +181,7 @@ export default class MapComponent extends Component { resolution, coordinates ); - const diameterInMeters = (accuracy || 50) * 2; + const diameterInMeters = (accuracy || 50) * 2; const diameterInPixels = diameterInMeters / pointResolution; this.locationOverlayElement.style.width = `${diameterInPixels}px`; @@ -248,7 +248,7 @@ export default class MapComponent extends Component { // 4. Start Following locateListenerKey = geolocation.on('change:position', zoomToLocation); - + // 5. Set Safety Timeout (10s) locateTimeout = setTimeout(() => { stopLocating(); @@ -425,12 +425,10 @@ export default class MapComponent extends Component { // Sort by distance from click pois = pois .map((p) => { - // Use center lat/lon for ways/relations if available, else lat/lon - const pLat = p.lat || p.center?.lat; - const pLon = p.lon || p.center?.lon; + // p is already normalized by service, so lat/lon are at top level return { ...p, - _distance: pLat && pLon ? getDistance(lat, lon, pLat, pLon) : 9999, + _distance: getDistance(lat, lon, p.lat, p.lon), }; }) .sort((a, b) => a._distance - b._distance); @@ -442,9 +440,9 @@ export default class MapComponent extends Component { // 1. Exact Name Match matchedPlace = pois.find( (p) => - p.tags && - (p.tags.name === selectedFeatureName || - p.tags['name:en'] === selectedFeatureName) + p.osmTags && + (p.osmTags.name === selectedFeatureName || + p.osmTags['name:en'] === selectedFeatureName) ); // 2. If no exact match, look for VERY close (<=20m) and matching type @@ -454,9 +452,9 @@ export default class MapComponent extends Component { // Check type compatibility if available // (visual tile 'class' is often 'cafe', osm tag is 'amenity'='cafe') const pType = - topCandidate.tags.amenity || - topCandidate.tags.shop || - topCandidate.tags.tourism; + topCandidate.osmTags.amenity || + topCandidate.osmTags.shop || + topCandidate.osmTags.tourism; if ( selectedFeatureType && pType && diff --git a/app/components/places-sidebar.gjs b/app/components/places-sidebar.gjs index 84c820c..bc5e0f5 100644 --- a/app/components/places-sidebar.gjs +++ b/app/components/places-sidebar.gjs @@ -172,7 +172,8 @@ export default class PlacesSidebar extends Component { @selectedPlace.osmTags.amenity @selectedPlace.osmTags.shop @selectedPlace.osmTags.tourism - "Point of Interest" + @selectedPlace.osmTags.leisure + @selectedPlace.osmTags.historic }} {{#if @selectedPlace.description}} {{@selectedPlace.description}} @@ -241,15 +242,17 @@ export default class PlacesSidebar extends Component { {{on "click" (fn this.selectPlace place)}} >
{{or - place.tags.name - place.tags.name:en + place.title + place.osmTags.name + place.osmTags.name:en "Unnamed Place" }}
{{or - place.tags.amenity - place.tags.shop - place.tags.tourism - "Point of Interest" + place.osmTags.amenity + place.osmTags.shop + place.osmTags.tourism + place.osmTags.leisure + place.osmTags.historic }}
diff --git a/app/routes/place.js b/app/routes/place.js index 4eb5149..c8abc6e 100644 --- a/app/routes/place.js +++ b/app/routes/place.js @@ -8,6 +8,13 @@ export default class PlaceRoute extends Route { async model(params) { const id = params.place_id; + // Check for explicit OSM prefixes + if (id.startsWith('osm:node:') || id.startsWith('osm:way:')) { + const [, type, osmId] = id.split(':'); + console.log(`Fetching explicit OSM ${type}:`, osmId); + return this.loadOsmPlace(osmId, type); + } + // 1. Try to find in local bookmarks // We rely on the service maintaining the list let bookmark = this.storage.findPlaceById(id); @@ -23,29 +30,34 @@ export default class PlaceRoute extends Route { return bookmark; } - // 2. Fallback: Fetch from OSM + // 2. Fallback: Fetch from OSM (assuming generic ID or old format) console.log('Not in bookmarks, fetching from OSM:', id); + return this.loadOsmPlace(id); + } + + async loadOsmPlace(id, type = null) { try { - const poi = await this.osm.getPoiById(id); + const poi = await this.osm.getPoiById(id, type); if (poi) { console.debug('Found OSM POI:', poi); - // Map to our Place schema so the sidebar understands it - return { - title: poi.tags.name || poi.tags['name:en'] || 'Untitled Place', - lat: poi.lat || poi.center?.lat, - lon: poi.lon || poi.center?.lon, - url: poi.tags.website, - osmId: String(poi.id), - osmTags: poi.tags, // raw tags - osmType: poi.type, // "node" or "way" - description: poi.tags.description, // ensure description maps - // No ID/Geohash/CreatedAt means it's not saved - }; + return poi; } } catch (e) { console.error('Failed to fetch POI', e); } - return null; } + + serialize(model) { + // If the model is a saved bookmark, use its ID + if (model.id) { + return { place_id: model.id }; + } + // If it's an OSM POI, use the explicit format + if (model.osmId && model.osmType) { + return { place_id: `osm:${model.osmType}:${model.osmId}` }; + } + // Fallback + return { place_id: model.osmId }; + } } diff --git a/app/services/osm.js b/app/services/osm.js index 4f5c5c6..1def730 100644 --- a/app/services/osm.js +++ b/app/services/osm.js @@ -31,7 +31,9 @@ out center; const res = await this.fetchWithRetry(url, { signal }); if (!res.ok) throw new Error('Overpass request failed'); const data = await res.json(); - return data.elements; + + // Normalize data + return data.elements.map(this.normalizePoi); } catch (e) { if (e.name === 'AbortError') { console.log('Overpass request aborted'); @@ -41,6 +43,19 @@ out center; } } + normalizePoi(poi) { + return { + title: poi.tags?.name || poi.tags?.['name:en'] || 'Untitled Place', + lat: poi.lat || poi.center?.lat, + lon: poi.lon || poi.center?.lon, + url: poi.tags?.website, + osmId: String(poi.id), + osmType: poi.type, + osmTags: poi.tags || {}, + description: poi.tags?.description, + }; + } + async fetchWithRetry(url, options = {}, retries = 3) { try { const res = await fetch(url, options); @@ -64,22 +79,25 @@ out center; } } - async getPoiById(id) { - // Assuming 'id' is just the numeric ID. - // Overpass needs type(id). But we might not know the type (node, way, relation). - // We can query all types for this ID. - // However, typical usage often passes just the numeric ID. - // A query for just ID(numeric) is tricky without type. - // Let's assume 'node' first or try to query all three types by ID. + async getPoiById(id, type = null) { + // If type is provided, we can be specific. + // If not, we query both node and way. + let query; - const query = ` + if (type === 'node') { + query = `[out:json][timeout:25];node(${id});out center;`; + } else if (type === 'way') { + query = `[out:json][timeout:25];way(${id});out center;`; + } else { + query = ` [out:json][timeout:25]; ( node(${id}); way(${id}); ); out center; - `.trim(); + `.trim(); + } const url = `https://overpass-api.de/api/interpreter?data=${encodeURIComponent( query @@ -87,6 +105,7 @@ out center; const res = await this.fetchWithRetry(url); if (!res.ok) throw new Error('Overpass request failed'); const data = await res.json(); - return data.elements[0]; // Return the first match + if (!data.elements[0]) return null; + return this.normalizePoi(data.elements[0]); } } diff --git a/app/templates/application.gjs b/app/templates/application.gjs index 04cec0e..afcc754 100644 --- a/app/templates/application.gjs +++ b/app/templates/application.gjs @@ -30,11 +30,9 @@ export default class ApplicationComponent extends Component { showPlaces(places, selectedPlace = null) { // If we have a specific place, transition to the route if (selectedPlace) { - // Use ID if available, or osmId - const id = selectedPlace.id || selectedPlace.osmId; - if (id) { - this.router.transitionTo('place', id); - } + // Pass the FULL object model to avoid re-fetching! + // The Route's serialize() hook handles URL generation. + this.router.transitionTo('place', selectedPlace); this.nearbyPlaces = null; // Clear list when selecting specific } else if (places && places.length > 0) { // Show list case @@ -46,10 +44,8 @@ export default class ApplicationComponent extends Component { @action selectFromList(place) { if (place) { - const id = place.id || place.osmId; - if (id) { - this.router.transitionTo('place', id); - } + // Optimize: Pass full object to avoid fetch + this.router.transitionTo('place', place); } }