Skip to content

Commit d0c8f18

Browse files
devversionkara
authored andcommitted
feat(viewport-ruler): cache document client rect (#2538)
1 parent 1b44880 commit d0c8f18

File tree

6 files changed

+74
-24
lines changed

6 files changed

+74
-24
lines changed

src/lib/core/overlay/position/connected-position-strategy.spec.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
import {ElementRef} from '@angular/core';
22
import {ConnectedPositionStrategy} from './connected-position-strategy';
3-
import {ViewportRuler} from './viewport-ruler';
3+
import {ViewportRuler, VIEWPORT_RULER_PROVIDER} from './viewport-ruler';
44
import {OverlayPositionBuilder} from './overlay-position-builder';
55
import {ConnectedOverlayPositionChange} from './connected-position';
66
import {Scrollable} from '../scroll/scrollable';
77
import {Subscription} from 'rxjs';
8+
import {TestBed, inject} from '@angular/core/testing';
89
import Spy = jasmine.Spy;
10+
import {SCROLL_DISPATCHER_PROVIDER} from '../scroll/scroll-dispatcher';
911

1012

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

1921
describe('ConnectedPositionStrategy', () => {
2022

23+
let viewportRuler: ViewportRuler;
24+
25+
beforeEach(() => TestBed.configureTestingModule({
26+
providers: [VIEWPORT_RULER_PROVIDER, SCROLL_DISPATCHER_PROVIDER]
27+
}));
28+
29+
beforeEach(inject([ViewportRuler], (_ruler: ViewportRuler) => {
30+
viewportRuler = _ruler;
31+
}));
32+
2133
describe('with origin on document body', () => {
2234
const ORIGIN_HEIGHT = DEFAULT_HEIGHT;
2335
const ORIGIN_WIDTH = DEFAULT_WIDTH;
@@ -48,7 +60,7 @@ describe('ConnectedPositionStrategy', () => {
4860
overlayContainerElement.appendChild(overlayElement);
4961

5062
fakeElementRef = new FakeElementRef(originElement);
51-
positionBuilder = new OverlayPositionBuilder(new ViewportRuler());
63+
positionBuilder = new OverlayPositionBuilder(viewportRuler);
5264
});
5365

5466
afterEach(() => {
@@ -457,7 +469,7 @@ describe('ConnectedPositionStrategy', () => {
457469
scrollable.appendChild(originElement);
458470

459471
// Create a strategy with knowledge of the scrollable container
460-
let positionBuilder = new OverlayPositionBuilder(new ViewportRuler());
472+
let positionBuilder = new OverlayPositionBuilder(viewportRuler);
461473
let fakeElementRef = new FakeElementRef(originElement);
462474
strategy = positionBuilder.connectedTo(
463475
fakeElementRef,

src/lib/core/overlay/position/viewport-ruler.spec.ts

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
import {ViewportRuler} from './viewport-ruler';
2-
1+
import {ViewportRuler, VIEWPORT_RULER_PROVIDER} from './viewport-ruler';
2+
import {TestBed, inject} from '@angular/core/testing';
3+
import {SCROLL_DISPATCHER_PROVIDER} from '../scroll/scroll-dispatcher';
34

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

23-
beforeEach(() => {
24-
ruler = new ViewportRuler();
24+
beforeEach(() => TestBed.configureTestingModule({
25+
providers: [VIEWPORT_RULER_PROVIDER, SCROLL_DISPATCHER_PROVIDER]
26+
}));
27+
28+
beforeEach(inject([ViewportRuler], (viewportRuler: ViewportRuler) => {
29+
ruler = viewportRuler;
2530
scrollTo(0, 0);
26-
});
31+
}));
2732

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

3641
it('should get the viewport bounds when the page is scrolled', () => {
3742
document.body.appendChild(veryLargeElement);
43+
3844
scrollTo(1500, 2000);
45+
// Force an update of the cached viewport geometries because IE11 emits the scroll event later.
46+
ruler._cacheViewportGeometry();
3947

4048
let bounds = ruler.getViewportRect();
4149

@@ -63,14 +71,17 @@ describe('ViewportRuler', () => {
6371
});
6472

6573
it('should get the scroll position when the page is not scrolled', () => {
66-
var scrollPos = ruler.getViewportScrollPosition();
74+
let scrollPos = ruler.getViewportScrollPosition();
6775
expect(scrollPos.top).toBe(0);
6876
expect(scrollPos.left).toBe(0);
6977
});
7078

7179
it('should get the scroll position when the page is scrolled', () => {
7280
document.body.appendChild(veryLargeElement);
81+
7382
scrollTo(1500, 2000);
83+
// Force an update of the cached viewport geometries because IE11 emits the scroll event later.
84+
ruler._cacheViewportGeometry();
7485

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

85-
var scrollPos = ruler.getViewportScrollPosition();
96+
let scrollPos = ruler.getViewportScrollPosition();
8697
expect(scrollPos.top).toBe(2000);
8798
expect(scrollPos.left).toBe(1500);
8899

src/lib/core/overlay/position/viewport-ruler.ts

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import {Injectable, Optional, SkipSelf} from '@angular/core';
2+
import {ScrollDispatcher} from '../scroll/scroll-dispatcher';
23

34

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

12+
/** Cached document client rectangle. */
13+
private _documentRect?: ClientRect;
14+
15+
constructor(scrollDispatcher: ScrollDispatcher) {
16+
// Initially cache the document rectangle.
17+
this._cacheViewportGeometry();
18+
19+
// Subscribe to scroll and resize events and update the document rectangle on changes.
20+
scrollDispatcher.scrolled().subscribe(() => this._cacheViewportGeometry());
21+
}
1322

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

5563
return {top, left};
5664
}
65+
66+
/** Caches the latest client rectangle of the document element. */
67+
_cacheViewportGeometry?() {
68+
this._documentRect = document.documentElement.getBoundingClientRect();
69+
}
70+
5771
}
5872

59-
export function VIEWPORT_RULER_PROVIDER_FACTORY(parentDispatcher: ViewportRuler) {
60-
return parentDispatcher || new ViewportRuler();
61-
};
73+
export function VIEWPORT_RULER_PROVIDER_FACTORY(parentRuler: ViewportRuler,
74+
scrollDispatcher: ScrollDispatcher) {
75+
return parentRuler || new ViewportRuler(scrollDispatcher);
76+
}
6277

6378
export const VIEWPORT_RULER_PROVIDER = {
6479
// If there is already a ViewportRuler available, use that. Otherwise, provide a new one.
6580
provide: ViewportRuler,
66-
deps: [[new Optional(), new SkipSelf(), ViewportRuler]],
81+
deps: [[new Optional(), new SkipSelf(), ViewportRuler], ScrollDispatcher],
6782
useFactory: VIEWPORT_RULER_PROVIDER_FACTORY
6883
};

src/lib/core/ripple/ripple.spec.ts

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1-
import {TestBed, ComponentFixture, fakeAsync, tick} from '@angular/core/testing';
1+
import {TestBed, ComponentFixture, fakeAsync, tick, inject} from '@angular/core/testing';
22
import {Component, ViewChild} from '@angular/core';
33
import {MdRipple, MdRippleModule} from './ripple';
4+
import {ViewportRuler} from '../overlay/position/viewport-ruler';
45

56

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

6466
beforeEach(() => {
6567
TestBed.configureTestingModule({
@@ -72,11 +74,13 @@ describe('MdRipple', () => {
7274
});
7375
});
7476

75-
beforeEach(() => {
77+
beforeEach(inject([ViewportRuler], (ruler: ViewportRuler) => {
78+
viewportRuler = ruler;
79+
7680
// Set body margin to 0 during tests so it doesn't mess up position calculations.
7781
originalBodyMargin = document.body.style.margin;
7882
document.body.style.margin = '0';
79-
});
83+
}));
8084

8185
afterEach(() => {
8286
document.body.style.margin = originalBodyMargin;
@@ -228,6 +232,9 @@ describe('MdRipple', () => {
228232
document.documentElement.scrollTop = pageScrollTop;
229233
// Mobile safari
230234
window.scrollTo(pageScrollLeft, pageScrollTop);
235+
// Force an update of the cached viewport geometries because IE11 emits the
236+
// scroll event later.
237+
viewportRuler._cacheViewportGeometry();
231238
});
232239

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

244254
it('create ripple with correct position', () => {

src/lib/core/ripple/ripple.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
} from './ripple-renderer';
1919
import {CompatibilityModule} from '../compatibility/compatibility';
2020
import {ViewportRuler, VIEWPORT_RULER_PROVIDER} from '../overlay/position/viewport-ruler';
21+
import {SCROLL_DISPATCHER_PROVIDER} from '../overlay/scroll/scroll-dispatcher';
2122

2223

2324
@Directive({
@@ -238,7 +239,7 @@ export class MdRipple implements OnInit, OnDestroy, OnChanges {
238239
imports: [CompatibilityModule],
239240
exports: [MdRipple, CompatibilityModule],
240241
declarations: [MdRipple],
241-
providers: [VIEWPORT_RULER_PROVIDER],
242+
providers: [VIEWPORT_RULER_PROVIDER, SCROLL_DISPATCHER_PROVIDER],
242243
})
243244
export class MdRippleModule {
244245
/** @deprecated */

src/lib/tabs/tab-group.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import {MdTab} from './tab';
2828
import {MdTabBody} from './tab-body';
2929
import {VIEWPORT_RULER_PROVIDER} from '../core/overlay/position/viewport-ruler';
3030
import {MdTabHeader} from './tab-header';
31+
import {SCROLL_DISPATCHER_PROVIDER} from '../core/overlay/scroll/scroll-dispatcher';
3132

3233

3334
/** Used to generate unique ID's for each tab component */
@@ -214,7 +215,7 @@ export class MdTabGroup {
214215
exports: [MdTabGroup, MdTabLabel, MdTab, MdTabNavBar, MdTabLink, MdTabLinkRipple],
215216
declarations: [MdTabGroup, MdTabLabel, MdTab, MdInkBar, MdTabLabelWrapper,
216217
MdTabNavBar, MdTabLink, MdTabBody, MdTabLinkRipple, MdTabHeader],
217-
providers: [VIEWPORT_RULER_PROVIDER],
218+
providers: [VIEWPORT_RULER_PROVIDER, SCROLL_DISPATCHER_PROVIDER],
218219
})
219220
export class MdTabsModule {
220221
/** @deprecated */

0 commit comments

Comments
 (0)