From a34bfe99b6ab5aae98b051a88a15816121a20f39 Mon Sep 17 00:00:00 2001 From: Gaurav Munjal Date: Mon, 6 Mar 2023 12:36:28 -0500 Subject: [PATCH] Correct preserveScrollPosition calculation in nested controllers --- addon/services/router-scroll.js | 15 +++++++++----- tests/unit/router-scroll-test.js | 35 +++++++++++++++++++++++++++++--- 2 files changed, 42 insertions(+), 8 deletions(-) diff --git a/addon/services/router-scroll.js b/addon/services/router-scroll.js index 54d6b9b..7f652cc 100644 --- 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 index 0cabfec..4de6f52 100644 --- a/tests/unit/router-scroll-test.js +++ b/tests/unit/router-scroll-test.js @@ -16,7 +16,7 @@ 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', }); @@ -24,7 +24,7 @@ module('router-scroll', function (hooks) { const transition = { handler: { controller: { - preserveScrollPosition: isPreserveScroll || false, + preserveScrollPosition: nested || isPreserveScroll, }, }, router: { @@ -32,7 +32,7 @@ module('router-scroll', function (hooks) { { route: { controller: { - preserveScrollPosition: isPreserveScroll || false, + preserveScrollPosition: !nested && isPreserveScroll, }, }, }, @@ -177,6 +177,35 @@ module('router-scroll', function (hooks) { await settled(); }); + test('Update Scroll Position: Can override preserveScrollPosition', async function (assert) { + assert.expect(1); + + window.scrollTo = () => { + assert.ok(true, 'Scroll To should be called'); + }; + + 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);