(Re)load places on change properly

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
+    // Depend on the tracked storage.savedPlaces to automatically update when they change
-    if (this.storage.version >= 0) {
+    const places = this.storage.savedPlaces;
-      this.handleMapMove();
+    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.

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.
+      debounce(this, this.reloadCurrentView, 200);
-      // 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.
-      }
     });
   }
 
@@ -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?
+          // Identify existing places that belong to the reloaded prefixes and remove them
-          // For now, let's keep the simple behavior: replacing the tracked array triggers updates.
+          const prefixSet = new Set(prefixes);
-          // However, replacing 'all saved places' with 'just these visible places'
-          // might hide other bookmarks from the list.
 
-          // Strategy: maintain a Map of loaded places to avoid duplicates/overwrites?
+          const keptPlaces = this.savedPlaces.filter((place) => {
-          // Or just append unique ones?
+            if (!place.lat || !place.lon) return false;
-          const currentIds = new Set(this.savedPlaces.map((p) => p.id));
+            try {
-          const newPlaces = places.filter((p) => !currentIds.has(p.id));
+              // Calculate 4-char geohash for the existing place
-          this.savedPlaces = [...this.savedPlaces, ...newPlaces];
+              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;