Skip to content

Commit dd508ea

Browse files
tinayuangaommalerba
tinayuangao
authored and
mmalerba
committed
fix(ripple): Fix the ripple position (#1907)
* Add test. Use viewport ruler * fix lint * Add ViewportRuler to providers * . * . * .
1 parent 4c49f5f commit dd508ea

File tree

14 files changed

+121
-15
lines changed

14 files changed

+121
-15
lines changed

‎src/lib/button/button.spec.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ import {async, TestBed, ComponentFixture} from '@angular/core/testing';
22
import {Component} from '@angular/core';
33
import {By} from '@angular/platform-browser';
44
import {MdButtonModule} from './button';
5+
import {ViewportRuler} from '../core/overlay/position/viewport-ruler';
6+
import {FakeViewportRuler} from '../core/overlay/position/fake-viewport-ruler';
57

68

79
describe('MdButton', () => {
@@ -10,6 +12,9 @@ describe('MdButton', () => {
1012
TestBed.configureTestingModule({
1113
imports: [MdButtonModule.forRoot()],
1214
declarations: [TestApp],
15+
providers: [
16+
{provide: ViewportRuler, useClass: FakeViewportRuler},
17+
]
1318
});
1419

1520
TestBed.compileComponents();

‎src/lib/button/button.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
} from '@angular/core';
1212
import {CommonModule} from '@angular/common';
1313
import {MdRippleModule, coerceBooleanProperty, DefaultStyleCompatibilityModeModule} from '../core';
14+
import {ViewportRuler} from '../core/overlay/position/viewport-ruler';
1415

1516

1617
// TODO(jelbourn): Make the `isMouseDown` stuff done with one global listener.
@@ -166,7 +167,7 @@ export class MdButtonModule {
166167
static forRoot(): ModuleWithProviders {
167168
return {
168169
ngModule: MdButtonModule,
169-
providers: []
170+
providers: [ViewportRuler]
170171
};
171172
}
172173
}

‎src/lib/checkbox/checkbox.spec.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ import {
1414
import {Component, DebugElement} from '@angular/core';
1515
import {By} from '@angular/platform-browser';
1616
import {MdCheckbox, MdCheckboxChange, MdCheckboxModule} from './checkbox';
17+
import {ViewportRuler} from '../core/overlay/position/viewport-ruler';
18+
import {FakeViewportRuler} from '../core/overlay/position/fake-viewport-ruler';
1719

1820

1921
// TODO: Implement E2E tests for spacebar/click behavior for checking/unchecking
@@ -35,6 +37,9 @@ describe('MdCheckbox', () => {
3537
CheckboxWithChangeEvent,
3638
CheckboxWithFormControl,
3739
],
40+
providers: [
41+
{provide: ViewportRuler, useClass: FakeViewportRuler},
42+
]
3843
});
3944

4045
TestBed.compileComponents();

‎src/lib/checkbox/checkbox.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {CommonModule} from '@angular/common';
1717
import {NG_VALUE_ACCESSOR, ControlValueAccessor} from '@angular/forms';
1818
import {coerceBooleanProperty} from '../core/coersion/boolean-property';
1919
import {MdRippleModule, DefaultStyleCompatibilityModeModule} from '../core';
20+
import {ViewportRuler} from '../core/overlay/position/viewport-ruler';
2021

2122

2223
/**
@@ -391,7 +392,7 @@ export class MdCheckboxModule {
391392
static forRoot(): ModuleWithProviders {
392393
return {
393394
ngModule: MdCheckboxModule,
394-
providers: []
395+
providers: [ViewportRuler]
395396
};
396397
}
397398
}

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
export class FakeViewportRuler {
2+
getViewportRect() {
3+
return {
4+
left: 0, top: 0, width: 1014, height: 686, bottom: 686, right: 1014
5+
};
6+
}
7+
8+
getViewportScrollPosition() {
9+
return {top: 0, left: 0};
10+
}
11+
}

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

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,8 @@ export class ViewportRuler {
5050
// `scrollTop` and `scrollLeft` is inconsistent. However, using the bounding rect of
5151
// `document.documentElement` works consistently, where the `top` and `left` values will
5252
// equal negative the scroll position.
53-
const top = documentRect.top < 0 && document.body.scrollTop == 0 ?
54-
-documentRect.top :
55-
document.body.scrollTop;
56-
const left = documentRect.left < 0 && document.body.scrollLeft == 0 ?
57-
-documentRect.left :
58-
document.body.scrollLeft;
53+
const top = -documentRect.top || document.body.scrollTop || window.scrollY || 0;
54+
const left = -documentRect.left || document.body.scrollLeft || window.scrollX || 0;
5955

6056
return {top, left};
6157
}

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

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,7 @@ describe('MdRipple', () => {
195195
expect(pxStringToFloat(ripple.style.height)).toBeCloseTo(2 * expectedRadius, 1);
196196
});
197197

198+
198199
it('cleans up the event handlers when the container gets destroyed', () => {
199200
fixture = TestBed.createComponent(RippleContainerWithNgIf);
200201
fixture.detectChanges();
@@ -208,6 +209,68 @@ describe('MdRipple', () => {
208209
rippleElement.dispatchEvent(createMouseEvent('mousedown'));
209210
expect(rippleBackground.classList).not.toContain('md-ripple-active');
210211
});
212+
213+
describe('when page is scrolled', () => {
214+
var veryLargeElement: HTMLDivElement = document.createElement('div');
215+
var pageScrollTop = 500;
216+
var pageScrollLeft = 500;
217+
218+
beforeEach(() => {
219+
// Add a very large element to make the page scroll
220+
veryLargeElement.style.width = '4000px';
221+
veryLargeElement.style.height = '4000px';
222+
document.body.appendChild(veryLargeElement);
223+
document.body.scrollTop = pageScrollTop;
224+
document.body.scrollLeft = pageScrollLeft;
225+
// Firefox
226+
document.documentElement.scrollLeft = pageScrollLeft;
227+
document.documentElement.scrollTop = pageScrollTop;
228+
// Mobile safari
229+
window.scrollTo(pageScrollLeft, pageScrollTop);
230+
});
231+
232+
afterEach(() => {
233+
document.body.removeChild(veryLargeElement);
234+
document.body.scrollTop = 0;
235+
document.body.scrollLeft = 0;
236+
// Firefox
237+
document.documentElement.scrollLeft = 0;
238+
document.documentElement.scrollTop = 0;
239+
// Mobile safari
240+
window.scrollTo(0, 0);
241+
});
242+
243+
it('create ripple with correct position', () => {
244+
let elementTop = 600;
245+
let elementLeft = 750;
246+
let left = 50;
247+
let top = 75;
248+
249+
rippleElement.style.position = 'absolute';
250+
rippleElement.style.left = `${elementLeft}px`;
251+
rippleElement.style.top = `${elementTop}px`;
252+
253+
// Simulate a keyboard-triggered click by setting event coordinates to 0.
254+
const clickEvent = createMouseEvent('click', {
255+
clientX: left + elementLeft - pageScrollLeft,
256+
clientY: top + elementTop - pageScrollTop,
257+
screenX: left + elementLeft,
258+
screenY: top + elementTop
259+
});
260+
rippleElement.dispatchEvent(clickEvent);
261+
262+
const expectedRadius = Math.sqrt(250 * 250 + 125 * 125);
263+
const expectedLeft = left - expectedRadius;
264+
const expectedTop = top - expectedRadius;
265+
266+
const ripple = <HTMLElement>rippleElement.querySelector('.md-ripple-foreground');
267+
expect(pxStringToFloat(ripple.style.left)).toBeCloseTo(expectedLeft, 1);
268+
expect(pxStringToFloat(ripple.style.top)).toBeCloseTo(expectedTop, 1);
269+
expect(pxStringToFloat(ripple.style.width)).toBeCloseTo(2 * expectedRadius, 1);
270+
expect(pxStringToFloat(ripple.style.height)).toBeCloseTo(2 * expectedRadius, 1);
271+
});
272+
});
273+
211274
});
212275

213276
describe('configuring behavior', () => {

‎src/lib/core/ripple/ripple.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
ForegroundRippleState,
1818
} from './ripple-renderer';
1919
import {DefaultStyleCompatibilityModeModule} from '../compatibility/default-mode';
20+
import {ViewportRuler} from '../overlay/position/viewport-ruler';
2021

2122

2223
@Directive({
@@ -62,14 +63,16 @@ export class MdRipple implements OnInit, OnDestroy, OnChanges {
6263
@HostBinding('class.md-ripple-unbounded') @Input('md-ripple-unbounded') unbounded: boolean;
6364

6465
private _rippleRenderer: RippleRenderer;
66+
_ruler: ViewportRuler;
6567

66-
constructor(_elementRef: ElementRef, _ngZone: NgZone) {
68+
constructor(_elementRef: ElementRef, _ngZone: NgZone, _ruler: ViewportRuler) {
6769
// These event handlers are attached to the element that triggers the ripple animations.
6870
const eventHandlers = new Map<string, (e: Event) => void>();
6971
eventHandlers.set('mousedown', (event: MouseEvent) => this._mouseDown(event));
7072
eventHandlers.set('click', (event: MouseEvent) => this._click(event));
7173
eventHandlers.set('mouseleave', (event: MouseEvent) => this._mouseLeave(event));
7274
this._rippleRenderer = new RippleRenderer(_elementRef, eventHandlers, _ngZone);
75+
this._ruler = _ruler;
7376
}
7477

7578
/** TODO: internal */
@@ -163,7 +166,10 @@ export class MdRipple implements OnInit, OnDestroy, OnChanges {
163166
// FIXME: This fails on IE11, which still sets pageX/Y and screenX/Y on keyboard clicks.
164167
const isKeyEvent =
165168
(event.screenX === 0 && event.screenY === 0 && event.pageX === 0 && event.pageY === 0);
166-
this.end(event.pageX, event.pageY, isKeyEvent);
169+
170+
this.end(event.pageX - this._ruler.getViewportScrollPosition().left,
171+
event.pageY - this._ruler.getViewportScrollPosition().top,
172+
isKeyEvent);
167173
}
168174
}
169175

@@ -188,7 +194,7 @@ export class MdRippleModule {
188194
static forRoot(): ModuleWithProviders {
189195
return {
190196
ngModule: MdRippleModule,
191-
providers: []
197+
providers: [ViewportRuler]
192198
};
193199
}
194200
}

‎src/lib/radio/radio.spec.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import {NgControl, FormsModule, ReactiveFormsModule, FormControl} from '@angular
33
import {Component, DebugElement} from '@angular/core';
44
import {By} from '@angular/platform-browser';
55
import {MdRadioGroup, MdRadioButton, MdRadioChange, MdRadioModule} from './radio';
6+
import {ViewportRuler} from '../core/overlay/position/viewport-ruler';
7+
import {FakeViewportRuler} from '../core/overlay/position/fake-viewport-ruler';
68

79

810
describe('MdRadio', () => {
@@ -16,6 +18,9 @@ describe('MdRadio', () => {
1618
RadioGroupWithFormControl,
1719
StandaloneRadioButtons,
1820
],
21+
providers: [
22+
{provide: ViewportRuler, useClass: FakeViewportRuler},
23+
]
1924
});
2025

2126
TestBed.compileComponents();

‎src/lib/radio/radio.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
DefaultStyleCompatibilityModeModule,
2727
} from '../core';
2828
import {coerceBooleanProperty} from '../core/coersion/boolean-property';
29+
import {ViewportRuler} from '../core/overlay/position/viewport-ruler';
2930

3031

3132
/**
@@ -478,7 +479,7 @@ export class MdRadioModule {
478479
static forRoot(): ModuleWithProviders {
479480
return {
480481
ngModule: MdRadioModule,
481-
providers: [MdUniqueSelectionDispatcher],
482+
providers: [MdUniqueSelectionDispatcher, ViewportRuler],
482483
};
483484
}
484485
}

‎src/lib/tabs/tab-group.spec.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import {Component, ViewChild} from '@angular/core';
66
import {By} from '@angular/platform-browser';
77
import {Observable} from 'rxjs/Observable';
88
import {MdTab} from './tab';
9+
import {ViewportRuler} from '../core/overlay/position/viewport-ruler';
10+
import {FakeViewportRuler} from '../core/overlay/position/fake-viewport-ruler';
911

1012

1113
describe('MdTabGroup', () => {
@@ -19,6 +21,9 @@ describe('MdTabGroup', () => {
1921
AsyncTabsTestApp,
2022
DisabledTabsTestApp,
2123
TabGroupWithSimpleApi,
24+
],
25+
providers: [
26+
{provide: ViewportRuler, useClass: FakeViewportRuler},
2227
]
2328
});
2429

‎src/lib/tabs/tab-group.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import 'rxjs/add/operator/map';
3030
import {MdRippleModule} from '../core/ripple/ripple';
3131
import {MdTab} from './tab';
3232
import {MdTabBody} from './tab-body';
33+
import {ViewportRuler} from '../core/overlay/position/viewport-ruler';
3334

3435

3536
/** Used to generate unique ID's for each tab component */
@@ -296,7 +297,7 @@ export class MdTabsModule {
296297
static forRoot(): ModuleWithProviders {
297298
return {
298299
ngModule: MdTabsModule,
299-
providers: []
300+
providers: [ViewportRuler]
300301
};
301302
}
302303
}

‎src/lib/tabs/tab-nav-bar/tab-nav-bar.spec.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ import {async, ComponentFixture, TestBed} from '@angular/core/testing';
22
import {MdTabsModule} from '../tab-group';
33
import {Component} from '@angular/core';
44
import {By} from '@angular/platform-browser';
5+
import {ViewportRuler} from '../../core/overlay/position/viewport-ruler';
6+
import {FakeViewportRuler} from '../../core/overlay/position/fake-viewport-ruler';
57

68

79
describe('MdTabNavBar', () => {
@@ -13,6 +15,9 @@ describe('MdTabNavBar', () => {
1315
SimpleTabNavBarTestApp,
1416
TabLinkWithNgIf,
1517
],
18+
providers: [
19+
{provide: ViewportRuler, useClass: FakeViewportRuler},
20+
]
1621
});
1722

1823
TestBed.compileComponents();

‎src/lib/tabs/tab-nav-bar/tab-nav-bar.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
} from '@angular/core';
1111
import {MdInkBar} from '../ink-bar';
1212
import {MdRipple} from '../../core/ripple/ripple';
13+
import {ViewportRuler} from '../../core/overlay/position/viewport-ruler';
1314

1415
/**
1516
* Navigation component matching the styles of the tab group header.
@@ -60,8 +61,8 @@ export class MdTabLink {
6061
selector: '[md-tab-link], [mat-tab-link]',
6162
})
6263
export class MdTabLinkRipple extends MdRipple implements OnDestroy {
63-
constructor(private _element: ElementRef, private _ngZone: NgZone) {
64-
super(_element, _ngZone);
64+
constructor(private _element: ElementRef, private _ngZone: NgZone, _ruler: ViewportRuler) {
65+
super(_element, _ngZone, _ruler);
6566
}
6667

6768
// In certain cases the parent destroy handler

0 commit comments

Comments
 (0)