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(openchallenges): customize scroll position restoration with different navigation types #2189

Closed
wants to merge 19 commits into from

Conversation

rrchai
Copy link
Contributor

@rrchai rrchai commented Oct 2, 2023

Changelogs

  • Add a service to enable custom scroll position restoration. The built-in scrollPositionRestoration seems not to support the navigations when only parameters change. For example, in the search pages, the scroll position is always reset to "top" when filter values changes (if scrollPositionRestoration: 'top'. Therefore, adding a custom service could overwrite default scroll reset behavior and restore the previous position depending on different navigations:

    CustomScrollRestorationService: 
    A service for restoring scroll positions based on the method of navigation.
    It helps preserve the user's scroll position during various types of navigation actions.
    - Backward navigation: Scrolls to the previous position.
    - Forward navigation: Scrolls to the top of the page.
    - Anchor navigation: Scrolls to a specified anchor (no example currently).
    - Param navigation: Maintains the current scroll position when base url is the same, i.e. /challenge -> /challenge?status=active
    - Same url navigation (was not set before): Scrolls to the top of the page, i.e. /home -> /home
    

Preview

Backward navigation

backward

Forward navigation

forward

Same url navigation

sameurl

Param navigation

param

@rrchai rrchai self-assigned this Oct 2, 2023
@rrchai
Copy link
Contributor Author

rrchai commented Oct 2, 2023

Some attempts using custom directive to prevent scroll position from being reset to 'top' is unsuccessful.

When navigate back from challenge-profile page to challenge-search page, the custom directive works to keep the previous scroll position. However, the scroll position will still be reset to 'top' when filters on the challenge-search page change.

Looking for other options...

@rrchai
Copy link
Contributor Author

rrchai commented Oct 3, 2023

Update

It looks like scrollPositionRestoration: 'enabled' will overwrite the manual scrolls at least in ngOnit. Not sure if it's because of race issues. I am going to test window.scroll(0, 200) in different lifecycle hooks with scrollPositionRestoration: 'enabled'.

Results:

  • ngOnit : ❌
  • ngAfterContentInit: ❌
  • ngAfterViewInit: ❌

The scroll position-y is not changed to 200 in all three lifecycle hooks until scrollPositionRestoration: 'enabled' is commented out.

Update 2

using viewportScroller.scrollToPosition will work with scrollPositionRestoration

@rrchai rrchai changed the title feat(angular): Restore scroll position when routes change fix(angular): Restore scroll position when routes change Oct 3, 2023
@rrchai rrchai changed the title fix(angular): Restore scroll position when routes change fix(openchallenges): Restore scroll position when routes change Oct 20, 2023
@rrchai rrchai changed the title fix(openchallenges): Restore scroll position when routes change fix(openchallenges): restore scroll position when routes change Oct 20, 2023
@rrchai
Copy link
Contributor Author

rrchai commented Oct 23, 2023

I initially use navigationTrigger === 'imperative' of NavigationStart to check if it's the same page navigation. However, the navigation of "home > challenge-search > home" is also an imperative trigger and it will remain the scroll position, which is unexpected.

Looking for options to store the previous url before navigation...

Update

Looking for options to store the previous url before navigation...

pairwise() can do the trick.

@rrchai rrchai marked this pull request as ready for review October 23, 2023 22:12
@rrchai rrchai changed the title fix(openchallenges): restore scroll position when routes change fix(openchallenges): customize scroll position restoration with different navigation types Oct 23, 2023
Copy link
Member

@tschaffter tschaffter left a comment

Choose a reason for hiding this comment

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

This looks like an elegant solution. I need to confirm that the use of window will not break SSR before approving this PR.

apps/openchallenges/app/src/app/app.config.ts Outdated Show resolved Hide resolved
@rrchai
Copy link
Contributor Author

rrchai commented Oct 24, 2023

I will explore the simpler solution @tschaffter suggested and convert this pr to draft.

@rrchai rrchai marked this pull request as draft October 24, 2023 16:07
@rrchai
Copy link
Contributor Author

rrchai commented Oct 26, 2023

Closing this - the new approach to address the issue is tracked in #2273

@rrchai rrchai closed this Oct 26, 2023
@rrchai rrchai deleted the reset-scrollbar branch January 10, 2024 19:56
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.

[Bug] Reset scrolling position when navigating to another page (Take 2)
2 participants