Merge pull request 'Fix some routing and click/nav UX issues' (#67) from bugfix/search_routing into master

Reviewed-on: #67

app/components/app-header.gjs

@@ -9,6 +9,7 @@ import SearchBox from '#components/search-box';
 import CategoryChips from '#components/category-chips';
 import { and } from 'ember-truth-helpers';
 import cachedImage from '../modifiers/cached-image';
+import { POI_CATEGORIES } from '../utils/poi-categories';
 
 export default class AppHeaderComponent extends Component {
   @service storage;
@@ -16,11 +17,59 @@ export default class AppHeaderComponent extends Component {
   @service nostrAuth;
   @service nostrData;
   @service mapUi;
+  @service router;
   @tracked isUserMenuOpen = false;
   @tracked searchQuery = '';
 
-  get hasQuery() {
-    return !!this.searchQuery;
+  constructor() {
+    super(...arguments);
+    if (this.router && typeof this.router.on === 'function') {
+      this.router.on('routeDidChange', this.syncSearchQuery);
+    }
+    this.syncSearchQuery();
+  }
+
+  willDestroy() {
+    if (this.router && typeof this.router.off === 'function') {
+      this.router.off('routeDidChange', this.syncSearchQuery);
+    }
+    super.willDestroy(...arguments);
+  }
+
+  @action
+  syncSearchQuery() {
+    const qp =
+      this.mapUi.currentSearch || this.router?.currentRoute?.queryParams;
+    if (qp?.q) {
+      this.searchQuery = qp.q;
+    } else if (qp?.category) {
+      const category = POI_CATEGORIES.find((c) => c.id === qp.category);
+      this.searchQuery = category ? category.label : qp.category;
+    } else {
+      this.searchQuery = '';
+    }
+  }
+
+  get isSearching() {
+    // 1. If we are actively focusing/typing in the search box with a query, hide pills
+    if (this.mapUi.searchBoxHasFocus && this.searchQuery) {
+      return true;
+    }
+
+    // 2. If we are on the search route, check loading and results status
+    if (this.router?.currentRouteName === 'search') {
+      if (this.mapUi.loadingState) {
+        return false; // Keep pills visible while loading
+      }
+      return this.mapUi.searchResults && this.mapUi.searchResults.length > 0;
+    }
+
+    // 3. Fallback for integration tests (non-search route with a query)
+    if (this.router?.currentRouteName !== 'search' && this.searchQuery) {
+      return true;
+    }
+
+    return false;
   }
 
   get showQuickSearch() {
@@ -61,7 +110,7 @@ export default class AppHeaderComponent extends Component {
       </div>
 
       {{#if this.showQuickSearch}}
-        <div class="header-center {{if this.hasQuery 'searching'}}">
+        <div class="header-center {{if this.isSearching 'searching'}}">
           <CategoryChips @onSelect={{this.handleChipSelect}} />
         </div>
       {{/if}}

app/components/map.gjs

@@ -1088,6 +1088,7 @@ export default class MapComponent extends Component {
     const bbox = { minLat, minLon, maxLat, maxLon };
     this.mapUi.updateBounds(bbox);
     await this.storage.loadPlacesInBounds(bbox);
+    if (this.isDestroying || this.isDestroyed) return;
     this.nostrData.loadPlacesInBounds(bbox);
     this.loadBookmarks(this.storage.placesInView);
 
@@ -1191,6 +1192,12 @@ export default class MapComponent extends Component {
       return;
     }
 
+    if (this.mapUi.searchResults && this.mapUi.searchResults.length > 0) {
+      console.debug('Clearing active search and markers on map click');
+      this.router.transitionTo('index');
+      return;
+    }
+
     // Require Zoom >= 17 for generic map searches
     // This prevents accidental searches when interacting with the map at a high level
     const currentZoom = this.mapInstance.getView().getZoom();

app/controllers/search.js

@@ -33,7 +33,9 @@ export default class SearchController extends Controller {
 
     // 2. If it's a back navigation to the exact same search, resolve instantly with no animation
     if (isSameSearch && hasResults) {
-      this.mapUi.showSidebar();
+      if (this.mapUi.isSidebarVisible) {
+        this.mapUi.showSidebar();
+      }
       return;
     }
 

app/styles/app.css

@@ -1424,6 +1424,9 @@ button.create-place {
     border-top-left-radius: 16px;
     border-top-right-radius: 16px;
     inset: auto 0 0;
+  }
+
+  .sidebar-opening .sidebar {
     animation: sidebar-slide-up-bottom 0.18s cubic-bezier(0.16, 1, 0.3, 1)
       forwards;
   }

app/templates/application.gjs

@@ -55,6 +55,14 @@ export default class ApplicationComponent extends Component {
       this.mapUi.hideSidebar();
       if (name === 'menu' || name.startsWith('lists')) {
         this.router.transitionTo('index');
+      } else if (name === 'place') {
+        if (this.mapUi.returnToSearch && this.mapUi.currentSearch) {
+          this.router.transitionTo('search', {
+            queryParams: this.mapUi.currentSearch,
+          });
+        } else {
+          this.router.transitionTo('index');
+        }
       }
     }
   }

app/templates/place.gjs

@@ -104,6 +104,13 @@ export default class PlaceTemplate extends Component {
   close() {
     this.mapUi.clearSelection();
     this.mapUi.hideSidebar();
+    if (this.mapUi.returnToSearch && this.mapUi.currentSearch) {
+      this.router.transitionTo('search', {
+        queryParams: this.mapUi.currentSearch,
+      });
+    } else {
+      this.router.transitionTo('index');
+    }
   }
 
   <template>

tests/acceptance/map-search-reset-test.js

@@ -101,7 +101,7 @@ module('Acceptance | map search reset', function (hooks) {
         'Should have stayed on the search route with markers intact'
       );
 
-      // Second Click (Start new search)
+      // Second Click (Clear search and markers)
       // Click slightly differently to ensure fresh event
       await triggerEvent(canvas, 'pointerdown', {
         clientX: 250,
@@ -125,11 +125,6 @@ module('Acceptance | map search reset', function (hooks) {
     // 3. Wait for transition
     await new Promise((r) => setTimeout(r, 1000));
 
-    const newUrl = currentURL();
-    assert.notOk(
-      newUrl.includes('category=coffee'),
-      `New URL ${newUrl} should not contain category param`
-    );
-    assert.ok(newUrl.includes('/search'), 'Should be on search route');
+    assert.strictEqual(currentURL(), '/', 'Should have transitioned to index');
   });
 });

tests/acceptance/navigation-test.js

@@ -85,7 +85,7 @@ module('Acceptance | navigation', function (hooks) {
     );
   });
 
-  test('closing the sidebar resets the returnToSearch flag', async function (assert) {
+  test('closing the sidebar transitions back to search route when opened from search results', async function (assert) {
     const mapUi = this.owner.lookup('service:map-ui');
 
     await visit('/search?lat=1&lon=1');
@@ -97,7 +97,22 @@ module('Acceptance | navigation', function (hooks) {
     await click('.close-btn');
 
     assert.dom('.sidebar').doesNotExist('Sidebar should be closed');
-    assert.ok(currentURL().includes('/place/'), 'Remains on place route');
+    assert.strictEqual(
+      currentURL(),
+      '/search?lat=1&lon=1',
+      'Should transition back to search route'
+    );
+  });
+
+  test('closing the sidebar when visiting a place directly transitions to index', async function (assert) {
+    await visit('/place/osm:node:123');
+    assert.ok(currentURL().includes('/place/'), 'Visited place directly');
+
+    // Click the Close (X) button
+    await click('.close-btn');
+
+    assert.dom('.sidebar').doesNotExist('Sidebar should be closed');
+    assert.strictEqual(currentURL(), '/', 'Should transition back to index');
   });
 
   test('navigating directly to place and back closes sidebar', async function (assert) {

tests/acceptance/search-loading-test.js

@@ -169,4 +169,77 @@ module('Acceptance | search loading', function (hooks) {
     // Verify we are back on index (or at least query is gone)
     assert.strictEqual(currentURL(), '/', 'Navigated to index');
   });
+
+  test('quick search pills visibility during category search transition', async function (assert) {
+    const mapUi = this.owner.lookup('service:map-ui');
+    mapUi.currentZoom = 15;
+
+    // Seed localStorage with a high zoom level to ensure quick search buttons show
+    const highZoomState = {
+      center: [13.4, 52.5],
+      zoom: 18,
+    };
+    window.localStorage.setItem(
+      'marco:map-view',
+      JSON.stringify(highZoomState)
+    );
+
+    try {
+      // Make sure quick search buttons setting is enabled
+      const settings = this.owner.lookup('service:settings');
+      settings.showQuickSearchButtons = true;
+
+      // 1. Visit slowly loading category search
+      const catPromise = visit('/search?category=slow_category&lat=1&lon=1');
+      await new Promise((r) => setTimeout(r, 10));
+
+      // Verify loading state is set and pills are visible (i.e. header-center does NOT have .searching)
+      assert.ok(mapUi.loadingState, 'Search is loading');
+      assert
+        .dom('.header-center')
+        .doesNotHaveClass(
+          'searching',
+          'Pills remain visible while search is loading'
+        );
+
+      // Resolve the promise with empty results
+      osmResolve();
+      await catPromise;
+      await settled();
+
+      // Verify search completed and since results are empty, pills are still visible
+      assert.strictEqual(mapUi.searchResults.length, 0, 'No results found');
+      assert
+        .dom('.header-center')
+        .doesNotHaveClass(
+          'searching',
+          'Pills remain visible after search completes with no results'
+        );
+
+      // 2. Now simulate a fast category search that returns results
+      const osmService = this.owner.lookup('service:osm');
+      osmService.getCategoryPois = async () => [
+        {
+          title: 'Latte Art Cafe',
+          lat: 1,
+          lon: 1,
+          osmId: '101',
+          osmType: 'N',
+        },
+      ];
+
+      await visit('/search?category=coffee&lat=1&lon=1');
+
+      // Verify search completed with results, so pills are hidden
+      assert.ok(mapUi.searchResults.length > 0, 'Results found');
+      assert
+        .dom('.header-center')
+        .hasClass(
+          'searching',
+          'Pills are hidden after search completes with results'
+        );
+    } finally {
+      window.localStorage.removeItem('marco:map-view');
+    }
+  });
 });

tests/acceptance/search-test.js

@@ -270,4 +270,76 @@ module('Acceptance | search', function (hooks) {
       .hasText('Search request failed. Please try again.');
     assert.dom('.places-sidebar').doesNotExist('Results panel should not open');
   });
+
+  test('search box query synchronized with active route query parameters', async function (assert) {
+    // Mock Osm Service
+    class MockOsmService extends Service {
+      async getCategoryPois() {
+        return [];
+      }
+    }
+    this.owner.register('service:osm', MockOsmService);
+
+    // Mock Photon Service
+    class MockPhotonService extends Service {
+      async search() {
+        return [];
+      }
+    }
+    this.owner.register('service:photon', MockPhotonService);
+
+    // Mock Storage Service
+    class MockStorageService extends Service {
+      savedPlaces = [];
+      findPlaceById() {
+        return null;
+      }
+      isPlaceSaved() {
+        return false;
+      }
+      rs = { on: () => {} };
+      placesInView = [];
+      loadPlacesInBounds() {
+        return Promise.resolve();
+      }
+    }
+    this.owner.register('service:storage', MockStorageService);
+
+    // Mock Map Service
+    class MockMapService extends Service {
+      getBounds() {
+        return {
+          minLat: 52.5,
+          minLon: 13.4,
+          maxLat: 52.6,
+          maxLon: 13.5,
+        };
+      }
+    }
+    this.owner.register('service:map', MockMapService);
+
+    // 1. Visit a search URL directly
+    await visit('/search?q=Berlin');
+    assert
+      .dom('.search-input')
+      .hasValue(
+        'Berlin',
+        'Search input is populated with search term on direct load'
+      );
+
+    // 2. Visit a category search URL
+    await visit('/search?category=coffee&lat=52.52&lon=13.405');
+    assert
+      .dom('.search-input')
+      .hasValue(
+        'Coffee',
+        'Search input is populated with mapped category label'
+      );
+
+    // 3. Go back to index
+    await visit('/');
+    assert
+      .dom('.search-input')
+      .hasValue('', 'Search input is cleared on transitioning to index');
+  });
 });