hotwired / turbo

The speed of a single-page web application without having to write any JavaScript
https://turbo.hotwired.dev
MIT License
6.65k stars 421 forks source link

Anchor links appear at bottom of page instead of top #400

Closed tao closed 2 years ago

tao commented 2 years ago

I found that on Firefox / Firefox mobile / Chrome that when I use an anchor link, sometimes it loads it at the bottom of the page instead of the top.

--------------
| #anchor    |   <-- expected position after page load
|            |
|            |
|            |
|            |
--------------
--------------
|            |
|            |
|            |
|            |
| #anchor    |   <-- the anchor appears at the bottom instead
--------------

I believe I've narrowed this down to the following css:

        scroll-behavior: smooth;

If I remove this scroll-behavior then the issue disappears but my app requires smooth scroll so I don't disorientate the user when jumping between paragraphs.

My pages are long articles with anchors for each paragraph and they are numbered starting at anchor#1 up to anchor#40 for example.

If I am at anchor#3 and click on a higher number like anchor#15 it'll scroll down and put that anchor at the bottom of the page... so it must be scrolling until the anchor is in-screen but not to the top of the viewport.

If I am at anchor anchor#35 for example, and click a lower number like anchor#4 then it looks like it scrolls up the page and puts the anchor at the top of the page because it's higher up. This must be because the anchor only appears into view at the top of the page and then it stops. This bug happens when you load new pages, but not when jumping between paragraphs on the same page.

So I only found the problem going from lower numbers like anchor#3 to a higher one like anchor#15 where the next link has a much higher Y position on the new page. As soon as the anchor appears at the bottom of the page it must stop the scroll calculation and leave the anchor at the bottom instead of the top of viewport.

tao commented 2 years ago

Here's a demo (https://github.com/tao/turbo-scroll-bug).

  1. Start a server: php -S localhost:9000 and visit http://localhost:9000/one.html
  2. Scroll down to the bottom and click the link to two.html#10 (works fine)
  3. Click the link to go back to one.html#25 (the paragraph will be at the bottom of the page instead of the top)

This happens on Firefox. On Chrome, the turbo link behaviour seems to puts the link in the center of both pages.

seanpdoyle commented 2 years ago

Thank you for providing such a simple example @tao!

I've opened https://github.com/hotwired/turbo/pull/476. Locally, I was unable to re-produce the issue with those changes.

For now, that PR is only the implementation changes. If this resolves your issue, I'll try and transform your example repository into the Turbo test suite.

tao commented 2 years ago

Thanks, this does seem to solve the example problem that I linked on both Firefox and Chrome... but having trouble with my production site first and I'm not 100% yet what's going on yet or if I'm just making a mistake including the new code.

On Chrome it still seems to place the content in the centre.

On Firefox, when I update the url manually in the url bar then it loads the page and scrolls down to the correct place each time, but it seems like when I click on a link inside the page and turbo replaces the content then it still loads the anchor link at the bottom of the page.

I'm going to try debug it a bit more over the next few days and see if I can figure something out... perhaps it's some javascript code interfering with the page render step.

https://user-images.githubusercontent.com/1446331/143786032-d8c27930-a19a-4f85-9b53-16b0cab0ad06.mov

tao commented 2 years ago

It appears that the new update in the pull request with this commit (0406188e63617610359b074d39dba8d99e7f1867) plus the addition of this snippet mentioned in #426 does solve my problem.

If I only use this snippet on the current build of turbo.es2017-umd.js then it doesn't work.

https://user-images.githubusercontent.com/1446331/143845100-581d4dd2-73e6-4c2a-bda9-ab98e1c4c41b.mov

tao commented 2 years ago

The initial problem seems to be solved, so I'm happy with that solution.

However, another problem does occur, but it doesn't seem related as it happens whether I use the pull request or not, and with or without the snippet. So might be best to open another ticket for this. In this video below, you can see I'm on the top of the page, visit a link and it appears at the top of the view in the correct position... but when I return to the starting page it loads it at the bottom?

Perhaps when I visit the anchor link it stores the scroll position which is half way down the page, and when you return to the cached page it doesn't restore the scroll position of the cached page?

https://user-images.githubusercontent.com/1446331/143846240-2ce4a218-74db-4ba7-bf2b-c30ce7dcd899.mov

It appears to work up to a certain point, so links 1-13 allow you to return to the top of the previous page... but at 14 in this example it goes to the bottom... so maybe after the current page length is greater than the cached page length it puts it at the bottom?

https://user-images.githubusercontent.com/1446331/143851738-2c3b3d79-196a-41fc-a12e-a4e3c19f4945.mov

seanpdoyle commented 2 years ago

Was this resolved by https://github.com/hotwired/turbo/pull/571?

seanpdoyle commented 2 years ago

@tao I've rebased #476, could you verify whether or not it still fixes the behavior like you mentioned in https://github.com/hotwired/turbo/issues/400#issuecomment-981484031?

tao commented 2 years ago

@seanpdoyle I tried again with #476 and If I disable scroll-behavior: smooth then it seems like it does work as expected and the other related issues I pointed out in my videos all seem like they've been resolved and working correctly on my production site when I tested it.

With smooth scroll enabled it still does lots of strange things on different browsers, especially on Firefox it's giving me strange behaviour. Chrome seems like it works with the smooth scrolling enabled much better but loads anchor links in the middle of the page.

But if I include this script again #426 just after I include the turbo script then it seems to all work correctly without any issues and smooth scrolling enabled.

        <script src="/js/turbo.es2017-umd.js" defer></script>
        <script type="text/javascript">
            document.addEventListener(`turbo:load`, () => {
                document.documentElement.style.scrollBehavior = `smooth`
            })

            document.addEventListener(`turbo:before-visit`, () => {
                document.documentElement.style.scrollBehavior = `unset`
            })
        </script>

So it's all looking good, I'm going to continue testing it tomorrow and see if I can finally include turbo on my site.