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 scroll restoration in the Pagination component #1508

Merged
merged 5 commits into from
Nov 20, 2023
Merged

Conversation

blittle
Copy link
Contributor

@blittle blittle commented Nov 13, 2023

WHY are these changes introduced?

Fix the Pagination component to always restore scroll correctly on back/forth navigation.

Expected behavior and design considerations

This fix has the side effect of causing a hydration error only when the user refreshes an already paginated page. This is because during a refresh, the Paginated list is rendered with the browser's navigation state, not the state returned by the server. Navigation state might contain many pages of data, all of which is necessary to properly render the page height and restore scroll position. The result is a hydration mismatch between the browser and the server. We feel it isn't often the user would hard refresh the page, and when doing so, a hydration mismatch and flash of rendered content is an acceptable tradeoff for accurately maintaining the scroll location.

HOW to test your changes?

On the /products page as well as the /collections/:handle do the following tasks (the collection page paginated on scroll):

  1. Load pages all the way until it ends. Make sure there are no duplicate items in the list.
  2. Navigate to a product towards the end of the list. Then click the back button. Scroll should exactly restore.
  3. Make sure at the end of the list the Load more button does not show up.
  4. Go the end of the list. Copy the URL, and paste it in an incognito tab. Click the previous button until it disappears (it should disappear once you reach the start of the list). Make sure there are no duplicates in the list.
  5. Disable JavaScript. You should be able to navigate between pages.

Another thing I already tested, but if you want to go the extra mile, create a link on a collection page to another collection page. Paginate through one collection page, then click a link to directly go to another collection page (same route, different params). Make sure that products don't bleed between the pages and pagination state is reset.

Checklist

  • I've read the Contributing Guidelines
  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've added a changeset if this PR contains user-facing or noteworthy changes
  • I've added tests to cover my changes
  • I've added or updated the documentation

@blittle blittle merged commit 74ea1db into main Nov 20, 2023
9 checks passed
@blittle blittle deleted the bl-fix-scroll-reset branch November 20, 2023 21:00
Copy link
Contributor

@juanpprieto juanpprieto left a comment

Choose a reason for hiding this comment

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

🙏🏼

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