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

fix(ui): Scroll restoration #1833

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

yeinerfonseca
Copy link
Contributor

@yeinerfonseca yeinerfonseca commented Jul 7, 2024

This adds a reusable hook to restore positioning in any route with an scrollable element.
The scroll positioning will be restored regardless of the navigation type.

An enhancement could be implemented for the following scenario:
Say we have route foo/bar, if it accessed through a POP navigation type, then scroll is restored
to the last positioning. Otherwise, if navigation type is PUSH, then scroll is restored to zero (top).

Let me know your thoughts.

@yeinerfonseca yeinerfonseca changed the title Fix/1601: Scroll restoration fix(ui): Scroll restoration Jul 7, 2024
Copy link
Member

@elliotcourant elliotcourant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like how you implemented the scroll restoration in such a way that it can easily be reused by basically any component. Thank you!

I'll play around with this locally here soon, but i want to get this merged since it'll simplify a lot of stuff!

I'll take a deeper look into the code then, and merge/comment from there!

return context;
}

export function ScrollPositionProvider({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This entire thing is fantastic, I need to play around with it for a bit. But I really love this implementation.

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

Successfully merging this pull request may close these issues.

2 participants