Correct preserveScrollPosition calculation in nested controllers

DockYard · Mar 6, 2023 · c47a107 · c47a107
1 parent 2f17728
commit c47a107
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 8 deletions.
diff --git a/addon/services/router-scroll.js b/addon/services/router-scroll.js
@@ -134,13 +134,18 @@ class RouterScroll extends Service {
       this.unsetFirstLoad();
     }
 
-    let scrollPosition = this.position;
+    let scrollPosition = this.position || { x: 0, y: 0 };
 
     // If `preserveScrollPosition` was not set on the controller, attempt fallback to `preserveScrollPosition` which was set on the router service.
-    let preserveScrollPosition =
-      (transition.router.currentRouteInfos || []).some(
-        (routeInfo) => routeInfo.route.controller.preserveScrollPosition
-      ) || this.preserveScrollPosition;
+    let preserveScrollPosition = false;
+    (transition.router.currentRouteInfos || []).forEach((routeInfo) => {
+      let thisPreserveScrollPosition =
+        routeInfo.route.controller.preserveScrollPosition;
+      preserveScrollPosition =
+        typeof thisPreserveScrollPosition === 'boolean' &&
+        thisPreserveScrollPosition;
+    });
+    preserveScrollPosition ||= this.preserveScrollPosition;
 
     if (!preserveScrollPosition) {
       const { scrollElement, targetElement, currentURL } = this;

diff --git a/tests/unit/router-scroll-test.js b/tests/unit/router-scroll-test.js
@@ -16,23 +16,23 @@ module('router-scroll', function (hooks) {
     window.scrollTo = scrollTo;
   });
 
-  function getTransitionsMock(URL, isPreserveScroll) {
+  function getTransitionsMock(URL, isPreserveScroll = false, nested = false) {
     Object.defineProperty(subject, 'currentURL', {
       value: URL || 'Hello/World',
     });
 
     const transition = {
       handler: {
         controller: {
-          preserveScrollPosition: isPreserveScroll || false,
+          preserveScrollPosition: nested || isPreserveScroll,
         },
       },
       router: {
         currentRouteInfos: [
           {
             route: {
               controller: {
-                preserveScrollPosition: isPreserveScroll || false,
+                preserveScrollPosition: !nested && isPreserveScroll,
               },
             },
           },
@@ -177,6 +177,37 @@ module('router-scroll', function (hooks) {
     await settled();
   });
 
+  test('Update Scroll Position: Can override preserveScrollPosition', async function (assert) {
+    assert.expect(1);
+    const done = assert.async();
+
+    window.scrollTo = () => {
+      assert.ok(true, 'Scroll To should be called');
+      done();
+    };
+
+    this.owner.register(
+      'service:fastboot',
+      class extends EmberObject {
+        isFastBoot = false;
+      }
+    );
+    const routerScrollService = this.owner.lookup('service:router-scroll');
+    Object.defineProperty(routerScrollService, 'position', {
+      get position() {
+        return { x: 0, y: 0 };
+      },
+    });
+    routerScrollService.scrollElement = 'window';
+
+    subject = this.owner.lookup('service:router');
+    subject.trigger(
+      'routeDidChange',
+      getTransitionsMock('Hello/World', false, true)
+    );
+    await settled();
+  });
+
   test('Update Scroll Position: Can preserve position using routerService', async function (assert) {
     assert.expect(0);