-
Notifications
You must be signed in to change notification settings - Fork 3
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
[Ready for Review] Save scroll positions and results to show #381
Conversation
[diff-counting] Significant lines: 149. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feature successfully stores scroll location and makes browsing the site much more easy and seamless! Saving the scroll position only when the session is active and not checking the scroll position for large movements etc. are good details.
I'm only requesting one small change: after testing this by extensively browsing the site I found the smooth scroll setting to give me motion sickness. As our site aims for long periods of browsing, a scroll position adjustment without an animation will be more ideal. To fix this you simply have to change the scroll behavior from 'smooth' to 'auto.' Thanks again for a simple yet integral feature!
Summary
This pull request includes several changes to improve the user experience by saving your scroll position and number of results loaded on the Home Page, Search Results Page, and the 4 location pages (north, west, etc..). The most important changes include the addition of scroll position saving, new properties for the
ApartmentCards
component, and the updates to multiple pages to utilize these new features.Enhancements to
ApartmentCards
component:frontend/src/components/ApartmentCard/ApartmentCards.tsx
: Added new optional propertiesinitialResultsToShow
andonMoreResultsLoaded
to handle the initial number of results to display and to trigger a callback when more results are loaded. [1] [2] [3]Scroll position saving:
frontend/src/utils/saveScrollPosition.ts
: Introduced a new hookuseSaveScrollPosition
to save and restore the scroll position across page navigations.Page updates to utilize new features:
frontend/src/pages/ApartmentPage.tsx
: Added auseEffect
to scroll to the top of the page when the component mounts.frontend/src/pages/HomePage.tsx
: IntegrateduseSaveScrollPosition
to save the scroll position and updated the component to use the newinitialResultsToShow
andonMoreResultsLoaded
properties ofApartmentCards
. [1] [2] [3]frontend/src/pages/LandlordPage.tsx
: Added auseEffect
to scroll to the top of the page when the component mounts.frontend/src/pages/LocationPage.tsx
: IntegrateduseSaveScrollPosition
and updated the component to use the newinitialResultsToShow
andonMoreResultsLoaded
properties ofApartmentCards
. [1] [2] [3] [4]frontend/src/pages/SearchResultsPage.tsx
: IntegrateduseSaveScrollPosition
and updated the component to use the newinitialResultsToShow
andonMoreResultsLoaded
properties ofApartmentCards
. [1] [2] [3]Test Plan
Tested scrolling features on Chromium browsers including mobile, and on Safari.
Notes
I could not find a way to remove the event listener on scroll quick enough before the page changes. When the page changes, the ApartmentCards component vanishes from the page in an instant, which pulls your scroll immediately to the top. This causes the scroll to save that top position when you change pages. To resolve this, I set a max scroll difference which would prevent it from updating the scrollY in sessionStorage if the scroll position changes too much per update. However, this means that if you scroll really fast to the bottom, it will not save.