fix(aio): fix scrolling to elements near the bottom of the page

Previously, we always assumed that elements would be scrolled to the top of the
page, when calling `element.scrollIntoView()`. This is not true for elements
that cannot be scrolled to the top, e.g. when the viewport height is larger than
the height of the content after the element (common for small sections near the
end of the page).
In such cases, we would unnecessarily scroll up to account for the static
toolbar, which was unnecessary (since the element was not behind the toolbar
anyway) and caused ScrollSpy to fail to identify the scrolled-to section as
active.

This commit fixes it by ensuring that we do not scroll more than necessary in
order to align the top of the element with the bottom of the toolbar.

Fixes #17452

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

@@ -22,6 +22,8 @@ describe('ScrollService', () => {
   }
 
   class MockElement {
+    getBoundingClientRect = jasmine.createSpy('Element getBoundingClientRect')
+                                   .and.returnValue({top: 0});
     scrollIntoView = jasmine.createSpy('Element scrollIntoView');
   }
 
@@ -144,6 +146,22 @@ describe('ScrollService', () => {
       expect(window.scrollBy).toHaveBeenCalledWith(0, -scrollService.topOffset);
     });
 
+    it('should not scroll more than necessary (e.g. for elements close to the bottom)', () => {
+      const element: Element = new MockElement() as any;
+      const getBoundingClientRect = element.getBoundingClientRect as jasmine.Spy;
+      const topOffset = scrollService.topOffset;
+
+      getBoundingClientRect.and.returnValue({top: topOffset + 100});
+      scrollService.scrollToElement(element);
+      expect(element.scrollIntoView).toHaveBeenCalledTimes(1);
+      expect(window.scrollBy).toHaveBeenCalledWith(0, 100);
+
+      getBoundingClientRect.and.returnValue({top: topOffset - 10});
+      scrollService.scrollToElement(element);
+      expect(element.scrollIntoView).toHaveBeenCalledTimes(2);
+      expect(window.scrollBy).toHaveBeenCalledWith(0, -10);
+    });
+
     it('should scroll all the way to the top if close enough', () => {
       const element: Element = new MockElement() as any;
 

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

@@ -57,11 +57,17 @@ export class ScrollService {
   scrollToElement(element: Element) {
     if (element) {
       element.scrollIntoView();
+
       if (window && window.scrollBy) {
-        window.scrollBy(0, -this.topOffset);
+        // Scroll as much as necessary to align the top of `element` at `topOffset`.
+        // (Usually, `.top` will be 0, except for cases where the element cannot be scrolled all the
+        //  way to the top, because the viewport is larger than the height of the content after the
+        //  element.)
+        window.scrollBy(0, element.getBoundingClientRect().top - this.topOffset);
+
+        // If we are very close to the top (<20px), then scroll all the way up.
+        // (This can happen if `element` is at the top of the page, but has a small top-margin.)
         if (window.pageYOffset < 20) {
-          // If we are very close to the top (<20px), then scroll all the way up.
-          // (This can happen if `element` is at the top of the page, but has a small top-margin.)
           window.scrollBy(0, -window.pageYOffset);
         }
       }