Skip to content

Commit 519e24e

Browse files
committed
Address feedback
1 parent c88c6bc commit 519e24e

File tree

4 files changed

+18
-22
lines changed

4 files changed

+18
-22
lines changed

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

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,7 @@ describe('ConnectedPositionStrategy', () => {
4141
let overlayContainerElement: HTMLElement;
4242
let strategy: ConnectedPositionStrategy;
4343
let fakeElementRef: ElementRef;
44-
// TODO: The `any` type is used here because the FakeViewportRuler can't extend or implement
45-
// the real ViewportRuler class because Typescript complains when using a class as an interface.
46-
// Also we don't want to extend the ViewportRuler because we'd need to call the constructor
47-
// in the superclass. In short: We should use Dependency Injection to fake the ViewportRuler.
48-
let fakeViewportRuler: any;
44+
let fakeViewportRuler: FakeViewportRuler;
4945
let positionBuilder: OverlayPositionBuilder;
5046

5147
let originRect: ClientRect;
@@ -592,7 +588,7 @@ function createOverflowContainerElement() {
592588

593589

594590
/** Fake implementation of ViewportRuler that just returns the previously given ClientRect. */
595-
class FakeViewportRuler {
591+
class FakeViewportRuler implements ViewportRuler {
596592
fakeRect: ClientRect = {left: 0, top: 0, width: 1014, height: 686, bottom: 686, right: 1014};
597593
fakeScrollPos: {top: number, left: number} = {top: 0, left: 0};
598594

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,8 @@ describe('ViewportRuler', () => {
4242
document.body.appendChild(veryLargeElement);
4343

4444
scrollTo(1500, 2000);
45-
ruler.updateDocumentRectangle();
45+
// Force an update of the cached viewport geometries because IE11 emits the scroll event later.
46+
ruler._cacheViewportGeometry();
4647

4748
let bounds = ruler.getViewportRect();
4849

@@ -79,7 +80,8 @@ describe('ViewportRuler', () => {
7980
document.body.appendChild(veryLargeElement);
8081

8182
scrollTo(1500, 2000);
82-
ruler.updateDocumentRectangle();
83+
// Force an update of the cached viewport geometries because IE11 emits the scroll event later.
84+
ruler._cacheViewportGeometry();
8385

8486
// In the iOS simulator (BrowserStack & SauceLabs), adding the content to the
8587
// body causes karma's iframe for the test to stretch to fit that content once we attempt to

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

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,17 +11,14 @@ import {ScrollDispatcher} from '../scroll/scroll-dispatcher';
1111
export class ViewportRuler {
1212

1313
/** Cached document client rectangle. */
14-
private _documentRect: ClientRect;
14+
private _documentRect?: ClientRect;
1515

1616
constructor(scrollDispatcher: ScrollDispatcher) {
17-
18-
// Initially update the document rectangle.
19-
this.updateDocumentRectangle();
17+
// Initially cache the document rectangle.
18+
this._cacheViewportGeometry();
2019

2120
// Subscribe to scroll and resize events and update the document rectangle on changes.
22-
scrollDispatcher
23-
.scrolled()
24-
.subscribe(() => this.updateDocumentRectangle());
21+
scrollDispatcher.scrolled().subscribe(() => this._cacheViewportGeometry());
2522
}
2623

2724
/** Gets a ClientRect for the viewport's bounds. */
@@ -68,10 +65,9 @@ export class ViewportRuler {
6865
}
6966

7067
/**
71-
* @internal
72-
* Updates the cached client rectangle of the document element.
68+
* Caches the latest client rectangle of the document element.
7369
*/
74-
updateDocumentRectangle() {
70+
_cacheViewportGeometry?() {
7571
this._documentRect = document.documentElement.getBoundingClientRect();
7672
}
7773

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -232,8 +232,9 @@ describe('MdRipple', () => {
232232
document.documentElement.scrollTop = pageScrollTop;
233233
// Mobile safari
234234
window.scrollTo(pageScrollLeft, pageScrollTop);
235-
// Force an update of the cached rectangle in the ViewportRuler.
236-
viewportRuler.updateDocumentRectangle();
235+
// Force an update of the cached viewport geometries because IE11 emits the
236+
// scroll event later.
237+
viewportRuler._cacheViewportGeometry();
237238
});
238239

239240
afterEach(() => {
@@ -245,8 +246,9 @@ describe('MdRipple', () => {
245246
document.documentElement.scrollTop = 0;
246247
// Mobile safari
247248
window.scrollTo(0, 0);
248-
// Force an update of the cached rectangle in the ViewportRuler.
249-
viewportRuler.updateDocumentRectangle();
249+
// Force an update of the cached viewport geometries because IE11 emits the
250+
// scroll event later.
251+
viewportRuler._cacheViewportGeometry();
250252
});
251253

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

0 commit comments

Comments
 (0)