Skip to content

Commit

Permalink
fixup! fix(router): Delay router scroll event until navigated compone…
Browse files Browse the repository at this point in the history
…nts have rendered
  • Loading branch information
atscott committed Oct 17, 2022
1 parent 8c1ea3d commit a3b17a1
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 13 deletions.
25 changes: 15 additions & 10 deletions packages/router/src/router_scroller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import {ViewportScroller} from '@angular/common';
import {Injectable, InjectionToken, OnDestroy} from '@angular/core';
import {Injectable, InjectionToken, NgZone, OnDestroy} from '@angular/core';
import {Unsubscribable} from 'rxjs';

import {NavigationEnd, NavigationStart, Scroll} from './events';
Expand All @@ -29,7 +29,8 @@ export class RouterScroller implements OnDestroy {

constructor(
private router: Router,
/** @docsNotRequired */ public readonly viewportScroller: ViewportScroller, private options: {
/** @docsNotRequired */ public readonly viewportScroller: ViewportScroller,
private readonly zone: NgZone, private options: {
scrollPositionRestoration?: 'disabled'|'enabled'|'top',
anchorScrolling?: 'disabled'|'enabled'
} = {}) {
Expand Down Expand Up @@ -85,14 +86,18 @@ export class RouterScroller implements OnDestroy {
}

private scheduleScrollEvent(routerEvent: NavigationEnd, anchor: string|null): void {
// The scroll event needs to be delayed until after change detection. Otherwise, we may attempt
// to restore the scroll position before the router outlet has fully rendered the component by
// executing its update block of the template function.
setTimeout(() => {
this.router.triggerEvent(new Scroll(
routerEvent, this.lastSource === 'popstate' ? this.store[this.restoredId] : null,
anchor));
}, 0);
this.zone.runOutsideAngular(() => {
// The scroll event needs to be delayed until after change detection. Otherwise, we may
// attempt to restore the scroll position before the router outlet has fully rendered the
// component by executing its update block of the template function.
setTimeout(() => {
this.zone.run(() => {
this.router.triggerEvent(new Scroll(
routerEvent, this.lastSource === 'popstate' ? this.store[this.restoredId] : null,
anchor));
});
}, 0);
});
}

/** @nodoc */
Expand Down
11 changes: 8 additions & 3 deletions packages/router/test/router_scroller.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {FnParam} from '@angular/compiler/src/output/output_ast';
import {fakeAsync, tick} from '@angular/core/testing';
import {DefaultUrlSerializer, Event, NavigationEnd, NavigationStart} from '@angular/router';
import {Subject} from 'rxjs';
Expand All @@ -15,6 +16,10 @@ import {Scroll} from '../src/events';
import {RouterScroller} from '../src/router_scroller';

// TODO: add tests that exercise the `withInMemoryScrolling` feature of the provideRouter function
const fakeZone = {
runOutsideAngular: (fn: any) => fn(),
run: (fn: any) => fn()
};
describe('RouterScroller', () => {
it('defaults to disabled', () => {
const events = new Subject<Event>();
Expand All @@ -28,7 +33,7 @@ describe('RouterScroller', () => {
'viewportScroller',
['getScrollPosition', 'scrollToPosition', 'scrollToAnchor', 'setHistoryScrollRestoration']);
setScroll(viewportScroller, 0, 0);
const scroller = new RouterScroller(router, router);
const scroller = new RouterScroller(router, router, fakeZone as any);

expect((scroller as any).options.scrollPositionRestoration).toBe('disabled');
expect((scroller as any).options.anchorScrolling).toBe('disabled');
Expand Down Expand Up @@ -195,8 +200,8 @@ describe('RouterScroller', () => {
['getScrollPosition', 'scrollToPosition', 'scrollToAnchor', 'setHistoryScrollRestoration']);
setScroll(viewportScroller, 0, 0);

const scroller =
new RouterScroller(router, viewportScroller, {scrollPositionRestoration, anchorScrolling});
const scroller = new RouterScroller(
router, viewportScroller, fakeZone as any, {scrollPositionRestoration, anchorScrolling});
scroller.init();

return {events, viewportScroller, router};
Expand Down

0 comments on commit a3b17a1

Please sign in to comment.