qwik icon indicating copy to clipboard operation
qwik copied to clipboard

fix(client-navigate.ts): Wait to scroll the page until navigation has finished

Open MarmadileManteater opened this issue 2 years ago • 1 comments

What is it?

  • [ ] Feature / enhancement
  • [X] Bug
  • [ ] Docs / tests

Description

win.scrollTo(0,0) is being fired too early on route navigation. This causes the page to jitter to the top just before the navigation takes place. This PR aims to fix this by awaiting modification to the page before scrolling to the top.

Replicating the problem

This problem can be replicated in the project created by npm create qwik@latest by simply adding content to the body until the page scrolls and clicking the "Blow my mind 🤯" button after scrolling to the bottom of the page.

Screenshots

before these changes:

after these changes:

Checklist:

  • [X] My code follows the developer guidelines of this project
  • [X] I have performed a self-review of my own code
  • [ ] I have made corresponding changes to the documentation
  • [ ] Added new tests to cover the fix / functionality

Additional Information

I'm not entirely married to this solution, but I wanted to bring to attention the fact that this problem exists and that there is a solution for it. My implementation for waitForDOMNodeInserted may not be taking into consideration every edge case. I personally can't think of any edge cases to check for, but there may be one. DOMNodeInserted might also not be the best listener to wait on in order to solve this problem.

I ran the tests locally, but I was having some issues getting the e2e tests to work even on the main branch, so I must be missing something on my machine. I also am getting a weird error from stackblitz on yarn install, so 🤷‍♀️I don't know what that is about.

I posted a little about this issue in thie #qwik-help channel in the Builder.io discord, but as I kept digging, I realized the source of the problem was this race condition in client-navigate.ts.

MarmadileManteater avatar Nov 08 '22 17:11 MarmadileManteater

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

stackblitz[bot] avatar Nov 08 '22 17:11 stackblitz[bot]

Hi @MarmadileManteater 👋 Thank you very much for creating this PR and pointing out the issue 🙏 and i am sorry for this ultra late response. I've just reproduced it on the current version and imo this should be fixed with qwik internal events (if possible).

@adamdbradley my reproduction is on 0.18.1 / 0.2.1 on a new starter. added some more content, throttled the network to slow 3g and hit the blow my mind button. the scrolling is happening before the router outlet is loaded on the network panel.

Bildschirmfoto_2023-02-17_um_00_00_12

zanettin avatar Feb 16 '23 23:02 zanettin