From 344eb544f562008ea343ed180c171946c54a58a6 Mon Sep 17 00:00:00 2001 From: Edil Kratskih Date: Fri, 29 Jan 2021 00:49:13 +0600 Subject: [PATCH 1/3] Allow scroll for shallow links --- packages/next/next-server/lib/router/router.ts | 7 +++++-- .../pages/nav/shallow-routing.js | 9 +++++++++ .../client-navigation/test/index.test.js | 15 ++++++++++++++- 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/packages/next/next-server/lib/router/router.ts b/packages/next/next-server/lib/router/router.ts index 6238e1c139aa..c543fbb4e8de 100644 --- a/packages/next/next-server/lib/router/router.ts +++ b/packages/next/next-server/lib/router/router.ts @@ -757,7 +757,7 @@ export default class Router implements BaseRouter { // Default to scroll reset behavior unless explicitly specified to be // `false`! This makes the behavior between using `Router#push` and a // `` consistent. - options.scroll = !!(options.scroll ?? true) + options.scroll = !!(options.scroll ?? (options.shallow ? false : true)) let localeChange = options.locale !== this.locale @@ -1095,6 +1095,7 @@ export default class Router implements BaseRouter { // shallow routing is only allowed for same page URL changes. const isValidShallowRoute = options.shallow && this.route === route + const resetScroll = !options.scroll ? null : { x: 0, y: 0 } await this.set( route, pathname!, @@ -1102,7 +1103,7 @@ export default class Router implements BaseRouter { cleanedAs, routeInfo, forcedScroll || - (isValidShallowRoute || !options.scroll ? null : { x: 0, y: 0 }) + (isValidShallowRoute ? resetScroll : null || resetScroll) ).catch((e) => { if (e.cancelled) error = error || e else throw e @@ -1328,6 +1329,8 @@ export default class Router implements BaseRouter { ): Promise { this.isFallback = false + console.log({ route, resetScroll }) + this.route = route this.pathname = pathname this.query = query diff --git a/test/integration/client-navigation/pages/nav/shallow-routing.js b/test/integration/client-navigation/pages/nav/shallow-routing.js index 728467f9afde..d83548061bad 100644 --- a/test/integration/client-navigation/pages/nav/shallow-routing.js +++ b/test/integration/client-navigation/pages/nav/shallow-routing.js @@ -27,6 +27,12 @@ export default class extends Component { Router.push(href, href, { shallow: true }) } + increaseWithScroll() { + const counter = this.getCurrentCounter() + const href = `/nav/shallow-routing?counter=${counter + 1}` + Router.push(href, href, { shallow: true, scroll: true }) + } + increaseNonShallow() { const counter = this.getCurrentCounter() const href = `/nav/shallow-routing?counter=${counter + 1}` @@ -58,6 +64,9 @@ export default class extends Component { + diff --git a/test/integration/client-navigation/test/index.test.js b/test/integration/client-navigation/test/index.test.js index 0d87cdeda148..9b7aebdf7819 100644 --- a/test/integration/client-navigation/test/index.test.js +++ b/test/integration/client-navigation/test/index.test.js @@ -883,11 +883,24 @@ describe('Client Navigation', () => { expect(scrollPositionDown).toBeGreaterThan(3000) - await browser.elementByCss('#invalidShallow').click() + await browser.elementByCss('#increase3').click() await waitFor(500) const newScrollPosition3 = await browser.eval('window.pageYOffset') expect(newScrollPosition3).toBe(0) + + await browser.eval(() => + document.querySelector('#increase3').scrollIntoView() + ) + const scrollPositionDown2 = await browser.eval('window.pageYOffset') + + expect(scrollPositionDown2).toBeGreaterThan(3000) + + await browser.elementByCss('#invalidShallow').click() + await waitFor(500) + const newScrollPosition4 = await browser.eval('window.pageYOffset') + + expect(newScrollPosition4).toBe(0) }) }) From 11a7f911b34d18a9bee6413409ea3bde28d619f0 Mon Sep 17 00:00:00 2001 From: Edil Kratskih Date: Tue, 2 Feb 2021 20:47:17 +0600 Subject: [PATCH 2/3] Simplify the code --- packages/next/next-server/lib/router/router.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/next/next-server/lib/router/router.ts b/packages/next/next-server/lib/router/router.ts index c543fbb4e8de..c3995527e6ee 100644 --- a/packages/next/next-server/lib/router/router.ts +++ b/packages/next/next-server/lib/router/router.ts @@ -1093,17 +1093,13 @@ export default class Router implements BaseRouter { !(routeInfo.Component as any).getInitialProps } - // shallow routing is only allowed for same page URL changes. - const isValidShallowRoute = options.shallow && this.route === route - const resetScroll = !options.scroll ? null : { x: 0, y: 0 } await this.set( route, pathname!, query, cleanedAs, routeInfo, - forcedScroll || - (isValidShallowRoute ? resetScroll : null || resetScroll) + forcedScroll || (options.scroll ? { x: 0, y: 0 } : null) ).catch((e) => { if (e.cancelled) error = error || e else throw e From fbc981c7c293657e72d6bdf93f5e8af5138dab09 Mon Sep 17 00:00:00 2001 From: Edil Kratskih Date: Wed, 3 Feb 2021 11:37:55 +0600 Subject: [PATCH 3/3] Update router.ts --- packages/next/next-server/lib/router/router.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/next/next-server/lib/router/router.ts b/packages/next/next-server/lib/router/router.ts index b73ae0fbfe53..4a30241ffc67 100644 --- a/packages/next/next-server/lib/router/router.ts +++ b/packages/next/next-server/lib/router/router.ts @@ -1324,9 +1324,6 @@ export default class Router implements BaseRouter { resetScroll: { x: number; y: number } | null ): Promise { this.isFallback = false - - console.log({ route, resetScroll }) - this.route = route this.pathname = pathname this.query = query