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

Restore List Scroll Position on Edit and Create Views side effects #9774

Merged
merged 10 commits into from Apr 16, 2024

Conversation

djhi
Copy link
Contributor

@djhi djhi commented Apr 11, 2024

Problem

When users have a long list page, navigate from it to an edit or create page, the onSuccess redirection of those pages redirect them to the list but loose their previous scroll position. This is bad UX.

Solution

  • Introduce the useRestoreScrollPosition hook that tracks the page scroll position and restore it when the component mounts.
  • Use it in the Resource component for list only
  • Remove the _scrollToTop state when redirecting to a list view

How to test

  • Run the demo
  • Go to the visitor page and set the page size to 50
  • scroll to the bottom and edit a record

@djhi djhi added RFR Ready For Review v5 labels Apr 11, 2024
Copy link
Member

@fzaninotto fzaninotto left a comment

Choose a reason for hiding this comment

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

Can you explain how to test this feature?

packages/ra-core/src/routing/useRedirect.ts Outdated Show resolved Hide resolved
@djhi djhi added WIP Work In Progress and removed RFR Ready For Review labels Apr 11, 2024
@fzaninotto
Copy link
Member

Now I'm thinking: what if there is more than one <List> per page? Or if the List is not the main page component (e.g. in the TagEdit page in the simple example)? We don't have any way to disable the scroll restoration in List. I think that's a problem.

@djhi
Copy link
Contributor Author

djhi commented Apr 12, 2024

Should we add a disableScrollRestoration prop?

@fzaninotto
Copy link
Member

Other suggestion: scroll restoration should be dealt with earlier, in the CoreAdminRoutes, instead of in the List component.

Copy link
Member

@fzaninotto fzaninotto left a comment

Choose a reason for hiding this comment

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

Nearly there!

* import { RestoreScrollPosition } from 'ra-core';
*
* const MyCustomPage = () => {
* <RestoreScrollPosition key="my-list>
Copy link
Member

Choose a reason for hiding this comment

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

syntax error, missing quote

packages/ra-core/src/routing/useRedirect.ts Outdated Show resolved Hide resolved
Co-authored-by: Francois Zaninotto <francois@marmelab.com>
@fzaninotto fzaninotto merged commit ab6c201 into next Apr 16, 2024
12 checks passed
@fzaninotto fzaninotto deleted the scroll-restoration branch April 16, 2024 14:28
@fzaninotto fzaninotto added this to the 5.0.0 milestone Apr 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFR Ready For Review v5
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants