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

Scrolling to top on navigation broke expected existing behavior #25145

Closed
ptrdom opened this issue Jul 26, 2018 · 4 comments
Closed

Scrolling to top on navigation broke expected existing behavior #25145

ptrdom opened this issue Jul 26, 2018 · 4 comments
Labels
area: router jira-sync regression Indicates than the issue relates to something that worked in a previous version
Milestone

Comments

@ptrdom
Copy link

ptrdom commented Jul 26, 2018

I'm submitting a...


[ X] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report  
[ ] Performance issue
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question
[ ] Other... Please describe:

Current behavior

I'm using navigation to update URL and trigger data reload for pagination, filtering and sorting of data table. On 6.1.0 it scrolls to the top of the page.

Expected behavior

Before 6.1.0, navigation didn't scroll to top and that was good behavior in my case, since I'm only changing table contents, so logically I don't need to scroll to top, I can just stay where the data table is. There should be an option, by providing some NavigationExtras or router scroll settings, to choose if you want to scroll to top, or stay where you were.

Minimal reproduction of the problem with instructions

Any kind of navigation scrolls to top now, there's no specific code for this issue.

What is the motivation / use case for changing the behavior?

I think there is no reason to not leave an option for things to stay as they were before 6.1.0. Preferably an optional runtime setting. I could implement this in some other way, but I'm not even sure how? Save scroll positions myself and pass them through URL parameters or cookies? Don't use navigation, update URL with location.go and use ngOnChanges() to update data? Those don't seem like some kind of "best practices".

Environment


Angular version: 6.1.0


Browser:
- [ X] Chrome (desktop) version 68.0.3440.75
- [ ] Chrome (Android) version XX
- [ ] Chrome (iOS) version XX
- [ ] Firefox version XX
- [ ] Safari (desktop) version XX
- [ ] Safari (iOS) version XX
- [ ] IE version XX
- [ ] Edge version XX
 
For Tooling issues:
- Node version: 8.11.3  
- Platform:  Linux 

Others:

@ericmartinezr
Copy link
Contributor

Try setting it to disabled https://next.angular.io/api/router/ExtraOptions#scrollPositionRestoration

@ptrdom
Copy link
Author

ptrdom commented Jul 26, 2018

@ericmartinezr
That did it. Interestingly enough, documentation notes that scrollPositionRestoration: 'disabled' is default, but if I don't set anything - RouterModule.forRoot(routes) - then the issue appears. Did I incorrectly understood the documentation or this is not intended? Also, even though my case is fixed by this now, it would still be nice to be able to specify scroll behavior not only at application initialization time, but also at navigation by using some additional parameters, and it would be nice to have scrollPositionRestoration: 'enabled' without always impacting navigation forward too.

@ericmartinezr
Copy link
Contributor

ericmartinezr commented Jul 26, 2018

@dope9967 probably the behavior changed while they were coding it and they forgot to update the docs. Leave the issue open so they can clarify the docs (and land it in the main angular.io website). And regarding your second point it would be nice to open a different issue with a feature request (I'm not yet familiarized with the scroll restoration thing so I'm not sure if that option exists at all or not)

Edit: related #24547

@ngbot ngbot bot added this to the needsTriage milestone Jul 26, 2018
@IgorMinar IgorMinar added the regression Indicates than the issue relates to something that worked in a previous version label Jul 26, 2018
@jasonaden jasonaden self-assigned this Aug 20, 2018
jasonaden added a commit to jasonaden/angular that referenced this issue Aug 21, 2018
jasonaden added a commit to jasonaden/angular that referenced this issue Aug 21, 2018
jasonaden added a commit to jasonaden/angular that referenced this issue Aug 21, 2018
jasonaden added a commit to jasonaden/angular that referenced this issue Aug 21, 2018
jasonaden added a commit that referenced this issue Aug 21, 2018
petebacondarwin pushed a commit to petebacondarwin/angular that referenced this issue Aug 22, 2018
FrederikSchlemmer pushed a commit to FrederikSchlemmer/angular that referenced this issue Jan 3, 2019
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: router jira-sync regression Indicates than the issue relates to something that worked in a previous version
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants