fix(aio): fix scrolling to an element
Previously, the top-bar's height wasn't taken into account when scrolling an element into view. As a result, the element would be hidden behind the top-bar. Taking the top-bar height into account was not necessary before #17155, because the top-bar was not fixed (i.e. it scrolled away). This commit fixes the scrolling behavior by accounting for the top-bar's height when scrolling an element into view. (This partially reverts #17102.) Fixes #17219 Fixes #17226
This commit is contained in:
parent
4759975be6
commit
d837bfc2d7
|
@ -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 `<body>` 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 = <Element><any> 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', () => {
|
||||
|
|
|
@ -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);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -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;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -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) {
|
||||
|
|
Loading…
Reference in New Issue