Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Turbo scrolls to Top of current page before rendering new visited page (related to data-turbo-track="reload") #387

Closed
chipairon opened this issue Sep 10, 2021 · 3 comments · Fixed by #601

Comments

@chipairon
Copy link

chipairon commented Sep 10, 2021

I have a demo of the issue here https://prairie-swamp-observatory.glitch.me/index.html
second_page.html has a script with data-turbo-track="reload" that the first page does NOT have.

When clicking the link at the bottom of the page (from either page):
1- The scroll is moved to the top of the current page.
2- Then the new page is rendered.

My expectation would be that the current page is not scrolled to top before the next page is rendered.

If the turbo-track:reload attribute is removed from the script tag, the scroll does not happen:

<script src="/second_page_script.js" defer data-turbo-track="reload"></script>

The source code of the demo is here: https://glitch.com/edit/#!/remix/prairie-swamp-observatory

The demo loads Turbo from skypack (currently @hotwired/turbo@7.0.0-rc.3).

PS: it might be related to the behaviour described in #114 but I'm not sure.

srt32 added a commit to srt32/turbo that referenced this issue Apr 13, 2022
@srt32
Copy link
Contributor

srt32 commented Apr 14, 2022

Thanks for reporting this! We've bumped into a similar issue. I've been debugging for a bit but no success yet. I can report that I can repro the issue by running the server and going to http://localhost:9000/src/tests/fixtures/rendering.html.

Then, scroll down (you may need to shrink the height of your window), and click on "Visit control: reload". The URL will update, the page scroll up, and then navigate.

@srt32
Copy link
Contributor

srt32 commented Apr 15, 2022

I've determined the problem is in

this.performScroll()
and in the "reload" case we shouldn't scroll but I'm not sure how to properly gate this action without breaking other behavior like properly scrolling back to anchors on the same page.

srt32 added a commit to srt32/turbo that referenced this issue Apr 18, 2022
Potential solution for hotwired#387
dhh pushed a commit that referenced this issue May 19, 2022
* Don't scroll on reload

Potential solution for #387

* Be more specific with the check

* Put the check back

* Guard the performScroll call directly

* WIP - spec

* Properly assert in scroll spec

Co-authored-by: manuelpuyol@github.com

* Update rendering_tests.ts
@srt32
Copy link
Contributor

srt32 commented Jun 10, 2022

@chipairon does #571 resolve your issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants