Skip to content

Commit

Permalink
fix(ripple): use element coordinates instead of page coordinates
Browse files Browse the repository at this point in the history
Uses the `clientX` and `clientY` coordinates when creating a ripple, instead of taking `pageX`/`pageY` and subtracting the scroll position since the result is exactly the same. Using the client coordinates has the advantage of being simpler and not having to read the scroll position.

Fixes angular#7436.
  • Loading branch information
crisbeto committed Sep 30, 2017
1 parent 3c6f7a2 commit a48670e
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 35 deletions.
3 changes: 1 addition & 2 deletions src/lib/core/ripple/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import {NgModule} from '@angular/core';
import {ScrollDispatchModule, VIEWPORT_RULER_PROVIDER} from '@angular/cdk/scrolling';
import {ScrollDispatchModule} from '@angular/cdk/scrolling';
import {PlatformModule} from '@angular/cdk/platform';
import {MatCommonModule} from '../common-behaviors/common-module';
import {MatRipple} from './ripple';
Expand All @@ -20,6 +20,5 @@ export {RippleConfig, RIPPLE_FADE_IN_DURATION, RIPPLE_FADE_OUT_DURATION} from '.
imports: [MatCommonModule, PlatformModule, ScrollDispatchModule],
exports: [MatRipple, MatCommonModule],
declarations: [MatRipple],
providers: [VIEWPORT_RULER_PROVIDER],
})
export class MatRippleModule {}
44 changes: 19 additions & 25 deletions src/lib/core/ripple/ripple-renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

import {ElementRef, NgZone} from '@angular/core';
import {Platform} from '@angular/cdk/platform';
import {ViewportRuler} from '@angular/cdk/scrolling';
import {RippleRef, RippleState} from './ripple-ref';


