angular / angular

Deliver web apps with confidence 🚀
https://angular.dev
MIT License
94.88k stars 24.76k forks source link

Changing route doesn't scroll to top in the new page #7791

Closed netlander closed 5 years ago

netlander commented 8 years ago

When navigating from one route to another then scrolling down a bit and navigate back to the previous route, the page remains at the same scroll position.

Current behavior

Navigate from one view to another then scroll down a bit and navigate back to the previous view. the page remains at the same scroll position.

Expected/desired behavior

When navigating back to another route the page should be scrolled to the top.

zoechi commented 8 years ago

dup of #6595 ?

mgol commented 8 years ago

dup of #6595 ?

I don't think so.

zoechi commented 8 years ago

6946 shows a workaround

xak2000 commented 8 years ago

Expected/desired behavior

When navigating back to another route the page should be scrolled to the top.

It isn't expected behaviour for me. Expected behaviour for me is:

When navigating back to another route the page should be scrolled to position at which it was before going forward. This is the behaviour currently implemented in all browsers when navigating between normal pages (not SPA). But I never seen this behaviour in any implementation of angular1 router.

It is very annoying when you scroll down some list, click on list item to see details page, scrolling down details, then click browser Back button and not see the list page in exactly the same state as it was before.

The github, for example, implements this correctly: Open issues list and scroll down, then open any issue details, then click Back browser button. You will end up at exactly the same scroll position at which you were been before opening issue details page.

It will be very cool if angular2 router able to do it. It is really a killer feature I never seen in any other routers.

Edit There is dedicated issue for this behaviour now https://github.com/angular/angular/issues/10929

sulandr commented 8 years ago

may be would be better ta have some attribute or param to change this default scroll behaviour?

andyrue commented 7 years ago

Is there a recommended method for scrolling to the top in RC.4? The suggested workaround in #6946 no longer works.

cortopy commented 7 years ago

@andyrue I don't think this would be recommended but I posted an update on #6946

mhamel06 commented 7 years ago

In my case the window.scrollTo(0, 0) trick wouldn't work because I actually need to scroll to the top of an overflowing router-outlet. If you have a similar issue this is what I did as a workaround:

@Component({
  template: '<router-outlet (activate)="onActivate($event, outlet)" #outlet></router-outlet>,
})
export class MyParentComponent {
  onActivate(e, outlet){
    outlet.scrollTop = 0;
  }
}
Ariix commented 7 years ago

Any solution? None of the proposed solutions work. This seems to be working for me-for how long, not sure:

ngAfterViewChecked() { window.scrollTo(0, 0); }

cortopy commented 7 years ago

@Ariix wouldn't that scroll after every time change detection runs?

Ariix commented 7 years ago

@Cortopy Yes, yes it does. So still looking

robinkedia commented 7 years ago

Any update or milestone for this?

naveedahmed1 commented 7 years ago

Any update on this? Its annoying when you scroll down some list, click a list item to see details (which is on another route) you have to scroll up. If the next details page's length is small and the list page is very long and by chance you clicked the last item in the list. It would give an impression that there aren't any contents on details page (as the user has to scroll up to view contents).

zoechi commented 7 years ago

@naveedahmed1 https://github.com/angular/angular/issues/6595#issuecomment-244232725

naveedahmed1 commented 7 years ago

@zoechi I believe that comment is about URL with hash, in my case I don't' have any hash:

List page has a url: /list

details page has a url: /details/123

zoechi commented 7 years ago

@naveedahmed1 you can still call scrollIntoView() or scrollTop=0

robinkedia commented 7 years ago

@zoechi - Can you share the full code? i have parent child scenario where child routes are changing

sime commented 7 years ago

My work around involves the ElementRef class.

  constructor(
    private route: ActivatedRoute,
    private service: GuideService,
    private router: Router,
    private element: ElementRef
  ) {
    this.scrollUp = this.router.events.subscribe((path) => {
      element.nativeElement.scrollIntoView();
    });
  }
naveedahmed1 commented 7 years ago

How about this?

import { Component, OnInit} from '@angular/core';
import { Router, NavigationEnd } from '@angular/router';

@Component({
    moduleId: module.id,
    selector: 'app',
    templateUrl: './app.component.html'
})

export class AppComponent implements OnInit {
    constructor(private router: Router) {}

