Correct preserveScrollPosition calculation in nested controllers

.github/workflows/ci.yml

@@ -17,7 +17,7 @@ jobs:
       - name: Install Node
         uses: actions/setup-node@v2
         with:
-          node-version: 12.x
+          node-version: 14.x
           cache: yarn
       - name: Install Dependencies
         run: yarn install --frozen-lockfile
@@ -34,7 +34,7 @@ jobs:
       - uses: actions/checkout@v2
       - uses: actions/setup-node@v2
         with:
-          node-version: 12.x
+          node-version: 14.x
           cache: yarn
       - name: Install Dependencies
         run: yarn install --no-lockfile
@@ -54,6 +54,9 @@ jobs:
           - ember-lts-3.16
           - ember-lts-3.20
           - ember-lts-3.24
+          - ember-lts-3.28
+          - ember-lts-4.4
+          - ember-lts-4.8
           - ember-release
           - ember-beta
           - ember-canary
@@ -67,7 +70,7 @@ jobs:
       - name: Install Node
         uses: actions/setup-node@v2
         with:
-          node-version: 12.x
+          node-version: 14.x
           cache: yarn
       - name: Install Dependencies
         run: yarn install --frozen-lockfile

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;

config/ember-try.js

@@ -39,6 +39,30 @@ module.exports = async function () {
           },
         },
       },
+      {
+        name: 'ember-lts-3.28',
+        npm: {
+          devDependencies: {
+            'ember-source': '~3.28.0',
+          },
+        },
+      },
+      {
+        name: 'ember-lts-4.4',
+        npm: {
+          devDependencies: {
+            'ember-source': '~4.4.0',
+          },
+        },
+      },
+      {
+        name: 'ember-lts-4.8',
+        npm: {
+          devDependencies: {
+            'ember-source': '~4.8.0',
+          },
+        },
+      },
       {
         name: 'ember-release',
         npm: {

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);