Expand Down Expand Up @@ -56,11 +55,7 @@ export class RippleRenderer {
/** Whether mouse ripples should be created or not. */
rippleDisabled: boolean = false;

constructor(
elementRef: ElementRef,
private _ngZone: NgZone,
private _ruler: ViewportRuler,
platform: Platform) {
constructor(elementRef: ElementRef, private _ngZone: NgZone, platform: Platform) {
// Only do anything if we're on the browser.
if (platform.isBrowser) {
this._containerElement = elementRef.nativeElement;
Expand All @@ -79,27 +74,26 @@ export class RippleRenderer {
}
}

/** Fades in a ripple at the given coordinates. */
fadeInRipple(pageX: number, pageY: number, config: RippleConfig = {}): RippleRef {
let containerRect = this._containerElement.getBoundingClientRect();
/**
* Fades in a ripple at the given coordinates.
* @param x Coordinate within the element, along the X axis at which to start the ripple.
* @param Y Coordinate within the element, along the Y axis at which to start the ripple.
* @param config Extra ripple options.
*/
fadeInRipple(x: number, y: number, config: RippleConfig = {}): RippleRef {
const containerRect = this._containerElement.getBoundingClientRect();

if (config.centered) {
pageX = containerRect.left + containerRect.width / 2;
pageY = containerRect.top + containerRect.height / 2;
} else {
// Subtract scroll values from the coordinates because calculations below
// are always relative to the viewport rectangle.
let scrollPosition = this._ruler.getViewportScrollPosition();
pageX -= scrollPosition.left;
pageY -= scrollPosition.top;
x = containerRect.left + containerRect.width / 2;
y = containerRect.top + containerRect.height / 2;
}

let radius = config.radius || distanceToFurthestCorner(pageX, pageY, containerRect);
let duration = RIPPLE_FADE_IN_DURATION * (1 / (config.speedFactor || 1));
let offsetX = pageX - containerRect.left;
let offsetY = pageY - containerRect.top;
const radius = config.radius || distanceToFurthestCorner(x, y, containerRect);
const duration = RIPPLE_FADE_IN_DURATION * (1 / (config.speedFactor || 1));
const offsetX = x - containerRect.left;
const offsetY = y - containerRect.top;

let ripple = document.createElement('div');
const ripple = document.createElement('div');
ripple.classList.add('mat-ripple-element');

ripple.style.left = `${offsetX - radius}px`;
Expand Down Expand Up @@ -189,7 +183,7 @@ export class RippleRenderer {
private onMousedown(event: MouseEvent) {
if (!this.rippleDisabled) {
this._isPointerDown = true;
this.fadeInRipple(event.pageX, event.pageY, this.rippleConfig);
this.fadeInRipple(event.clientX, event.clientY, this.rippleConfig);
}
}

Expand All @@ -215,9 +209,9 @@ export class RippleRenderer {
/** Function being called whenever the trigger is being touched. */
private onTouchstart(event: TouchEvent) {
if (!this.rippleDisabled) {
const {pageX, pageY} = event.touches[0];
const {clientX, clientY} = event.touches[0];
this._isPointerDown = true;
this.fadeInRipple(pageX, pageY, this.rippleConfig);
this.fadeInRipple(clientX, clientY, this.rippleConfig);
}
}

Expand Down
8 changes: 3 additions & 5 deletions src/lib/core/ripple/ripple.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import {
InjectionToken,
Optional,
} from '@angular/core';
import {ViewportRuler} from '@angular/cdk/scrolling';
import {Platform} from '@angular/cdk/platform';
import {RippleConfig, RippleRenderer} from './ripple-renderer';
import {RippleRef} from './ripple-ref';
Expand Down Expand Up @@ -91,11 +90,10 @@ export class MatRipple implements OnChanges, OnDestroy {
constructor(
elementRef: ElementRef,
ngZone: NgZone,
ruler: ViewportRuler,
platform: Platform,
@Optional() @Inject(MAT_RIPPLE_GLOBAL_OPTIONS) globalOptions: RippleGlobalOptions
) {
this._rippleRenderer = new RippleRenderer(elementRef, ngZone, ruler, platform);
this._rippleRenderer = new RippleRenderer(elementRef, ngZone, platform);
this._globalOptions = globalOptions ? globalOptions : {};

this._updateRippleRenderer();
Expand All @@ -115,8 +113,8 @@ export class MatRipple implements OnChanges, OnDestroy {
}

/** Launches a manual ripple at the specified position. */
launch(pageX: number, pageY: number, config = this.rippleConfig): RippleRef {
return this._rippleRenderer.fadeInRipple(pageX, pageY, config);
launch(x: number, y: number, config = this.rippleConfig): RippleRef {
return this._rippleRenderer.fadeInRipple(x, y, config);
}

/** Fades out all currently showing ripple elements. */
Expand Down
4 changes: 1 addition & 3 deletions src/lib/tabs/tab-nav-bar/tab-nav-bar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {Directionality} from '@angular/cdk/bidi';
import {coerceBooleanProperty} from '@angular/cdk/coercion';
import {Platform} from '@angular/cdk/platform';
import {auditTime, takeUntil} from '@angular/cdk/rxjs';
import {ViewportRuler} from '@angular/cdk/scrolling';
import {
AfterContentInit,
ChangeDetectionStrategy,
Expand Down Expand Up @@ -225,14 +224,13 @@ export class MatTabLink extends _MatTabLinkMixinBase implements OnDestroy, CanDi
constructor(private _tabNavBar: MatTabNav,
private _elementRef: ElementRef,
ngZone: NgZone,
ruler: ViewportRuler,
platform: Platform,
@Optional() @Inject(MAT_RIPPLE_GLOBAL_OPTIONS) globalOptions: RippleGlobalOptions) {
super();

// Manually create a ripple instance that uses the tab link element as trigger element.
// Notice that the lifecycle hooks for the ripple config won't be called anymore.
this._tabLinkRipple = new MatRipple(_elementRef, ngZone, ruler, platform, globalOptions);
this._tabLinkRipple = new MatRipple(_elementRef, ngZone, platform, globalOptions);
}

ngOnDestroy() {
Expand Down

0 comments on commit a48670e

Please sign in to comment.