Load places based on map bounds

app/components/map.gjs

@@ -49,7 +49,7 @@ export default class MapComponent extends Component {
       layers: [openfreemap, bookmarkLayer],
       controls: defaultControls({ zoom: false }),
       view: new View({
-        center: fromLonLat([99.04738, 7.58087]),
+        center: fromLonLat([99.05738, 7.55087]),
         zoom: 13.0,
         projection: 'EPSG:3857',
       }),
@@ -59,6 +59,9 @@ export default class MapComponent extends Component {
 
     this.mapInstance.on('singleclick', this.handleMapClick);
 
+    // Load places when map moves
+    this.mapInstance.on('moveend', this.handleMapMove);
+
     // Change cursor to pointer when hovering over a clickable feature
     this.mapInstance.on('pointermove', (e) => {
       const pixel = this.mapInstance.getEventPixel(e.originalEvent);
@@ -68,7 +71,8 @@ export default class MapComponent extends Component {
 
     // Load initial bookmarks
     this.storage.rs.on('ready', () => {
-      this.loadBookmarks();
+      // Initial load based on current view
+      this.handleMapMove();
     });
 
     // Listen for remote storage changes
@@ -81,7 +85,7 @@ export default class MapComponent extends Component {
     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.loadBookmarks();
+      this.handleMapMove();
     });
   });
 
@@ -89,29 +93,21 @@ export default class MapComponent extends Component {
   updateBookmarks = modifier(() => {
     // Depend on the tracked storage.version
     if (this.storage.version >= 0) {
-      this.loadBookmarks();
+      this.handleMapMove();
     }
   });
 
-  async loadBookmarks() {
+  async loadBookmarks(places = []) {
     try {
-      // For now, continue to load ALL places.
-      // In the future, we can pass geohash prefixes based on map view extent here.
-      // e.g. this.storage.loadAllPlaces(this.getVisiblePrefixes())
-      // But since the signature of list() changed to optional prefixes, we should use loadAllPlaces
-      // from the service instead of accessing storage.places.listAll directly if possible,
-      // OR update this call to match the new API.
-      // The service wraps it in loadAllPlaces(), but let's check what that does.
-      
-      // The Service now has: loadAllPlaces(prefixes = null) -> calls rs.places.list(prefixes)
-      
-      // Let's use the Service method if we want to update the tracked property,
-      // BUT this component seems to want to manage the vector source directly.
-      // Actually, looking at line 98, it calls `this.storage.places.listAll()`.
-      // The `listAll` method was REMOVED from the module and replaced with `list`.
-      
-      // So we MUST change this line.
-      const places = await this.storage.places.getPlaces(); 
+      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.
+        places = this.storage.savedPlaces;
+      }
+
+      // Previously: const places = await this.storage.places.getPlaces();
+      // We no longer want to fetch everything blindly.
+      // We rely on 'savedPlaces' being updated by handleMapMove calling storage.loadPlacesInBounds.
 
       this.bookmarkSource.clear();
 
@@ -134,6 +130,19 @@ export default class MapComponent extends Component {
     }
   }
 
+  handleMapMove = async () => {
+    if (!this.mapInstance) return;
+
+    const size = this.mapInstance.getSize();
+    const extent = this.mapInstance.getView().calculateExtent(size);
+    const [minLon, minLat] = toLonLat([extent[0], extent[1]]);
+    const [maxLon, maxLat] = toLonLat([extent[2], extent[3]]);
+
+    const bbox = { minLat, minLon, maxLat, maxLon };
+    await this.storage.loadPlacesInBounds(bbox);
+    this.loadBookmarks(this.storage.savedPlaces);
+  };
+
   handleMapClick = async (event) => {
     // Check if user clicked on a rendered feature (POI or Bookmark) FIRST
     const features = this.mapInstance.getFeaturesAtPixel(event.pixel);

app/services/storage.js

@@ -3,10 +3,13 @@ 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';
 
 export default class StorageService extends Service {
   rs;
   @tracked savedPlaces = [];
+  @tracked loadedPrefixes = [];
+  @tracked currentBbox = null;
   @tracked version = 0; // Shared version tracker for bookmarks
 
   constructor() {
@@ -40,11 +43,29 @@ export default class StorageService extends Service {
     // widget.attach();
 
     this.rs.on('ready', () => {
-      this.loadAllPlaces();
+      // this.loadAllPlaces();
     });
 
-    this.rs.scope('/places/').on('change', () => {
-      this.loadAllPlaces();
+    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.
+      }
     });
   }
 
@@ -54,7 +75,35 @@ export default class StorageService extends Service {
 
   notifyChange() {
     this.version++;
-    this.loadAllPlaces();
+    // this.loadAllPlaces();
+  }
+
+  async loadPlacesInBounds(bbox) {
+    // 1. Calculate required prefixes
+    const requiredPrefixes = getGeohashPrefixesInBbox(bbox);
+
+    // 2. Filter out prefixes we've already loaded
+    const missingPrefixes = requiredPrefixes.filter(
+      (p) => !this.loadedPrefixes.includes(p)
+    );
+
+    if (missingPrefixes.length === 0) {
+      // console.log('All prefixes already loaded for this view');
+      return;
+    }
+
+    console.log('Loading new prefixes:', missingPrefixes);
+
+    // 3. Load places for only the new prefixes
+    await this.loadAllPlaces(missingPrefixes);
+
+    // 4. Update our tracked list of loaded prefixes
+    // Using assignment to trigger reactivity if needed, though simple push/mutation might suffice
+    // depending on usage. Tracked arrays need reassignment or specific Ember array methods
+    // if we want to observe the array itself, but here we just check inclusion.
+    // Let's do a reassignment to be safe and clean.
+    this.loadedPrefixes = [...this.loadedPrefixes, ...missingPrefixes];
+    this.currentBbox = bbox;
   }
 
   async loadAllPlaces(prefixes = null) {

app/utils/geohash-coverage.js

@@ -0,0 +1,65 @@
+import Geohash from 'latlon-geohash';
+
+/**
+ * Calculates 4-character geohash prefixes that cover the given bounding box.
+ *
+ * @param {Object} bbox
+ * @param {number} bbox.minLat
+ * @param {number} bbox.minLon
+ * @param {number} bbox.maxLat
+ * @param {number} bbox.maxLon
+ * @returns {string[]} Array of unique 4-character geohash prefixes
+ */
+export function getGeohashPrefixesInBbox(bbox) {
+  const { minLat, minLon, maxLat, maxLon } = bbox;
+  const prefixes = new Set();
+
+  // 4-char geohash precision is approx 20km x 39km at equator.
+  // We can step through the bbox in increments smaller than that to ensure coverage.
+  // Latitude: 1 deg ~= 111km. 20km ~= 0.18 deg.
+  // Longitude: 1 deg ~= 111km (at equator). 39km ~= 0.35 deg.
+  // Let's use conservative steps to hit every cell.
+  // 0.1 degree steps should be safe enough for 4-char hashes.
+
+  // Safety check to avoid infinite loops or massive arrays if bbox is weird
+  if (Math.abs(maxLat - minLat) > 20 || Math.abs(maxLon - minLon) > 20) {
+    console.warn(
+      'BBox too large for 4-char geohash scanning, aborting fine scan.'
+    );
+    return [];
+  }
+
+  const latStep = 0.1;
+  const lonStep = 0.1;
+
+  for (let lat = minLat; lat <= maxLat + latStep; lat += latStep) {
+    for (let lon = minLon; lon <= maxLon + lonStep; lon += lonStep) {
+      // Clamp to bbox for the edge cases
+      const cLat = Math.min(lat, maxLat);
+      const cLon = Math.min(lon, maxLon);
+
+      try {
+        const hash = Geohash.encode(cLat, cLon, 4);
+        prefixes.add(hash);
+      } catch (e) {
+        // Ignore invalid coords if any
+      }
+    }
+  }
+
+  // Ensure corners are definitely included (floating point steps might miss slightly)
+  try {
+    prefixes.add(Geohash.encode(minLat, minLon, 4));
+  } catch (e) {}
+  try {
+    prefixes.add(Geohash.encode(maxLat, maxLon, 4));
+  } catch (e) {}
+  try {
+    prefixes.add(Geohash.encode(minLat, maxLon, 4));
+  } catch (e) {}
+  try {
+    prefixes.add(Geohash.encode(maxLat, minLon, 4));
+  } catch (e) {}
+
+  return Array.from(prefixes);
+}