    ngOnInit() {
        this.router.events.subscribe((evt) => {
            if (!(evt instanceof NavigationEnd)) {
                return;
            }

            var scrollToTop = window.setInterval(function () {
                var pos = window.pageYOffset;
                if (pos > 0) {
                    window.scrollTo(0, pos - 20); // how far to scroll on each step
                } else {
                    window.clearInterval(scrollToTop);
                }
            }, 16); // how fast to scroll (this equals roughly 60 fps)
        });
    }
}
luxalpa commented 7 years ago
    router.events.filter(event => event instanceof NavigationEnd).subscribe(event => {
        window.scroll(0, 0);
    });

This seems to remember the scroll position (in Chrome and Firefox but not IE), but I have no idea why.

daco commented 7 years ago

@SmaugTheGreat Yes, that does work! But how is that even possible that it remembers the last scroll position?

naveedahmed1 commented 7 years ago

Its strange, when using @SmaugTheGreat solution i.e. window.scroll(0, 0); it remembers scroll position. But when using it window.scroll in window.setInterval, it doesn't remember scroll position.

fxck commented 7 years ago

why doesn't NavigationEnd contain params, query params etc of the route? only string?

shanehickeylk commented 7 years ago

A mixture of Smaugs solution above and Guilherme Meireles on stackoverflow does the trick for me. Also unsubscribing in ngOnDestroy to avoid leakage.

import { Component } from '@angular/core'; 
import { Router, NavigationEnd } from '@angular/router'; 
import { Subscription } from 'rxjs/Rx';

@Component({
    moduleId: module.id, 
    selector: ‘my-app’,
    template: `<router-outlet></router-outlet>`
})

export class AppComponent {
    routerSubscription: Subscription;

    constructor(private router: Router) {}

    ngOnInit() {
        this.routerSubscription = this.router.events
            .filter(event => event instanceof NavigationEnd)
            .subscribe(event => {
                document.body.scrollTop = 0;
            });
    }

    ngOnDestroy() {
        this.routerSubscription.unsubscribe();
    }
}
gowthamrodda commented 7 years ago

use 'ng2-page-scroll' module. As a quick response, i am posting one of my component. This one worked for me.

import {Component, OnInit, Inject } from '@angular/core'; import {Router, Route, NavigationEnd} from '@angular/router'; import { AppSharedService} from '../app.service'; import { DOCUMENT } from '@angular/platform-browser'; import { Subscription } from 'rxjs/Rx'; import { PageScrollService, PageScrollInstance } from 'ng2-page-scroll';

@Component({ selector: 'app-resources', templateUrl: './resources.component.html', styleUrls: ['./resources.component.scss'] }) export class ResourcesComponent {

routerSubscription: Subscription;

constructor(private router: Router, private SharedService: AppSharedService, private pageScrollService: PageScrollService, @Inject(DOCUMENT) private document: any,) { let pageScrollInstance: PageScrollInstance = PageScrollInstance.simpleInstance(this.document, '#app-give'); this.pageScrollService.start(pageScrollInstance);

}

}

spock123 commented 7 years ago

@shanehickeylk
I believe you only need to unsubscribe because you specifically save the subscription into a variable.

If you just called router.events..... .subscribe() directly, there is no subscription to unsubscribe.

Update: yes you do :) sorry guys

shanehickeylk commented 7 years ago

@spock123 I believe that the subscription is still "alive" even when it is not assigned to a variable. This is how angular knows an event has been triggered. The reason for assigning it to a variable is so that it can be accessed later and unsubscribed.

Here is a good article by Brian Love, where he goes into some additional detail on the topic: http://brianflove.com/2016/12/11/anguar-2-unsubscribe-observables/

spock123 commented 7 years ago

Aight, thanks.. need to rewrite some production asap ,😎😎

JoniJnm commented 7 years ago

Maybe you want go to top only in root router changes (not in children, because you can load routes with lazy load in f.e. a tabset)

Another way to solve it:

<router-outlet (deactivate)="onDeactivate()"></router-outlet>
onDeactivate() {
    document.body.scrollTop = 0;
}
HauntedSmores commented 7 years ago

Just want to report that I see this issue whenever I use a route resolver. Looks like it has to do with the timing of a component being destroyed/loaded in the router. When I use a resolver, seems the component loads really fast, too fast, and its like the router outlet is never truly empty or something so the scroll position gets saved from previous component. On all my other routes/components, however, this is not an issue

josh81 commented 7 years ago

I solved this issue by adding autoscroll="true" in the ui-view

iwish0 commented 7 years ago

josh81, what you said is for angular 1, not angular 2

kormic commented 7 years ago

@JoniJnm maybe using the renderer would be better?

<router-outlet (deactivate)="onDeactivate()"></router-outlet>

