fix(router): Ensure anchor scrolling happens on ignored same URL navigations (#48025)

atscott · AndrewKushnir · commit 1f055b90b65c · 2023-02-17T11:00:27.000-08:00
The Router scroller only listens for NavigationEnd events. However, the default behavior of the Router is to ignore navigations to the same URL. This breaks the anchor scrolling when clicking on an anchor whose fragment is already in the URL. fixes #29099 BREAKING CHANGE: The `Scroll` event's `routerEvent` property may also be a `NavigationSkipped` event. Previously, it was only a `NavigationEnd` event. PR Close #48025
diff --git a/goldens/public-api/router/index.md b/goldens/public-api/router/index.md
@@ -950,15 +950,15 @@ export type RunGuardsAndResolvers = 'pathParamsChange' | 'pathParamsOrQueryParam
 // @public
 export class Scroll {
     constructor(
-    routerEvent: NavigationEnd,
+    routerEvent: NavigationEnd | NavigationSkipped,
     position: [number, number] | null,
     anchor: string | null);
     // (undocumented)
     readonly anchor: string | null;
     // (undocumented)
     readonly position: [number, number] | null;
     // (undocumented)
-    readonly routerEvent: NavigationEnd;
+    readonly routerEvent: NavigationEnd | NavigationSkipped;
     // (undocumented)
     toString(): string;
     // (undocumented)
diff --git a/packages/router/src/events.ts b/packages/router/src/events.ts
@@ -582,7 +582,7 @@ export class Scroll {
 
   constructor(
       /** @docsNotRequired */
-      readonly routerEvent: NavigationEnd,
+      readonly routerEvent: NavigationEnd|NavigationSkipped,
 
       /** @docsNotRequired */
       readonly position: [number, number]|null,
diff --git a/packages/router/src/router_scroller.ts b/packages/router/src/router_scroller.ts
@@ -10,9 +10,8 @@ import {ViewportScroller} from '@angular/common';
 import {Injectable, InjectionToken, NgZone, OnDestroy} from '@angular/core';
 import {Unsubscribable} from 'rxjs';
 
-import {NavigationEnd, NavigationStart, Scroll} from './events';
+import {NavigationEnd, NavigationSkipped, NavigationSkippedCode, NavigationStart, Scroll} from './events';
 import {NavigationTransitions} from './navigation_transition';
-import {Router} from './router';
 import {UrlSerializer} from './url_tree';
 
 export const ROUTER_SCROLLER = new InjectionToken<RouterScroller>('');
@@ -61,6 +60,12 @@ export class RouterScroller implements OnDestroy {
       } else if (e instanceof NavigationEnd) {
         this.lastId = e.id;
         this.scheduleScrollEvent(e, this.urlSerializer.parse(e.urlAfterRedirects).fragment);
+      } else if (
+          e instanceof NavigationSkipped &&
+          e.code === NavigationSkippedCode.IgnoredSameUrlNavigation) {
+        this.lastSource = undefined;
+        this.restoredId = 0;
+        this.scheduleScrollEvent(e, this.urlSerializer.parse(e.url).fragment);
       }
     });
   }
@@ -86,7 +91,8 @@ export class RouterScroller implements OnDestroy {
     });
   }
 
-  private scheduleScrollEvent(routerEvent: NavigationEnd, anchor: string|null): void {
+  private scheduleScrollEvent(routerEvent: NavigationEnd|NavigationSkipped, anchor: string|null):
+      void {
     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
diff --git a/packages/router/test/bootstrap.spec.ts b/packages/router/test/bootstrap.spec.ts
@@ -455,7 +455,7 @@ describe('bootstrap', () => {
               scrollPositionRestoration: 'enabled',
               anchorScrolling: 'enabled',
               scrollOffset: [0, 100],
-              onSameUrlNavigation: 'reload'
+              onSameUrlNavigation: 'ignore',
             })
       ],
       declarations: [TallComponent, RootCmp],
@@ -494,13 +494,16 @@ describe('bootstrap', () => {
 
     await router.navigateByUrl('/aa#marker2');
     await resolveAfter(100);
-    expect(window.pageYOffset).toBeGreaterThanOrEqual(5900);
-    expect(window.pageYOffset).toBeLessThan(6000);  // offset
+    expect(window.scrollY).toBeGreaterThanOrEqual(5900);
+    expect(window.scrollY).toBeLessThan(6000);  // offset
 
-    await router.navigateByUrl('/aa#marker3');
+    // Scroll somewhere else, then navigate to the hash again. Even though the same url navigation
+    // is ignored by the Router, we should still scroll.
+    window.scrollTo(0, 3000);
+    await router.navigateByUrl('/aa#marker2');
     await resolveAfter(100);
-    expect(window.pageYOffset).toBeGreaterThanOrEqual(8900);
-    expect(window.pageYOffset).toBeLessThan(9000);
+    expect(window.scrollY).toBeGreaterThanOrEqual(5900);
+    expect(window.scrollY).toBeLessThan(6000);  // offset
   });
 
   it('should cleanup "popstate" and "hashchange" listeners', async () => {
