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

Remove scroll position set/get for Community and Home #1472

Merged
merged 2 commits into from
Jun 22, 2023

Conversation

alectrocute
Copy link
Contributor

@alectrocute alectrocute commented Jun 22, 2023

Hi Lemdevs!

Pretty grave bug here with our scrolling:

Untitled.mov

This used to not be a problem in 0.17 because our routes were different, eg. https://sh.itjust.works/home/data_type/Post/listing_type/Local/sort/Active/page/2 vs. query params.

I propose we:

  • Remove scroll position set/get for Community and Home, eg. routes where pagination is a critical and frequently used part of the view

@alectrocute
Copy link
Contributor Author

@SleeplessOne1917 This seems like a pretty big deal. Is this how we should resolve it? Is there any reason to restore scroll position on these specific routes?

@@ -1,5 +1,6 @@
export default function restoreScrollPosition(context: any) {
Copy link
Member

Choose a reason for hiding this comment

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

Are this and saveScrollPosition still used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, on Profile, Search, and Post.

Copy link
Member

@SleeplessOne1917 SleeplessOne1917 left a comment

Choose a reason for hiding this comment

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

Is there any reason to restore scroll position on these specific routes?

Not that I know of.

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