diff --git a/app/components/category-chips.gjs b/app/components/category-chips.gjs index 5b66bee..8221efc 100644 --- a/app/components/category-chips.gjs +++ b/app/components/category-chips.gjs @@ -5,6 +5,7 @@ import { on } from '@ember/modifier'; import { fn } from '@ember/helper'; import Icon from '#components/icon'; import { POI_CATEGORIES } from '../utils/poi-categories'; +import { eq, and } from 'ember-truth-helpers'; export default class CategoryChipsComponent extends Component { @service router; @@ -41,6 +42,7 @@ export default class CategoryChipsComponent extends Component { class="category-chip" {{on "click" (fn this.searchCategory category)}} aria-label={{category.label}} + disabled={{and (eq this.mapUi.loadingState.type "category") (eq this.mapUi.loadingState.value category.id)}} > {{category.label}} diff --git a/app/components/search-box.gjs b/app/components/search-box.gjs index 8a70947..4e506b6 100644 --- a/app/components/search-box.gjs +++ b/app/components/search-box.gjs @@ -8,7 +8,7 @@ import { task, timeout } from 'ember-concurrency'; import Icon from '#components/icon'; import humanizeOsmTag from '../helpers/humanize-osm-tag'; import { POI_CATEGORIES } from '../utils/poi-categories'; -import eq from 'ember-truth-helpers/helpers/eq'; +import { eq, or } from 'ember-truth-helpers'; export default class SearchBoxComponent extends Component { @service photon; @@ -211,7 +211,11 @@ export default class SearchBoxComponent extends Component { /> {{#if this.query}} diff --git a/app/icons/270-ring.svg b/app/icons/270-ring.svg new file mode 100644 index 0000000..13df6b5 --- /dev/null +++ b/app/icons/270-ring.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/app/routes/search.js b/app/routes/search.js index 711a290..cf876a4 100644 --- a/app/routes/search.js +++ b/app/routes/search.js @@ -22,97 +22,119 @@ export default class SearchRoute extends Route { const lat = params.lat ? parseFloat(params.lat) : null; const lon = params.lon ? parseFloat(params.lon) : null; let pois = []; + let loadingType = null; + let loadingValue = null; - // Case 0: Category Search (category parameter present) - if (params.category && lat && lon) { - // We need bounds. If we have active map state, use it. - let bounds = this.mapUi.currentBounds; + try { + // Case 0: Category Search (category parameter present) + if (params.category && lat && lon) { + loadingType = 'category'; + loadingValue = params.category; + this.mapUi.startLoading(loadingType, loadingValue); - // If we don't have bounds (direct URL visit), estimate them from lat/lon/zoom(16) - // or just use a fixed box around the center. - if (!bounds) { - // Approximate 0.01 degrees ~ 1km at equator. A viewport is roughly 0.02x0.01 at zoom 16? - // Let's take a safe box of ~1km radius. - const delta = 0.01; - bounds = { - minLat: lat - delta, - maxLat: lat + delta, - minLon: lon - delta, - maxLon: lon + delta, - }; - } + // We need bounds. If we have active map state, use it. + let bounds = this.mapUi.currentBounds; - pois = await this.osm.getCategoryPois(bounds, params.category, lat, lon); - - // Sort by distance from center - pois = pois - .map((p) => ({ - ...p, - _distance: getDistance(lat, lon, p.lat, p.lon), - })) - .sort((a, b) => a._distance - b._distance); - } - // Case 1: Text Search (q parameter present) - else if (params.q) { - // Search with Photon (using lat/lon for bias if available) - pois = await this.photon.search(params.q, lat, lon); - - // Search local bookmarks by name - const queryLower = params.q.toLowerCase(); - const localMatches = this.storage.savedPlaces.filter((p) => { - return ( - p.title?.toLowerCase().includes(queryLower) || - p.description?.toLowerCase().includes(queryLower) - ); - }); - - // Merge local matches - localMatches.forEach((local) => { - const exists = pois.find( - (poi) => - (local.osmId && poi.osmId === local.osmId) || - (poi.id && poi.id === local.id) - ); - if (!exists) { - pois.push(local); + // If we don't have bounds (direct URL visit), estimate them from lat/lon/zoom(16) + // or just use a fixed box around the center. + if (!bounds) { + // Approximate 0.01 degrees ~ 1km at equator. A viewport is roughly 0.02x0.01 at zoom 16? + // Let's take a safe box of ~1km radius. + const delta = 0.01; + bounds = { + minLat: lat - delta, + maxLat: lat + delta, + minLon: lon - delta, + maxLon: lon + delta, + }; } - }); - } - // Case 2: Nearby Search (lat/lon present, no q) - else if (lat && lon) { - const searchRadius = 50; // Default radius - // Fetch POIs from Overpass - pois = await this.osm.getNearbyPois(lat, lon, searchRadius); - - // Get cached/saved places in search radius - const localMatches = this.storage.savedPlaces.filter((p) => { - const dist = getDistance(lat, lon, p.lat, p.lon); - return dist <= searchRadius; - }); - - // Merge local matches - localMatches.forEach((local) => { - const exists = pois.find( - (poi) => - (local.osmId && poi.osmId === local.osmId) || - (poi.id && poi.id === local.id) + pois = await this.osm.getCategoryPois( + bounds, + params.category, + lat, + lon ); - if (!exists) { - pois.push(local); - } - }); - - // Sort by distance from click - pois = pois - .map((p) => { - return { + // Sort by distance from center + pois = pois + .map((p) => ({ ...p, _distance: getDistance(lat, lon, p.lat, p.lon), - }; - }) - .sort((a, b) => a._distance - b._distance); + })) + .sort((a, b) => a._distance - b._distance); + } + // Case 1: Text Search (q parameter present) + else if (params.q) { + loadingType = 'text'; + loadingValue = params.q; + this.mapUi.startLoading(loadingType, loadingValue); + + // Search with Photon (using lat/lon for bias if available) + pois = await this.photon.search(params.q, lat, lon); + + // Search local bookmarks by name + const queryLower = params.q.toLowerCase(); + const localMatches = this.storage.savedPlaces.filter((p) => { + return ( + p.title?.toLowerCase().includes(queryLower) || + p.description?.toLowerCase().includes(queryLower) + ); + }); + + // Merge local matches + localMatches.forEach((local) => { + const exists = pois.find( + (poi) => + (local.osmId && poi.osmId === local.osmId) || + (poi.id && poi.id === local.id) + ); + if (!exists) { + pois.push(local); + } + }); + } + // Case 2: Nearby Search (lat/lon present, no q) + else if (lat && lon) { + // Nearby search does NOT trigger loading state (pulse is used instead) + const searchRadius = 50; // Default radius + + // Fetch POIs from Overpass + pois = await this.osm.getNearbyPois(lat, lon, searchRadius); + + // Get cached/saved places in search radius + const localMatches = this.storage.savedPlaces.filter((p) => { + const dist = getDistance(lat, lon, p.lat, p.lon); + return dist <= searchRadius; + }); + + // Merge local matches + localMatches.forEach((local) => { + const exists = pois.find( + (poi) => + (local.osmId && poi.osmId === local.osmId) || + (poi.id && poi.id === local.id) + ); + + if (!exists) { + pois.push(local); + } + }); + + // Sort by distance from click + pois = pois + .map((p) => { + return { + ...p, + _distance: getDistance(lat, lon, p.lat, p.lon), + }; + }) + .sort((a, b) => a._distance - b._distance); + } + } finally { + if (loadingType && loadingValue) { + this.mapUi.stopLoading(loadingType, loadingValue); + } } // Check if any of these are already bookmarked diff --git a/app/services/map-ui.js b/app/services/map-ui.js index 36dea1d..ca6e8e7 100644 --- a/app/services/map-ui.js +++ b/app/services/map-ui.js @@ -14,6 +14,7 @@ export default class MapUiService extends Service { @tracked preventNextZoom = false; @tracked searchResults = []; @tracked currentSearch = null; + @tracked loadingState = null; selectPlace(place, options = {}) { this.selectedPlace = place; @@ -70,4 +71,26 @@ export default class MapUiService extends Service { updateBounds(bounds) { this.currentBounds = bounds; } + + startLoading(type, value) { + this.loadingState = { type, value }; + } + + stopLoading(type = null, value = null) { + // If no arguments provided, force stop (legacy/cleanup) + if (!type && !value) { + this.loadingState = null; + return; + } + + // Only clear if the current state matches the request + // This prevents a previous search from clearing the state of a new search + if ( + this.loadingState && + this.loadingState.type === type && + this.loadingState.value === value + ) { + this.loadingState = null; + } + } } diff --git a/app/styles/app.css b/app/styles/app.css index 706e4c7..8c53979 100644 --- a/app/styles/app.css +++ b/app/styles/app.css @@ -1315,3 +1315,9 @@ button.create-place { .category-chip:active { background: #eee; } + +.category-chip:disabled { + opacity: 0.75; + cursor: not-allowed; + pointer-events: none; +} diff --git a/app/utils/icons.js b/app/utils/icons.js index 606cd6d..3eb5400 100644 --- a/app/utils/icons.js +++ b/app/utils/icons.js @@ -34,6 +34,7 @@ import user from 'feather-icons/dist/icons/user.svg?raw'; import villageBuildings from '@waysidemapping/pinhead/dist/icons/village_buildings.svg?raw'; import x from 'feather-icons/dist/icons/x.svg?raw'; import zap from 'feather-icons/dist/icons/zap.svg?raw'; +import loadingRing from '../icons/270-ring.svg?raw'; import angelfish from '@waysidemapping/pinhead/dist/icons/angelfish.svg?raw'; import barbell from '@waysidemapping/pinhead/dist/icons/barbell.svg?raw'; @@ -211,6 +212,7 @@ const ICONS = { wikipedia, x, zap, + 'loading-ring': loadingRing, }; const FILLED_ICONS = [ @@ -221,6 +223,7 @@ const FILLED_ICONS = [ 'shopping-basket', 'camera', 'person-sleeping-in-bed', + 'loading-ring', ]; export function getIcon(name) { diff --git a/tests/acceptance/search-loading-test.js b/tests/acceptance/search-loading-test.js new file mode 100644 index 0000000..6f15b39 --- /dev/null +++ b/tests/acceptance/search-loading-test.js @@ -0,0 +1,97 @@ +import { module, test } from 'qunit'; +import { visit } from '@ember/test-helpers'; +import { setupApplicationTest } from 'marco/tests/helpers'; +import Service from '@ember/service'; +import { Promise } from 'rsvp'; + +class MockPhotonService extends Service { + async search(query) { + // Simulate network delay + await new Promise((resolve) => setTimeout(resolve, 50)); + if (query === 'slow') { + await new Promise((resolve) => setTimeout(resolve, 200)); + } + return [ + { + title: 'Test Place', + lat: 1, + lon: 1, + osmId: '123', + osmType: 'node', + }, + ]; + } +} + +class MockOsmService extends Service { + async getCategoryPois(bounds, category) { + await new Promise((resolve) => setTimeout(resolve, 50)); + if (category === 'slow_category') { + await new Promise((resolve) => setTimeout(resolve, 200)); + } + return []; + } + async getNearbyPois() { + return []; + } +} + +module('Acceptance | search loading', function (hooks) { + setupApplicationTest(hooks); + + hooks.beforeEach(function () { + this.owner.register('service:photon', MockPhotonService); + this.owner.register('service:osm', MockOsmService); + }); + + test('search shows loading indicator but nearby search does not', async function (assert) { + const mapUi = this.owner.lookup('service:map-ui'); + + // 1. Text Search + // Start a search and check for loading state immediately + const searchPromise = visit('/search?q=slow'); + + // We can't easily check the DOM mid-transition in acceptance tests without complicated helpers, + // so we check the service state which drives the UI. + // Wait a tiny bit for the route to start processing + await new Promise((r) => setTimeout(r, 10)); + + assert.deepEqual( + mapUi.loadingState, + { type: 'text', value: 'slow' }, + 'Loading state is set for text search' + ); + + await searchPromise; + assert.strictEqual( + mapUi.loadingState, + null, + 'Loading state is cleared after text search' + ); + + // 2. Category Search + const catPromise = visit('/search?category=slow_category&lat=1&lon=1'); + await new Promise((r) => setTimeout(r, 10)); + + assert.deepEqual( + mapUi.loadingState, + { type: 'category', value: 'slow_category' }, + 'Loading state is set for category search' + ); + + await catPromise; + assert.strictEqual( + mapUi.loadingState, + null, + 'Loading state is cleared after category search' + ); + + // 3. Nearby Search + await visit('/search?lat=1&lon=1'); + assert.strictEqual( + mapUi.loadingState, + null, + 'Loading state is NOT set for nearby search' + ); + }); +}); diff --git a/tests/unit/services/map-ui-test.js b/tests/unit/services/map-ui-test.js new file mode 100644 index 0000000..bce6a98 --- /dev/null +++ b/tests/unit/services/map-ui-test.js @@ -0,0 +1,68 @@ +import { module, test } from 'qunit'; +import { setupTest } from 'marco/tests/helpers'; + +module('Unit | Service | map-ui', function (hooks) { + setupTest(hooks); + + test('it handles loading state correctly', function (assert) { + let service = this.owner.lookup('service:map-ui'); + + // Initial state + assert.strictEqual( + service.loadingState, + null, + 'loadingState starts as null' + ); + + // Start loading search A + service.startLoading('search', 'A'); + assert.deepEqual( + service.loadingState, + { type: 'search', value: 'A' }, + 'loadingState is set to search A' + ); + + // Stop loading search A (successful case) + service.stopLoading('search', 'A'); + assert.strictEqual( + service.loadingState, + null, + 'loadingState is cleared when stopped with matching parameters' + ); + }); + + test('it handles race condition: stopLoading only clears if parameters match', function (assert) { + let service = this.owner.lookup('service:map-ui'); + + // 1. Start loading search A + service.startLoading('search', 'A'); + assert.deepEqual(service.loadingState, { type: 'search', value: 'A' }); + + // 2. Start loading search B (interruption) + // In a real app, search B would start before search A finishes. + service.startLoading('search', 'B'); + assert.deepEqual( + service.loadingState, + { type: 'search', value: 'B' }, + 'loadingState updates to search B' + ); + + // 3. Search A finishes and tries to stop loading + // The service should ignore this because current loading state is for B + service.stopLoading('search', 'A'); + + assert.deepEqual( + service.loadingState, + { type: 'search', value: 'B' }, + 'loadingState remains search B even after stopping search A' + ); + + // 4. Search B finishes + service.stopLoading('search', 'B'); + assert.strictEqual( + service.loadingState, + null, + 'loadingState is cleared when search B stops' + ); + }); +});