diff --git a/app/components/map.gjs b/app/components/map.gjs index 80e2905..d1eb70a 100644 --- a/app/components/map.gjs +++ b/app/components/map.gjs @@ -92,26 +92,19 @@ export default class MapComponent extends Component { // this.storage.rs.on('connected', () => { // this.loadBookmarks(); // }); - - // Listen to changes in the /places/ scope - // keeping this as a backup or for future real-time sync support - this.storage.rs.scope('/places/').on('change', (event) => { - console.log('RemoteStorage change detected:', event); - // this.loadBookmarks(); // Disabling auto-update for now per instructions, using explicit version action instead - this.handleMapMove(); - }); }); // Re-fetch bookmarks when the version changes (triggered by parent action or service) updateBookmarks = modifier(() => { - // Depend on the tracked storage.version - if (this.storage.version >= 0) { - this.handleMapMove(); - } + // Depend on the tracked storage.savedPlaces to automatically update when they change + const places = this.storage.savedPlaces; + this.loadBookmarks(places); }); async loadBookmarks(places = []) { try { + if (!this.bookmarkSource) return; + if (!places || places.length === 0) { // Fallback or explicit check if we have tracked property usage? // The service updates 'savedPlaces'. We should probably use that if we want reactiveness. diff --git a/app/services/storage.js b/app/services/storage.js index d07b6c9..4adbc3b 100644 --- a/app/services/storage.js +++ b/app/services/storage.js @@ -1,9 +1,10 @@ import Service from '@ember/service'; import RemoteStorage from 'remotestoragejs'; import Places from '@remotestorage/module-places'; -import Widget from 'remotestorage-widget'; import { tracked } from '@glimmer/tracking'; import { getGeohashPrefixesInBbox } from '../utils/geohash-coverage'; +import { debounce } from '@ember/runloop'; +import Geohash from 'latlon-geohash'; export default class StorageService extends Service { rs; @@ -22,19 +23,6 @@ export default class StorageService extends Service { this.rs.access.claim('places', 'rw'); // Caching strategy: - // With the new nested structure, enabling caching on root '/' might try to sync everything. - // For now, let's keep it, but we might need to be more selective later if data grows huge. - // Note: The path structure changed from /places/ to just root access in the module? - // In places.ts: getPath returns "ab/cd/id". - // RemoteStorage modules usually implicitly use a base scope based on module name if not defined differently? - // Wait, the module defines `privateClient`. - // When we do `privateClient.storeObject`, it stores it under the module's scope. - // If module name is 'places', then it's stored under /places/. - // So getPath "ab/cd/id" becomes "/places/ab/cd/id". - // So enabling caching on '/places/' is correct. - // However, per instructions, we should set maxAge to false for listings in the module, - // which we did. - // We also need to be careful about what we cache here. this.rs.caching.enable('/places/'); window.remoteStorage = this.rs; @@ -47,25 +35,7 @@ export default class StorageService extends Service { }); this.rs.scope('/places/').on('change', (event) => { - // When data changes remotely or locally, we should ideally re-fetch the affected area. - // However, we don't easily know the bbox of the changed event without parsing paths. - // For now, let's trigger a reload of the *currently loaded* prefixes to ensure consistency. - // Or simpler: just let the manual user interaction drive it? - // No, if a sync happens, we want to see it. - if (this.currentBbox) { - console.log('Reloading loaded prefixes due to change event'); - // Reset loaded prefixes to force a reload of the current view - // Ideally we should just invalidate the specific changed one, but tracking that is harder. - // Or just re-run loadPlacesInBounds which filters? No, because filters exclude "loadedPrefixes". - - // Strategy: - // 1. Calculate required prefixes for current view - const required = getGeohashPrefixesInBbox(this.currentBbox); - // 2. Force load them - this.loadAllPlaces(required); - // Note: we don't update loadedPrefixes here as they are already in the set, - // but we just want to refresh their data. - } + debounce(this, this.reloadCurrentView, 200); }); } @@ -75,7 +45,18 @@ export default class StorageService extends Service { notifyChange() { this.version++; - // this.loadAllPlaces(); + debounce(this, this.reloadCurrentView, 200); + } + + reloadCurrentView() { + if (!this.currentBbox) return; + + // Recalculate prefixes for the current view + const required = getGeohashPrefixesInBbox(this.currentBbox); + console.log('Reloading view due to changes, prefixes:', required); + + // Force load these prefixes (bypassing the 'already loaded' check in loadPlacesInBounds) + this.loadAllPlaces(required); } async loadPlacesInBounds(bbox) { @@ -110,20 +91,28 @@ export default class StorageService extends Service { try { // If prefixes is null, it loads everything (recursive scan). // If prefixes is an array ['w1q7'], it loads just that sector. - const places = await this.rs.places.getPlaces(prefixes); + const places = await this.places.getPlaces(prefixes); if (places && Array.isArray(places)) { if (prefixes) { - // If partial load, we might want to merge instead of replace? - // For now, let's keep the simple behavior: replacing the tracked array triggers updates. - // However, replacing 'all saved places' with 'just these visible places' - // might hide other bookmarks from the list. + // Identify existing places that belong to the reloaded prefixes and remove them + const prefixSet = new Set(prefixes); - // Strategy: maintain a Map of loaded places to avoid duplicates/overwrites? - // Or just append unique ones? - const currentIds = new Set(this.savedPlaces.map((p) => p.id)); - const newPlaces = places.filter((p) => !currentIds.has(p.id)); - this.savedPlaces = [...this.savedPlaces, ...newPlaces]; + const keptPlaces = this.savedPlaces.filter((place) => { + if (!place.lat || !place.lon) return false; + try { + // Calculate 4-char geohash for the existing place + const hash = Geohash.encode(place.lat, place.lon, 4); + // If the hash is in the set of reloaded prefixes, we discard the old version + // (because the 'places' array contains the authoritative new state for this prefix) + return !prefixSet.has(hash); + } catch (e) { + return true; // Keep malformed/unknown places safe + } + }); + + // Merge the kept places (from other areas) with the fresh places (from these areas) + this.savedPlaces = [...keptPlaces, ...places]; } else { // Full reload this.savedPlaces = places;