diff --git a/aio/src/app/shared/scroll.service.spec.ts b/aio/src/app/shared/scroll.service.spec.ts index f419bd8fb9..643136aa8a 100644 --- a/aio/src/app/shared/scroll.service.spec.ts +++ b/aio/src/app/shared/scroll.service.spec.ts @@ -5,6 +5,7 @@ import { DOCUMENT } from '@angular/platform-browser'; import { ScrollService, topMargin } from './scroll.service'; describe('ScrollService', () => { + const topOfPageElem = {} as Element; let injector: ReflectiveInjector; let document: MockDocument; let location: MockPlatformLocation; @@ -16,7 +17,8 @@ describe('ScrollService', () => { class MockDocument { body = new MockElement(); - getElementById = jasmine.createSpy('Document getElementById'); + getElementById = jasmine.createSpy('Document getElementById').and.returnValue(topOfPageElem); + querySelector = jasmine.createSpy('Document querySelector'); } class MockElement { @@ -38,6 +40,69 @@ describe('ScrollService', () => { scrollService = injector.get(ScrollService); }); + describe('#topOffset', () => { + it('should query for the top-bar by CSS selector', () => { + expect(document.querySelector).not.toHaveBeenCalled(); + + expect(scrollService.topOffset).toBe(topMargin); + expect(document.querySelector).toHaveBeenCalled(); + }); + + it('should be calculated based on the top-bar\'s height + margin', () => { + (document.querySelector as jasmine.Spy).and.returnValue({clientHeight: 50}); + expect(scrollService.topOffset).toBe(50 + topMargin); + }); + + it('should only query for the top-bar once', () => { + expect(scrollService.topOffset).toBe(topMargin); + (document.querySelector as jasmine.Spy).calls.reset(); + + expect(scrollService.topOffset).toBe(topMargin); + expect(document.querySelector).not.toHaveBeenCalled(); + }); + + it('should retrieve the top-bar\'s height again after resize', () => { + let clientHeight = 50; + (document.querySelector as jasmine.Spy).and.callFake(() => ({clientHeight})); + + expect(scrollService.topOffset).toBe(50 + topMargin); + expect(document.querySelector).toHaveBeenCalled(); + + (document.querySelector as jasmine.Spy).calls.reset(); + clientHeight = 100; + + expect(scrollService.topOffset).toBe(50 + topMargin); + expect(document.querySelector).not.toHaveBeenCalled(); + + window.dispatchEvent(new Event('resize')); + + expect(scrollService.topOffset).toBe(100 + topMargin); + expect(document.querySelector).toHaveBeenCalled(); + }); + }); + + describe('#topOfPageElement', () => { + it('should query for the top-of-page element by ID', () => { + expect(document.getElementById).not.toHaveBeenCalled(); + + expect(scrollService.topOfPageElement).toBe(topOfPageElem); + expect(document.getElementById).toHaveBeenCalled(); + }); + + it('should only query for the top-of-page element once', () => { + expect(scrollService.topOfPageElement).toBe(topOfPageElem); + (document.getElementById as jasmine.Spy).calls.reset(); + + expect(scrollService.topOfPageElement).toBe(topOfPageElem); + expect(document.getElementById).not.toHaveBeenCalled(); + }); + + it('should return `` if unable to find the top-of-page element', () => { + (document.getElementById as jasmine.Spy).and.returnValue(null); + expect(scrollService.topOfPageElement).toBe(document.body as any); + }); + }); + describe('#scroll', () => { it('should scroll to the top if there is no hash', () => { location.hash = ''; @@ -73,10 +138,28 @@ describe('ScrollService', () => { describe('#scrollToElement', () => { it('should scroll to element', () => { - const element = new MockElement(); + const element: Element = new MockElement() as any; scrollService.scrollToElement(element); expect(element.scrollIntoView).toHaveBeenCalled(); - expect(window.scrollBy).toHaveBeenCalledWith(0, -topMargin); + expect(window.scrollBy).toHaveBeenCalledWith(0, -scrollService.topOffset); + }); + + it('should scroll all the way to the top if close enough', () => { + const element: Element = new MockElement() as any; + + (window as any).pageYOffset = 25; + scrollService.scrollToElement(element); + + expect(element.scrollIntoView).toHaveBeenCalled(); + expect(window.scrollBy).toHaveBeenCalledWith(0, -scrollService.topOffset); + (window.scrollBy as jasmine.Spy).calls.reset(); + + (window as any).pageYOffset = 15; + scrollService.scrollToElement(element); + + expect(element.scrollIntoView).toHaveBeenCalled(); + expect(window.scrollBy).toHaveBeenCalledWith(0, -scrollService.topOffset); + expect(window.scrollBy).toHaveBeenCalledWith(0, -15); }); it('should do nothing if no element', () => { diff --git a/aio/src/app/shared/scroll.service.ts b/aio/src/app/shared/scroll.service.ts index d33478ca7e..0d51b7c3d4 100644 --- a/aio/src/app/shared/scroll.service.ts +++ b/aio/src/app/shared/scroll.service.ts @@ -1,6 +1,7 @@ import { Injectable, Inject } from '@angular/core'; import { PlatformLocation } from '@angular/common'; import { DOCUMENT } from '@angular/platform-browser'; +import {fromEvent} from 'rxjs/observable/fromEvent'; export const topMargin = 16; /** @@ -9,20 +10,20 @@ export const topMargin = 16; @Injectable() export class ScrollService { - private _topOffset: number; + private _topOffset: number | null; private _topOfPageElement: Element; // Offset from the top of the document to bottom of any static elements // at the top (e.g. toolbar) + some margin get topOffset() { if (!this._topOffset) { - // Since the toolbar is not static, we don't need to account for its height. - this._topOffset = topMargin; + const toolbar = this.document.querySelector('md-toolbar.app-toolbar'); + this._topOffset = (toolbar && toolbar.clientHeight || 0) + topMargin; } return this._topOffset; } - private get topOfPageElement() { + get topOfPageElement() { if (!this._topOfPageElement) { this._topOfPageElement = this.document.getElementById('top-of-page') || this.document.body; } @@ -31,7 +32,10 @@ export class ScrollService { constructor( @Inject(DOCUMENT) private document: any, - private location: PlatformLocation) { } + private location: PlatformLocation) { + // On resize, the toolbar might change height, so "invalidate" the top offset. + fromEvent(window, 'resize').subscribe(() => this._topOffset = null); + } /** * Scroll to the element with id extracted from the current location hash fragment. @@ -53,7 +57,14 @@ export class ScrollService { scrollToElement(element: Element) { if (element) { element.scrollIntoView(); - if (window && window.scrollBy) { window.scrollBy(0, -this.topOffset); } + if (window && window.scrollBy) { + window.scrollBy(0, -this.topOffset); + 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); + } + } } } diff --git a/aio/src/styles/0-base/_typography.scss b/aio/src/styles/0-base/_typography.scss index edc017a6de..0d15597b5a 100755 --- a/aio/src/styles/0-base/_typography.scss +++ b/aio/src/styles/0-base/_typography.scss @@ -8,7 +8,7 @@ body { } h1 { - display:inline-block; + display: inline-block; font-size: 24px; font-weight: 500; margin: 8px 0px; @@ -158,4 +158,4 @@ code { &:hover { color: $mediumgray; } -} \ No newline at end of file +} diff --git a/aio/src/styles/1-layouts/_content-layout.scss b/aio/src/styles/1-layouts/_content-layout.scss index 3522cdcf33..944d531b98 100644 --- a/aio/src/styles/1-layouts/_content-layout.scss +++ b/aio/src/styles/1-layouts/_content-layout.scss @@ -7,7 +7,7 @@ aio-shell.page-docs { .sidenav-content { min-height: 100vh; - padding: 6rem 3rem 1rem; + padding: 80px 3rem 1rem; } @media (max-width: 600px) {