From c47a1071a05d98a80fe10933e452647b462edcaa Mon Sep 17 00:00:00 2001 From: Gaurav Munjal Date: Mon, 6 Mar 2023 12:36:28 -0500 Subject: [PATCH 1/2] Correct preserveScrollPosition calculation in nested controllers --- addon/services/router-scroll.js | 15 ++++++++----- tests/unit/router-scroll-test.js | 37 +++++++++++++++++++++++++++++--- 2 files changed, 44 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..a6dd6c8 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,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); From d7eb5206053a3ca424a11c2885ecebbd1499099b Mon Sep 17 00:00:00 2001 From: Gaurav Munjal Date: Mon, 6 Mar 2023 15:39:50 -0500 Subject: [PATCH 2/2] fix floating dependencies test --- .github/workflows/ci.yml | 9 ++++++--- config/ember-try.js | 24 ++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 777b705..64c5aed 100644 --- a/.github/workflows/ci.yml +++ b/.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 diff --git a/config/ember-try.js b/config/ember-try.js index 794ecb7..df81f6c 100644 --- a/config/ember-try.js +++ b/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: {