import { Component, Renderer } from '@angular/core';


  constructor(private renderer: Renderer) {
  }

  onDeactivate() {
    this.renderer.setElementProperty(document.body, "scrollTop", 0);
  }
}
mtpultz commented 7 years ago

I added this to my app.component::ngOnInit to get around scrolling issues after routing where native scrolling on the window or document body wasn't working due to the content being in the sidenav content container. In case this helps anyone temporarily until beta.4.

this.router
  .events
  .filter(event => event instanceof NavigationEnd)
  .subscribe(() => {
      const contentContainer = document.querySelector('.mat-sidenav-content') || this.window;
      contentContainer.scrollTo(0, 0);
  });
xe4me commented 7 years ago

I created a little service that handles this :

Tutorial

Source

Splaktar commented 7 years ago

@kormic using the Renderer is a good idea! However, Renderer is deprecated, so Renderer2 should be used now. Also setElementProperty() was changed to setProperty() in Renderer2.

I combined this with @kormic and @mtpultz's workarounds and this seems to be working fairly well for me atm:

    this.router.events.filter(event => event instanceof NavigationEnd).subscribe(() => {
      this.renderer.setProperty(document.body, 'scrollTop', 0);
    });
ysus commented 7 years ago

@Splaktar i have a angular universal app. how to avoid the server exception. the 3 way works but i have a server message, :

window.scrollTo(0,0); //window not defined. this.element.nativeElement.scrollIntoView(false); //scrollIntoView not defined this.renderer.setProperty(document.body,'scrollTop',0); //document not defined

ysus commented 7 years ago

After an hours of work finally found a solution for server side rendering

if (isPlatformBrowser(this.platformId)) {
                var scrollingElement = this.document.scrollingElement || this.document.documentElement;
                this.renderer.setProperty(scrollingElement ,'scrollTop',0);
            }
DavidFrahm commented 7 years ago

FYI, anyone who cares about this probably also cares about preserving scroll position for previous views when navigating "Back". That is in a separate issue #10929.

JohnGalt1717 commented 7 years ago

My question is how is this combined with #hash anchors not supported out of the box and not known after like 10 iterations of routers in Angular?

  1. When I click on a link it should scroll to the top.
  2. If I click on a link and it has a # it should go to that page and scroll to it, or if it's the same page scroll down to it.
  3. If I hit back or forward, it should go to where I was.

This isn't hard. I thought this was solved in like 1992. Someone at Google please get into a stand-up and knock heads. This needs to be fixed yesterday. I shouldn't have to write hacks just to do what every browser since Lynx has been able to do.

alanhg commented 6 years ago

app.component.ts,add following code this.router.events.filter(event => event instanceof NavigationEnd).subscribe((e: NavigationEnd) => { document.body.scrollTop = 0; });

JohnGalt1717 commented 6 years ago

That amongst other things doesn't work properly with # notation on pages etc. This needs to be fixed properly by the Angular team in the root code per the requirements above.

padrian commented 6 years ago

check this out it helped me;

https://stackoverflow.com/questions/39601026/angular-2-scroll-to-top-on-route-change

mhdatie commented 6 years ago

This really needs to be the expected behaviour in SPAs. We are trying a workaround for our production app and it really supposed to be out of the box solution.

JohnYoungers commented 6 years ago

I would agree with most of the prior comments: since my application isn't in Production yet I'm holding off hoping this will be implemented without any workarounds needed.

ghost commented 6 years ago

The autoscroll no longer works in Firefox :-/ 55.0.1 (64 bit)

Chrome works as expected though.

ysus commented 6 years ago

this works for me let number = window.scrollY || window.pageYOffset || document.body.scrollTop + (document.documentElement && document.documentElement.scrollTop || 0)

dockleryxk commented 6 years ago

My team has been using what the angular team uses on this repo on angular.io. Just make a service and inject it like usual. Then, on ngAfterViewInit or ngAfterContentInit on each page you want this behavior, just call this.[scroll service variable name].scrollToTop(). Finally, you'll need to add this to the top of <body> in index.html: <div id="top-of-page"></div>

ByronBeleris commented 6 years ago

I just used this.router.navigate(['myNextPage']).then(() => { window.scrollTo(0, 0) });

and it worked.

JohnGalt1717 commented 6 years ago

Again, that's isn't what it's supposed to do.

If you go to a regular old website and hit the back button it goes back AND keeps the exact position on the page where you clicked from so you're brought back to EXACTLY where you left off.

Angular needs to have this built in and it needs to just work. No hacking, no providing a bad estimation of the functionality. The real deal working exactly as a user would expect automatically.