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

Implement scroll to top on page navigate #514

Closed
wants to merge 3 commits into from
Closed

Conversation

lubej
Copy link
Collaborator

@lubej lubej commented Jun 13, 2023

No description provided.

@lubej lubej requested review from buberdds, lukaw3d and csillag June 13, 2023 14:04
@github-actions
Copy link

github-actions bot commented Jun 13, 2023

Deployed to Cloudflare Pages

Latest commit: 9784eb9d623295c92a57798f2a6cba53d6657ed0
Status:✅ Deploy successful!
Preview URL: https://1998bf82.oasis-explorer.pages.dev

@lubej
Copy link
Collaborator Author

lubej commented Jun 13, 2023

[Mobile] When accessing a dashboard it isn't scrolled to top
As you can see in the recording the dashboard page does not load all the way to the top partially hiding the search feature and logo: /video missing/

@lubej lubej force-pushed the ml/scroll-to-top branch from 262fabd to 0db3f33 Compare June 13, 2023 14:18
@lukaw3d
Copy link
Member

lukaw3d commented Jun 14, 2023

My preferred UX would be: scroll to top on navigation, but keep position when refreshing and reopening tabs

@lukaw3d
Copy link
Member

lukaw3d commented Jun 14, 2023

and restore position when clicking back (or forward)

@csillag
Copy link
Contributor

csillag commented Jun 15, 2023

My preferred UX would be: scroll to top on navigation, but keep position when refreshing and reopening tabs, and restore position when clicking back (or forward)

👍🏻

@lubej lubej force-pushed the ml/scroll-to-top branch 2 times, most recently from edd4199 to c3c76b6 Compare June 16, 2023 08:31
@lubej lubej force-pushed the ml/scroll-to-top branch from c3c76b6 to 9784eb9 Compare June 16, 2023 08:33
@lubej
Copy link
Collaborator Author

lubej commented Jun 16, 2023

My preferred UX would be: scroll to top on navigation, but keep position when refreshing and reopening tabs, and restore position when clicking back (or forward)

👍🏻

Implemented, ScrollRestoration did not quite cover all the cases, especially on forward and page refresh. As the case for reopening the tabs, it works as long there is an active tab open - since the scroll map is saved in sessionStorage.

@lubej lubej requested a review from buberdds June 16, 2023 08:37
@lukaw3d
Copy link
Member

lukaw3d commented Jun 20, 2023

@lubej can you check if #567 (https://2010762f.oasis-explorer.pages.dev/) satisfies all requirements? I checked the basics, and seems to behave the same/better, without a custom implementation

@lubej
Copy link
Collaborator Author

lubej commented Jun 21, 2023

@lubej can you check if #567 (https://2010762f.oasis-explorer.pages.dev/) satisfies all requirements? I checked the basics, and seems to behave the same/better, without a custom implementation

@lukaw3d one thing that does not work is reopening the tab, but given how much it simplifies the code, I would go with your implementation. Not sure why I was facing issues with ScrollRestoration -> forward particularly. When in your case it seems to work fine.

@lukaw3d lukaw3d closed this in #567 Jun 21, 2023
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.

4 participants