diff --git a/app/components/map.gjs b/app/components/map.gjs index f1ba88f..fd54578 100644 --- a/app/components/map.gjs +++ b/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); diff --git a/app/services/storage.js b/app/services/storage.js index 560d60c..d07b6c9 100644 --- a/app/services/storage.js +++ b/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) { diff --git a/app/utils/geohash-coverage.js b/app/utils/geohash-coverage.js new file mode 100644 index 0000000..f00e206 --- /dev/null +++ b/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); +} diff --git a/package.json b/package.json index 47988ec..107bba6 100644 --- a/package.json +++ b/package.json @@ -73,6 +73,7 @@ "prettier-plugin-ember-template-tag": "^2.1.2", "qunit": "^2.25.0", "qunit-dom": "^3.5.0", + "sinon": "^21.0.1", "stylelint": "^16.26.1", "stylelint-config-standard": "^38.0.0", "testem": "^3.17.0", @@ -87,6 +88,7 @@ "dependencies": { "@remotestorage/module-places": "link:vendor/remotestorage-module-places", "ember-truth-helpers": "^5.0.0", + "latlon-geohash": "^2.0.0", "ol": "^10.7.0", "ol-mapbox-style": "^13.2.0", "remotestorage-widget": "^1.8.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 645b4e0..fa54e0e 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -14,6 +14,9 @@ importers: ember-truth-helpers: specifier: ^5.0.0 version: 5.0.0 + latlon-geohash: + specifier: ^2.0.0 + version: 2.0.0 ol: specifier: ^10.7.0 version: 10.7.0 @@ -150,6 +153,9 @@ importers: qunit-dom: specifier: ^3.5.0 version: 3.5.0 + sinon: + specifier: ^21.0.1 + version: 21.0.1 stylelint: specifier: ^16.26.1 version: 16.26.1(typescript@5.9.3) @@ -1349,6 +1355,15 @@ packages: '@simple-dom/interface@1.4.0': resolution: {integrity: sha512-l5qumKFWU0S+4ZzMaLXFU8tQZsicHEMEyAxI5kDFGhJsRqDwe0a7/iPA/GdxlGyDKseQQAgIz5kzU7eXTrlSpA==} + '@sinonjs/commons@3.0.1': + resolution: {integrity: sha512-K3mCHKQ9sVh8o1C9cxkwxaOmXoAMlDxC1mYyHrjqOWEcBjYr76t96zL2zlj5dUGZ3HSw240X1qgH3Mjf1yJWpQ==} + + '@sinonjs/fake-timers@15.1.0': + resolution: {integrity: sha512-cqfapCxwTGsrR80FEgOoPsTonoefMBY7dnUEbQ+GRcved0jvkJLzvX6F4WtN+HBqbPX/SiFsIRUp+IrCW/2I2w==} + + '@sinonjs/samsam@8.0.3': + resolution: {integrity: sha512-hw6HbX+GyVZzmaYNh82Ecj1vdGZrqVIn/keDTg63IgAwiQPO+xCz99uG6Woqgb4tM0mUiFENKZ4cqd7IX94AXQ==} + '@socket.io/component-emitter@3.1.2': resolution: {integrity: sha512-9BCxFwvbGg/RsZK9tjXd8s4UcwR0MWeFQ1XEKIQVVvAGJyINdrqKMcTRyLoK8Rse1GjzLV9cwjWV1olXRWEXVA==} @@ -2137,6 +2152,10 @@ packages: resolution: {integrity: sha512-2sJGJTaXIIaR1w4iJSNoN0hnMY7Gpc/n8D4qSCJw8QqFWXf7cuAgnEHxBpweaVcPevC2l3KpjYCx3NypQQgaJg==} engines: {node: '>= 0.8', npm: 1.2.8000 || >= 1.4.16} + diff@8.0.3: + resolution: {integrity: sha512-qejHi7bcSD4hQAZE0tNAawRK1ZtafHDmMTMkrrIGgSLl7hTnQHmKCeB45xAcbfTqK2zowkM3j3bHt/4b/ARbYQ==} + engines: {node: '>=0.3.1'} + dir-glob@3.0.1: resolution: {integrity: sha512-WkrWp9GR4KXfKGYzOLmTuGVi1UWFfws377n9cc55/tb6DuqyF6pcQ5AbiHEshaDpY9v6oaSr2XCDidGmMwdzIA==} engines: {node: '>=8'} @@ -3042,6 +3061,9 @@ packages: known-css-properties@0.37.0: resolution: {integrity: sha512-JCDrsP4Z1Sb9JwG0aJ8Eo2r7k4Ou5MwmThS/6lcIe1ICyb7UBJKGRIUUdqc2ASdE/42lgz6zFUnzAIhtXnBVrQ==} + latlon-geohash@2.0.0: + resolution: {integrity: sha512-OKBswTwrvTdtenV+9C9euBmvgGuqyjJNAzpQCarRz1m8/pYD2nz9fKkXmLs2S3jeXaLi3Ry76twQplKKUlgS/g==} + lerc@3.0.0: resolution: {integrity: sha512-Rm4J/WaHhRa93nCN2mwWDZFoRVF18G1f47C+kvQWyHGEZxFpTUi73p7lMVSAndyxGt6lJ2/CFbOcf9ra5p8aww==} @@ -3854,6 +3876,9 @@ packages: simple-html-tokenizer@0.5.11: resolution: {integrity: sha512-C2WEK/Z3HoSFbYq8tI7ni3eOo/NneSPRoPpcM7WdLjFOArFuyXEjAoCdOC3DgMfRyziZQ1hCNR4mrNdWEvD0og==} + sinon@21.0.1: + resolution: {integrity: sha512-Z0NVCW45W8Mg5oC/27/+fCqIHFnW8kpkFOq0j9XJIev4Ld0mKmERaZv5DMLAb9fGCevjKwaEeIQz5+MBXfZcDw==} + slash@3.0.0: resolution: {integrity: sha512-g9Q1haeby36OSStwb4ntCGGGaKsaVSjQ68fBxoQcutl5fS1vuY18H3wSt3jFyFtrkx+Kz0V1G85A4MyAdDMi2Q==} engines: {node: '>=8'} @@ -4092,6 +4117,14 @@ packages: resolution: {integrity: sha512-XleUoc9uwGXqjWwXaUTZAmzMcFZ5858QA2vvx1Ur5xIcixXIP+8LnFDgRplU30us6teqdlskFfu+ae4K79Ooew==} engines: {node: '>= 0.8.0'} + type-detect@4.0.8: + resolution: {integrity: sha512-0fr/mIH1dlO+x7TlcMy+bIDqKPsw/70tVyeHW787goQjhmqaZe10uwLujubK9q9Lg6Fiho1KUKDYz0Z7k7g5/g==} + engines: {node: '>=4'} + + type-detect@4.1.0: + resolution: {integrity: sha512-Acylog8/luQ8L7il+geoSxhEkazvkslg7PSNKOX59mbB9cOveP5aq9h74Y7YU8yDpJwetzQQrfIwtf4Wp4LKcw==} + engines: {node: '>=4'} + type-fest@4.41.0: resolution: {integrity: sha512-TeTSQ6H5YHvpqVwBRcnLDCBnDOHWYu7IvGbHT6N8AOymcr9PJGjc1GTtiWZTYg0NCgYwvnYWEkVChQAr9bjfwA==} engines: {node: '>=16'} @@ -5763,6 +5796,19 @@ snapshots: '@simple-dom/interface@1.4.0': {} + '@sinonjs/commons@3.0.1': + dependencies: + type-detect: 4.0.8 + + '@sinonjs/fake-timers@15.1.0': + dependencies: + '@sinonjs/commons': 3.0.1 + + '@sinonjs/samsam@8.0.3': + dependencies: + '@sinonjs/commons': 3.0.1 + type-detect: 4.1.0 + '@socket.io/component-emitter@3.1.2': {} '@types/cors@2.8.19': @@ -6533,6 +6579,8 @@ snapshots: destroy@1.2.0: {} + diff@8.0.3: {} + dir-glob@3.0.1: dependencies: path-type: 4.0.0 @@ -7771,6 +7819,8 @@ snapshots: known-css-properties@0.37.0: {} + latlon-geohash@2.0.0: {} + lerc@3.0.0: {} levn@0.4.1: @@ -8567,6 +8617,14 @@ snapshots: simple-html-tokenizer@0.5.11: {} + sinon@21.0.1: + dependencies: + '@sinonjs/commons': 3.0.1 + '@sinonjs/fake-timers': 15.1.0 + '@sinonjs/samsam': 8.0.3 + diff: 8.0.3 + supports-color: 7.2.0 + slash@3.0.0: {} slice-ansi@4.0.0: @@ -8942,6 +9000,10 @@ snapshots: dependencies: prelude-ls: 1.2.1 + type-detect@4.0.8: {} + + type-detect@4.1.0: {} + type-fest@4.41.0: {} type-is@1.6.18: diff --git a/tests/unit/utils/geohash-coverage-test.js b/tests/unit/utils/geohash-coverage-test.js new file mode 100644 index 0000000..c7fbf7c --- /dev/null +++ b/tests/unit/utils/geohash-coverage-test.js @@ -0,0 +1,50 @@ +import { module, test } from 'qunit'; +import { setupTest } from 'ember-qunit'; +import { getGeohashPrefixesInBbox } from 'marco/utils/geohash-coverage'; + +module('Unit | Utility | geohash-coverage', function (hooks) { + setupTest(hooks); + + test('it returns correct prefixes for a small bounding box', function (assert) { + // A small area in Berlin (approx) + // 4-char geohash for Berlin is roughly 'u33d' + // lat ~ 52.5, lon ~ 13.4 + const bbox = { + minLat: 52.51, + minLon: 13.38, + maxLat: 52.52, + maxLon: 13.4, + }; + + // Precision 4 + const prefixes = getGeohashPrefixesInBbox(bbox); + + assert.ok(prefixes.length > 0, 'Should return prefixes'); + assert.ok( + prefixes.every((p) => p.length === 4), + 'Prefixes should be 4 chars long' + ); + // We expect 'u33d' to be in there or neighbors + // Berlin Alexanderplatz is u33dc1 + assert.ok( + prefixes.some((p) => p.startsWith('u33')), + 'Should contain Berlin region prefix' + ); + }); + + test('it handles cases where steps might miss a small edge', function (assert) { + // A bbox that is exactly one point + const bbox = { + minLat: 50.0, + minLon: 10.0, + maxLat: 50.0, + maxLon: 10.0, + }; + const prefixes = getGeohashPrefixesInBbox(bbox); + assert.strictEqual( + prefixes.length, + 1, + 'Single point should result in 1 prefix' + ); + }); +});