From b02fff63d0ec3dee7a49f835f35ae71027f50b26 Mon Sep 17 00:00:00 2001 From: Olivier Tassinari Date: Sat, 11 Aug 2018 22:04:16 +0200 Subject: [PATCH] Fix broken hash scroll logic (#4766) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `` does not receive any property. There is no way the *scrollToHash* logic can work right now. I believe it's a regression. It was working fine at some point. I'm sorry, I'm too lazy to add a test. This fix was tested on Material-UI 👌. This bug reproduction is the following: As soon as you want to transition to a new page with a hash. The scroll doesn't change. - start on pageA - you scrollTop to 100 - you move to pageB#hash - you stay at scrollTop 100, but #hash is at scrollTop 400. --- lib/app.js | 5 ++--- test/integration/basic/pages/nav/index.js | 1 + .../basic/test/client-navigation.js | 21 ++++++++++++++++++- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/lib/app.js b/lib/app.js index 15788ef6..58d2d23a 100644 --- a/lib/app.js +++ b/lib/app.js @@ -61,7 +61,7 @@ export class Container extends Component { } scrollToHash () { - const { hash } = this.props + const { hash } = this.context._containerProps if (!hash) return const el = document.getElementById(hash) @@ -73,8 +73,7 @@ export class Container extends Component { } render () { - const {children} = this.props - return <>{children} + return this.props.children } } diff --git a/test/integration/basic/pages/nav/index.js b/test/integration/basic/pages/nav/index.js index e09a2e86..e1926d37 100644 --- a/test/integration/basic/pages/nav/index.js +++ b/test/integration/basic/pages/nav/index.js @@ -53,6 +53,7 @@ export default class extends Component { Counter: {counter} + Scroll to hash ) } diff --git a/test/integration/basic/test/client-navigation.js b/test/integration/basic/test/client-navigation.js index fd0ee4dd..8d716f8a 100644 --- a/test/integration/basic/test/client-navigation.js +++ b/test/integration/basic/test/client-navigation.js @@ -248,7 +248,7 @@ export default (context, render) => { browser.close() }) - it('should scroll to the specified position', async () => { + it('should scroll to the specified position on the same page', async () => { let browser try { browser = await webdriver(context.appPort, '/nav/hash-changes') @@ -272,6 +272,25 @@ export default (context, render) => { } } }) + + it('should scroll to the specified position to a new page', async () => { + let browser + try { + browser = await webdriver(context.appPort, '/nav') + + // Scrolls to item 400 on the page + await browser + .elementByCss('#scroll-to-hash').click() + .waitForElementByCss('#hash-changes-page') + + const scrollPosition = await browser.eval('window.pageYOffset') + expect(scrollPosition).toBe(7258) + } finally { + if (browser) { + browser.close() + } + } + }) }) describe('when hash change via A tag', () => {