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

Disable smooth scroll in navigation attempts #43529

Closed
wants to merge 2 commits into from

Conversation

gurkerl83
Copy link
Contributor

@gurkerl83 gurkerl83 commented Nov 29, 2022

Disable smooth scroll in navigation attempts, restore scroll behaviour when the navigation completes.

The current implementation seems to be wrong, as the scroll behaviour "auto" immediately or during scrolling switches back to "smooth" or the globally defined behaviour of the html tag.

Local experiments showed that the scrolling behaviour "auto" must be initiated at the start of a route change "routeChangeStart" and must be maintained until a route change is completed "routeChangeComplete".

Pre-condition:
The scroll behaviour "smooth" must either be defined globally in css for the html element. From the hook provided this is done once in the effect-lifecycle, however a user defined configuration seems better, especially as the behaviour can be integrated into Next as described in the next step.

Summary of the lifecycle:

  • Subscribe to routeChangeStart, routeChangeComplete events.
  • The route change starts Switch the scroll behavior to 'auto' Scroll immediately to the top and hold the value until the route change finishes.
  • The route change finishes Switch back to the default value specified in global css for the html element. For smooth-scrolling smooth, the behavior is 'smooth'. Hash changes are no route changes; the result is smooth scrolling on hash changes.

Bug

  • Related issues linked using fixes #number
  • Integration tests added
  • Errors have a helpful link attached, see contributing.md

Feature

  • Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
  • Related issues linked using fixes #number
  • e2e tests added
  • Documentation added
  • Telemetry added. In case of a feature if it's used or not.
  • Errors have a helpful link attached, see contributing.md

Documentation / Examples

  • Make sure the linting passes by running pnpm build && pnpm lint
  • The "examples guidelines" are followed from our contributing doc

Disable smooth scroll in navigation attempts, restore scroll behaviour when the navigation completes.

The current implementation seems to be wrong, as the scroll behaviour "auto" immediately or during scrolling switches back to "smooth" or the globally defined behaviour of the html tag.

Local experiments showed that the scrolling behaviour "auto" must be initiated at the start of a route change "routeChangeStart" and must be maintained until a route change is completed "routeChangeComplete".

Pre-condition:
The scroll behaviour "smooth" must either be defined globally in css for the html element. From the hook provided this is done once in the effect-lifecycle, however a user defined configuration seems better, especially as the behaviour can be integrated into Next as described in the next step.

Summary of the lifecycle:
- Subscribe to routeChangeStart, routeChangeComplete events.
- The route change starts
   Switch the scroll behavior to 'auto'
   Scroll immediately to the top and hold the value until the route change finishes.
- The route change finishes
   Switch back to the default value specified in global css for the html element.
   For smooth-scrolling smooth, the behavior is 'smooth'. Hash changes are no route changes;
   the result is smooth scrolling on hash changes.
@gurkerl83
Copy link
Contributor Author

gurkerl83 commented Nov 29, 2022

fixes #40719

@gurkerl83
Copy link
Contributor Author

Can someone review the pr, please?

@jankaifer
Copy link
Contributor

jankaifer commented Dec 8, 2022

Hi @gurkerl83, It should be fixed now. I'll close this PR, but thanks a ton for raising the issue, giving us repro, and trying to tackle it with PR.

Open a new issue please if you still have issues with canary

@jankaifer jankaifer closed this Dec 8, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants