refactor(docs-infra): minor clean-up of ScrollService (#29658)

PR Close #29658

aio/src/app/shared/scroll.service.spec.ts

@@ -51,7 +51,7 @@ describe('ScrollService', () => {
     spyOn(window, 'scrollBy');
   });
 
-  it('should debounce `updateScrollPositonInHistory()` after 500ms', fakeAsync(() => {
+  it('should debounce `updateScrollPositonInHistory()`', fakeAsync(() => {
     const updateScrollPositionInHistorySpy = spyOn(scrollService, 'updateScrollPositionInHistory');
 
     window.dispatchEvent(new Event('scroll'));

aio/src/app/shared/scroll.service.ts

@@ -1,9 +1,14 @@
+import { DOCUMENT, Location, PlatformLocation, PopStateEvent, ViewportScroller } from '@angular/common';
 import { Injectable, Inject } from '@angular/core';
-import { Location, PlatformLocation, ViewportScroller } from '@angular/common';
-import { DOCUMENT } from '@angular/common';
 import { fromEvent } from 'rxjs';
 import { debounceTime } from 'rxjs/operators';
 
+type ScrollPosition = [number, number];
+interface ScrollPositionPopStateEvent extends PopStateEvent {
+  // If there is history state, it should always include `scrollPosition`.
+  state?: {scrollPosition: ScrollPosition};
+}
+
 export const topMargin = 16;
 /**
  * A service that scrolls document elements into view
@@ -14,12 +19,15 @@ export class ScrollService {
   private _topOffset: number | null;
   private _topOfPageElement: Element;
 
-  // true when popState event has been fired.
+  // Whether a `popstate` event has been fired (but the associated scroll position is not yet
+  // restored).
   popStateFired = false;
-  // scroll position which has to be restored after the popState event
+  // The scroll position which has to be restored, after a `popstate` event.
-  scrollPosition: [number, number] = [0, 0];
+  scrollPosition: ScrollPosition = [0, 0];
-  // true when the browser supports `scrollTo`, `scrollX`, `scrollY` and `scrollRestoration`
+  // Whether the browser supports the necessary features for manual scroll restoration.
-  supportManualScrollRestoration: boolean;
+  supportManualScrollRestoration: boolean =
+      !!window && ('scrollTo' in window) && ('scrollX' in window) && ('scrollY' in window) &&
+      !!history && ('scrollRestoration' in history);
 
   // Offset from the top of the document to bottom of any static elements
   // at the top (e.g. toolbar) + some margin
@@ -49,26 +57,22 @@ export class ScrollService {
     fromEvent(window, 'scroll')
       .pipe(debounceTime(250)).subscribe(() => this.updateScrollPositionInHistory());
 
-    this.supportManualScrollRestoration = !!window && 'scrollTo' in window && 'scrollX' in window
-      && 'scrollY' in window && !!history && 'scrollRestoration' in history;
-
     // Change scroll restoration strategy to `manual` if it's supported
     if (this.supportManualScrollRestoration) {
       history.scrollRestoration = 'manual';
       // we have to detect forward and back navigation thanks to popState event
-      this.location.subscribe(event => {
+      this.location.subscribe((event: ScrollPositionPopStateEvent) => {
         // the type is `hashchange` when the fragment identifier of the URL has changed. It allows us to go to position
         // just before a click on an anchor
         if (event.type === 'hashchange') {
           this.scrollToPosition();
         } else {
-          // Navigating with forward/back, we have to remove the position from the session storage in order to avoid a
+          // Navigating with the forward/back button, we have to remove the position from the
-          // race-condition
+          // session storage in order to avoid a race-condition.
           this.removeStoredScrollPosition();
-          // The popstate event is always triggered by doing a browser action such as a click on the back or forward button.
+          // The `popstate` event is always triggered by a browser action such as clicking the
-          // It can be follow by a event of type `hashchange`.
+          // forward/back button. It can be followed by a `hashchange` event.
           this.popStateFired = true;
-          // we always should have a scrollPosition in our state history
           this.scrollPosition = event.state ? event.state['scrollPosition'] : null;
         }
       });
@@ -107,16 +111,16 @@ export class ScrollService {
       this.viewportScroller.scrollToPosition(storedScrollPosition);
     } else {
       if (this.needToFixScrollPosition()) {
-        // The document was reloaded following a popState `event` (called by the forward/back button), so we manage
+        // The document was reloaded following a `popstate` event (triggered by clicking the
-        // the scroll position
+        // forward/back button), so we manage the scroll position.
         this.scrollToPosition();
       } else {
-        // The document was loaded either of the following cases: a direct navigation via typing the URL in the
+        // The document was loaded as a result of one of the following cases:
-        // address bar or a click on a link. If the location contains a hash, we have to wait for async
+        // - Typing the URL in the address bar (direct navigation).
-        // layout.
+        // - Clicking on a link.
+        // (If the location contains a hash, we have to wait for async layout.)
         if (this.isLocationWithHash()) {
-          // Scroll 500ms after the new document has been inserted into the doc-viewer.
+          // Delay scrolling by the specified amount to allow time for async layout to complete.
-          // The delay is to allow time for async layout to complete.
           setTimeout(() => this.scroll(), delay);
         } else {
           // If the location doesn't contain a hash, we scroll to the top of the page.
@@ -171,7 +175,7 @@ export class ScrollService {
     }
   }
 
-  getStoredScrollPosition(): [number, number] | null {
+  getStoredScrollPosition(): ScrollPosition | null {
     const position = window.sessionStorage.getItem('scrollPosition');
     return position ? JSON.parse('[' + position + ']') : null;
   }