Skip to content

Commit

Permalink
feat(viewport-ruler): cache document client rect (#2538)
Browse files Browse the repository at this point in the history
  • Loading branch information
devversion authored and kara committed Jan 31, 2017
1 parent 1b44880 commit d0c8f18
Show file tree
Hide file tree
Showing 6 changed files with 74 additions and 24 deletions.
18 changes: 15 additions & 3 deletions src/lib/core/overlay/position/connected-position-strategy.spec.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import {ElementRef} from '@angular/core';
import {ConnectedPositionStrategy} from './connected-position-strategy';
import {ViewportRuler} from './viewport-ruler';
import {ViewportRuler, VIEWPORT_RULER_PROVIDER} from './viewport-ruler';
import {OverlayPositionBuilder} from './overlay-position-builder';
import {ConnectedOverlayPositionChange} from './connected-position';
import {Scrollable} from '../scroll/scrollable';
import {Subscription} from 'rxjs';
import {TestBed, inject} from '@angular/core/testing';
import Spy = jasmine.Spy;
import {SCROLL_DISPATCHER_PROVIDER} from '../scroll/scroll-dispatcher';


// Default width and height of the overlay and origin panels throughout these tests.
Expand All @@ -18,6 +20,16 @@ const DEFAULT_WIDTH = 60;

describe('ConnectedPositionStrategy', () => {

let viewportRuler: ViewportRuler;

beforeEach(() => TestBed.configureTestingModule({
providers: [VIEWPORT_RULER_PROVIDER, SCROLL_DISPATCHER_PROVIDER]
}));

beforeEach(inject([ViewportRuler], (_ruler: ViewportRuler) => {
viewportRuler = _ruler;
}));

describe('with origin on document body', () => {
const ORIGIN_HEIGHT = DEFAULT_HEIGHT;
const ORIGIN_WIDTH = DEFAULT_WIDTH;
Expand Down Expand Up @@ -48,7 +60,7 @@ describe('ConnectedPositionStrategy', () => {
overlayContainerElement.appendChild(overlayElement);

fakeElementRef = new FakeElementRef(originElement);
positionBuilder = new OverlayPositionBuilder(new ViewportRuler());
positionBuilder = new OverlayPositionBuilder(viewportRuler);
});

afterEach(() => {
Expand Down Expand Up @@ -457,7 +469,7 @@ describe('ConnectedPositionStrategy', () => {
scrollable.appendChild(originElement);

// Create a strategy with knowledge of the scrollable container
let positionBuilder = new OverlayPositionBuilder(new ViewportRuler());
let positionBuilder = new OverlayPositionBuilder(viewportRuler);
let fakeElementRef = new FakeElementRef(originElement);
strategy = positionBuilder.connectedTo(
fakeElementRef,
Expand Down
25 changes: 18 additions & 7 deletions src/lib/core/overlay/position/viewport-ruler.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {ViewportRuler} from './viewport-ruler';

import {ViewportRuler, VIEWPORT_RULER_PROVIDER} from './viewport-ruler';
import {TestBed, inject} from '@angular/core/testing';
import {SCROLL_DISPATCHER_PROVIDER} from '../scroll/scroll-dispatcher';

// For all tests, we assume the browser window is 1024x786 (outerWidth x outerHeight).
// The karma config has been set to this for local tests, and it is the default size
Expand All @@ -20,10 +21,14 @@ describe('ViewportRuler', () => {
veryLargeElement.style.width = '6000px';
veryLargeElement.style.height = '6000px';

beforeEach(() => {
ruler = new ViewportRuler();
beforeEach(() => TestBed.configureTestingModule({
providers: [VIEWPORT_RULER_PROVIDER, SCROLL_DISPATCHER_PROVIDER]
}));

beforeEach(inject([ViewportRuler], (viewportRuler: ViewportRuler) => {
ruler = viewportRuler;
scrollTo(0, 0);
});
}));

it('should get the viewport bounds when the page is not scrolled', () => {
let bounds = ruler.getViewportRect();
Expand All @@ -35,7 +40,10 @@ describe('ViewportRuler', () => {

it('should get the viewport bounds when the page is scrolled', () => {
document.body.appendChild(veryLargeElement);

scrollTo(1500, 2000);
// Force an update of the cached viewport geometries because IE11 emits the scroll event later.
ruler._cacheViewportGeometry();

let bounds = ruler.getViewportRect();

Expand Down Expand Up @@ -63,14 +71,17 @@ describe('ViewportRuler', () => {
});

it('should get the scroll position when the page is not scrolled', () => {
var scrollPos = ruler.getViewportScrollPosition();
let scrollPos = ruler.getViewportScrollPosition();
expect(scrollPos.top).toBe(0);
expect(scrollPos.left).toBe(0);
});

it('should get the scroll position when the page is scrolled', () => {
document.body.appendChild(veryLargeElement);

scrollTo(1500, 2000);
// Force an update of the cached viewport geometries because IE11 emits the scroll event later.
ruler._cacheViewportGeometry();

// In the iOS simulator (BrowserStack & SauceLabs), adding the content to the
// body causes karma's iframe for the test to stretch to fit that content once we attempt to
Expand All @@ -82,7 +93,7 @@ describe('ViewportRuler', () => {
return;
}

var scrollPos = ruler.getViewportScrollPosition();
let scrollPos = ruler.getViewportScrollPosition();
expect(scrollPos.top).toBe(2000);
expect(scrollPos.left).toBe(1500);

Expand Down
33 changes: 24 additions & 9 deletions src/lib/core/overlay/position/viewport-ruler.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {Injectable, Optional, SkipSelf} from '@angular/core';
import {ScrollDispatcher} from '../scroll/scroll-dispatcher';


/**
Expand All @@ -7,12 +8,20 @@ import {Injectable, Optional, SkipSelf} from '@angular/core';
*/
@Injectable()
export class ViewportRuler {
// TODO(jelbourn): cache the document's bounding rect and only update it when the window
// is resized (debounced).

/** Cached document client rectangle. */
private _documentRect?: ClientRect;

constructor(scrollDispatcher: ScrollDispatcher) {
// Initially cache the document rectangle.
this._cacheViewportGeometry();

// Subscribe to scroll and resize events and update the document rectangle on changes.
scrollDispatcher.scrolled().subscribe(() => this._cacheViewportGeometry());
}

/** Gets a ClientRect for the viewport's bounds. */
getViewportRect(): ClientRect {
getViewportRect(documentRect = this._documentRect): ClientRect {
// Use the document element's bounding rect rather than the window scroll properties
// (e.g. pageYOffset, scrollY) due to in issue in Chrome and IE where window scroll
// properties and client coordinates (boundingClientRect, clientX/Y, etc.) are in different
Expand All @@ -22,7 +31,6 @@ export class ViewportRuler {
// We use the documentElement instead of the body because, by default (without a css reset)
// browsers typically give the document body an 8px margin, which is not included in
// getBoundingClientRect().
const documentRect = document.documentElement.getBoundingClientRect();
const scrollPosition = this.getViewportScrollPosition(documentRect);
const height = window.innerHeight;
const width = window.innerWidth;
Expand All @@ -42,7 +50,7 @@ export class ViewportRuler {
* Gets the (top, left) scroll position of the viewport.
* @param documentRect
*/
getViewportScrollPosition(documentRect = document.documentElement.getBoundingClientRect()) {
getViewportScrollPosition(documentRect = this._documentRect) {
// The top-left-corner of the viewport is determined by the scroll position of the document
// body, normally just (scrollLeft, scrollTop). However, Chrome and Firefox disagree about
// whether `document.body` or `document.documentElement` is the scrolled element, so reading
Expand All @@ -54,15 +62,22 @@ export class ViewportRuler {

return {top, left};
}

/** Caches the latest client rectangle of the document element. */
_cacheViewportGeometry?() {
this._documentRect = document.documentElement.getBoundingClientRect();
}

}

export function VIEWPORT_RULER_PROVIDER_FACTORY(parentDispatcher: ViewportRuler) {
return parentDispatcher || new ViewportRuler();
};
export function VIEWPORT_RULER_PROVIDER_FACTORY(parentRuler: ViewportRuler,
scrollDispatcher: ScrollDispatcher) {
return parentRuler || new ViewportRuler(scrollDispatcher);
}

export const VIEWPORT_RULER_PROVIDER = {
// If there is already a ViewportRuler available, use that. Otherwise, provide a new one.
provide: ViewportRuler,
deps: [[new Optional(), new SkipSelf(), ViewportRuler]],
deps: [[new Optional(), new SkipSelf(), ViewportRuler], ScrollDispatcher],
useFactory: VIEWPORT_RULER_PROVIDER_FACTORY
};
16 changes: 13 additions & 3 deletions src/lib/core/ripple/ripple.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {TestBed, ComponentFixture, fakeAsync, tick} from '@angular/core/testing';
import {TestBed, ComponentFixture, fakeAsync, tick, inject} from '@angular/core/testing';
import {Component, ViewChild} from '@angular/core';
import {MdRipple, MdRippleModule} from './ripple';
import {ViewportRuler} from '../overlay/position/viewport-ruler';


/** Creates a DOM event to indicate that a CSS transition for the given property ended. */
Expand Down Expand Up @@ -60,6 +61,7 @@ describe('MdRipple', () => {
let rippleElement: HTMLElement;
let rippleBackground: Element;
let originalBodyMargin: string;
let viewportRuler: ViewportRuler;

beforeEach(() => {
TestBed.configureTestingModule({
Expand All @@ -72,11 +74,13 @@ describe('MdRipple', () => {
});
});

beforeEach(() => {
beforeEach(inject([ViewportRuler], (ruler: ViewportRuler) => {
viewportRuler = ruler;

// Set body margin to 0 during tests so it doesn't mess up position calculations.
originalBodyMargin = document.body.style.margin;
document.body.style.margin = '0';
});
}));

afterEach(() => {
document.body.style.margin = originalBodyMargin;
Expand Down Expand Up @@ -228,6 +232,9 @@ describe('MdRipple', () => {
document.documentElement.scrollTop = pageScrollTop;
// Mobile safari
window.scrollTo(pageScrollLeft, pageScrollTop);
// Force an update of the cached viewport geometries because IE11 emits the
// scroll event later.
viewportRuler._cacheViewportGeometry();
});

afterEach(() => {
Expand All @@ -239,6 +246,9 @@ describe('MdRipple', () => {
document.documentElement.scrollTop = 0;
// Mobile safari
window.scrollTo(0, 0);
// Force an update of the cached viewport geometries because IE11 emits the
// scroll event later.
viewportRuler._cacheViewportGeometry();
});

it('create ripple with correct position', () => {
Expand Down
3 changes: 2 additions & 1 deletion src/lib/core/ripple/ripple.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
} from './ripple-renderer';
import {CompatibilityModule} from '../compatibility/compatibility';
import {ViewportRuler, VIEWPORT_RULER_PROVIDER} from '../overlay/position/viewport-ruler';
import {SCROLL_DISPATCHER_PROVIDER} from '../overlay/scroll/scroll-dispatcher';


@Directive({
Expand Down Expand Up @@ -238,7 +239,7 @@ export class MdRipple implements OnInit, OnDestroy, OnChanges {
imports: [CompatibilityModule],
exports: [MdRipple, CompatibilityModule],
declarations: [MdRipple],
providers: [VIEWPORT_RULER_PROVIDER],
providers: [VIEWPORT_RULER_PROVIDER, SCROLL_DISPATCHER_PROVIDER],
})
export class MdRippleModule {
/** @deprecated */
Expand Down
3 changes: 2 additions & 1 deletion src/lib/tabs/tab-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {MdTab} from './tab';
import {MdTabBody} from './tab-body';
import {VIEWPORT_RULER_PROVIDER} from '../core/overlay/position/viewport-ruler';
import {MdTabHeader} from './tab-header';
import {SCROLL_DISPATCHER_PROVIDER} from '../core/overlay/scroll/scroll-dispatcher';


/** Used to generate unique ID's for each tab component */
Expand Down Expand Up @@ -214,7 +215,7 @@ export class MdTabGroup {
exports: [MdTabGroup, MdTabLabel, MdTab, MdTabNavBar, MdTabLink, MdTabLinkRipple],
declarations: [MdTabGroup, MdTabLabel, MdTab, MdInkBar, MdTabLabelWrapper,
MdTabNavBar, MdTabLink, MdTabBody, MdTabLinkRipple, MdTabHeader],
providers: [VIEWPORT_RULER_PROVIDER],
providers: [VIEWPORT_RULER_PROVIDER, SCROLL_DISPATCHER_PROVIDER],
})
export class MdTabsModule {
/** @deprecated */
Expand Down

0 comments on commit d0c8f18

Please sign in to comment.