Merge pull request 'Save/restore scroll position for bookmark lists' (#112) from bugfix/saved-places_scroll into master

Reviewed-on: #112

app/controllers/lists/list.js

@@ -23,18 +23,34 @@ export default class ListsListController extends Controller {
 
   @tracked model;
   @tracked loadedPlaces = [];
+  @tracked loadedListId = null;
 
   get listId() {
     return this.model?.list_id;
   }
 
+  get isLoading() {
+    // Only show the spinner when there is nothing to display yet. On re-entry
+    // the previously loaded places (plus live storage state) render instantly,
+    // so we avoid a loading flash and preserve restored scroll position.
+    return this.loadPlacesTask.isRunning && this.places.length === 0;
+  }
+
   loadPlacesTask = task({ restartable: true }, async (listId) => {
+    // Already loaded this list: keep the existing places so returning from
+    // place details is instant (mirrors the activity service's load guard).
+    if (this.loadedListId === listId) {
+      return;
+    }
+
+    this.loadedListId = listId;
     this.loadedPlaces = []; // Clear previous elements immediately to show fresh loader
     try {
       this.loadedPlaces = await this.storage.getPlacesInList(listId);
     } catch (e) {
       console.error('Failed to load places in list', listId, e);
       this.loadedPlaces = [];
+      this.loadedListId = null; // Allow a retry on next entry
     }
   });
 

app/templates/lists/index.gjs

@@ -5,6 +5,7 @@ import { fn } from '@ember/helper';
 import { on } from '@ember/modifier';
 import Icon from '#components/icon';
 import { htmlSafe } from '@ember/template';
+import restoreScroll from '../../modifiers/restore-scroll';
 
 export default class ListsIndexTemplate extends Component {
   @service storage;
@@ -26,8 +27,16 @@ export default class ListsIndexTemplate extends Component {
     return htmlSafe(`background-color: ${finalColor}`);
   }
 
+  get scrollTop() {
+    return this.mapUi.getScrollPosition('lists-index');
+  }
+
   @action
   selectList(listId) {
+    const sidebarContent = document.querySelector('.sidebar-content');
+    if (sidebarContent) {
+      this.mapUi.saveScrollPosition('lists-index', sidebarContent.scrollTop);
+    }
     this.router.transitionTo('lists.list', listId);
   }
 
@@ -59,7 +68,7 @@ export default class ListsIndexTemplate extends Component {
           </button>
         </div>
 
-        <div class="sidebar-content">
+        <div class="sidebar-content" {{restoreScroll this.scrollTop}}>
           <ul class="places-list">
             <li>
               <button

app/templates/lists/list.gjs

@@ -7,7 +7,7 @@ import PlacesSidebar from '#components/places-sidebar';
       @title={{@controller.listTitle}}
       @color={{@controller.listColor}}
       @scrollTop={{@controller.scrollTop}}
-      @isLoading={{@controller.loadPlacesTask.isRunning}}
+      @isLoading={{@controller.isLoading}}
       @onSelect={{@controller.selectPlace}}
       @onClose={{@controller.close}}
       @onBack={{@controller.backToLists}}

tests/acceptance/collections-test.js

@@ -271,4 +271,85 @@ module('Acceptance | collections navigation', function (hooks) {
       'Saved Places are ordered by createdAt in descending order'
     );
   });
+
+  test('returning from place details preserves scroll position without a loading flash', async function (assert) {
+    const places = Array.from({ length: 40 }, (_, i) => ({
+      id: `place-${i}`,
+      title: `Place ${i}`,
+      geohash: 'u33dc0',
+      lat: 48.1,
+      lon: 11.5,
+      createdAt: new Date(2023, 0, 1, 12, 0, i).toISOString(),
+      osmTags: { name: `Place ${i}` },
+    }));
+
+    class ManyPlacesStorageService extends Service {
+      initialSyncDone = true;
+      savedPlaces = places;
+      lists = [
+        {
+          id: 'to-go',
+          title: 'Want to go',
+          color: '#2e9e4f',
+          placeRefs: places.map((p) => ({ id: p.id, geohash: p.geohash })),
+        },
+      ];
+
+      findPlaceById(id) {
+        return this.savedPlaces.find((p) => p.id === id) || null;
+      }
+
+      isPlaceSaved() {
+        return true;
+      }
+
+      loadPlacesInBounds() {
+        return [];
+      }
+
+      getPlacesInList(listId) {
+        if (listId === 'to-go') {
+          return Promise.resolve(this.savedPlaces);
+        }
+        return Promise.resolve([]);
+      }
+
+      rs = {
+        on: () => {},
+      };
+    }
+
+    this.owner.unregister('service:storage');
+    this.owner.register('service:storage', ManyPlacesStorageService);
+
+    await visit('/lists/to-go');
+    await waitFor('.places-list');
+
+    const sidebarContent = document.querySelector('.sidebar-content');
+    sidebarContent.scrollTop = 300;
+    assert.strictEqual(sidebarContent.scrollTop, 300, 'List is scrolled');
+
+    // Open place details from an item that is currently visible so the browser
+    // does not scroll it back into view (which would reset our saved position),
+    // then return via the back button.
+    const visibleItem = document.querySelectorAll('.place-item')[5];
+    await click(visibleItem);
+    await waitFor('.sidebar-content');
+    await click('.back-btn');
+    await waitFor('.places-list');
+
+    // Allow the restore-scroll requestAnimationFrame to run
+    await new Promise((resolve) => requestAnimationFrame(resolve));
+    await new Promise((resolve) => requestAnimationFrame(resolve));
+
+    const restored = document.querySelector('.sidebar-content');
+    assert.strictEqual(
+      restored.scrollTop,
+      300,
+      'Scroll position is restored on return'
+    );
+    assert
+      .dom('.sidebar-loading')
+      .doesNotExist('No loading spinner flashes when returning');
+  });
 });