fix(router): default scroll position restoration to disabled

Fixes angular#25145 FW-305 #resolve
jasonaden · Aug 21, 2018 · ecdec9c · ecdec9c
1 parent b05d4a5
commit ecdec9c
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 3 deletions.
diff --git a/packages/router/src/router_module.ts b/packages/router/src/router_module.ts
@@ -156,6 +156,11 @@ export class RouterModule {
    * from parent to child routes.
    */
   static forRoot(routes: Routes, config?: ExtraOptions): ModuleWithProviders<RouterModule> {
+
+    config = config || {};
+    config.scrollPositionRestoration = config.scrollPositionRestoration || 'disabled';
+    config.anchorScrolling = config.anchorScrolling || 'disabled';
+
     return {
       ngModule: RouterModule,
       providers: [
@@ -166,7 +171,7 @@ export class RouterModule {
           useFactory: provideForRootGuard,
           deps: [[Router, new Optional(), new SkipSelf()]]
         },
-        {provide: ROUTER_CONFIGURATION, useValue: config ? config : {}},
+        {provide: ROUTER_CONFIGURATION, useValue: config},
         {
           provide: LocationStrategy,
           useFactory: provideLocationStrategy,

diff --git a/packages/router/src/router_scroller.ts b/packages/router/src/router_scroller.ts
@@ -29,7 +29,11 @@ export class RouterScroller implements OnDestroy {
       /** @docsNotRequired */ public readonly viewportScroller: ViewportScroller, private options: {
         scrollPositionRestoration?: 'disabled' | 'enabled' | 'top',
         anchorScrolling?: 'disabled'|'enabled'
-      } = {}) {}
+      } = {}) {
+    // Default both options to 'disabled'
+    options.scrollPositionRestoration = options.scrollPositionRestoration || 'disabled';
+    options.anchorScrolling = options.anchorScrolling || 'disabled';
+  }
 
   init(): void {
     // we want to disable the automatic scrolling because having two places

diff --git a/packages/router/test/router_scroller.spec.ts b/packages/router/test/router_scroller.spec.ts
@@ -15,6 +15,24 @@ import {Scroll} from '../src/events';
 import {RouterScroller} from '../src/router_scroller';
 
 describe('RouterScroller', () => {
+  it('defaults to disabled', () => {
+    const events = new Subject<RouterEvent>();
+    const router = <any>{
+      events,
+      parseUrl: (url: any) => new DefaultUrlSerializer().parse(url),
+      triggerEvent: (e: any) => events.next(e)
+    };
+
+    const viewportScroller = jasmine.createSpyObj(
+        'viewportScroller',
+        ['getScrollPosition', 'scrollToPosition', 'scrollToAnchor', 'setHistoryScrollRestoration']);
+    setScroll(viewportScroller, 0, 0);
+    const scroller = new RouterScroller(router, router);
+
+    expect((scroller as any).options.scrollPositionRestoration).toBe('disabled');
+    expect((scroller as any).options.anchorScrolling).toBe('disabled');
+  });
+
   describe('scroll to top', () => {
     it('should scroll to the top', () => {
       const {events, viewportScroller} =

diff --git a/packages/router/testing/src/router_testing_module.ts b/packages/router/testing/src/router_testing_module.ts
@@ -182,11 +182,16 @@ export function setupTestingRouter(
 export class RouterTestingModule {
   static withRoutes(routes: Routes, config?: ExtraOptions):
       ModuleWithProviders<RouterTestingModule> {
+
+    config = config || {};
+    config.scrollPositionRestoration = config.scrollPositionRestoration || 'disabled';
+    config.anchorScrolling = config.anchorScrolling || 'disabled';
+
     return {
       ngModule: RouterTestingModule,
       providers: [
         provideRoutes(routes),
-        {provide: ROUTER_CONFIGURATION, useValue: config ? config : {}},
+        {provide: ROUTER_CONFIGURATION, useValue: config},
       ]
     };
   }