Skip to content

feat(viewport-ruler): cache document client rect #2538

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 31, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 {DefaultStyleCompatibilityModeModule} from '../compatibility/default-mode';
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: [DefaultStyleCompatibilityModeModule],
exports: [MdRipple, DefaultStyleCompatibilityModeModule],
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 @@ -204,7 +205,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