From d113cc6ac1a42a8dbe6fc38bf5c69eecaf210e7d Mon Sep 17 00:00:00 2001 From: Brian Nenninger Date: Sun, 13 Nov 2016 13:16:08 -0500 Subject: [PATCH 1/2] fix(ripple): don't create background div until ripple becomes enabled --- src/lib/core/ripple/ripple-renderer.ts | 12 ++++++-- src/lib/core/ripple/ripple.spec.ts | 39 ++++++++++++++++++++++++++ src/lib/core/ripple/ripple.ts | 7 +++++ 3 files changed, 55 insertions(+), 3 deletions(-) diff --git a/src/lib/core/ripple/ripple-renderer.ts b/src/lib/core/ripple/ripple-renderer.ts index 396c630c8cb9..ec7ce4fd1076 100644 --- a/src/lib/core/ripple/ripple-renderer.ts +++ b/src/lib/core/ripple/ripple-renderer.ts @@ -47,9 +47,15 @@ export class RippleRenderer { this._rippleElement = _elementRef.nativeElement; // It might be nice to delay creating the background until it's needed, but doing this in // fadeInRippleBackground causes the first click event to not be handled reliably. - this._backgroundDiv = document.createElement('div'); - this._backgroundDiv.classList.add('md-ripple-background'); - this._rippleElement.appendChild(this._backgroundDiv); + this._backgroundDiv = null; + } + + initializeIfNeeded() { + if (!this._backgroundDiv) { + this._backgroundDiv = document.createElement('div'); + this._backgroundDiv.classList.add('md-ripple-background'); + this._rippleElement.appendChild(this._backgroundDiv); + } } /** diff --git a/src/lib/core/ripple/ripple.spec.ts b/src/lib/core/ripple/ripple.spec.ts index 422f908a0ba2..c6985a7c603e 100644 --- a/src/lib/core/ripple/ripple.spec.ts +++ b/src/lib/core/ripple/ripple.spec.ts @@ -293,6 +293,45 @@ describe('MdRipple', () => { expect(pxStringToFloat(ripple.style.height)).toBeCloseTo(2 * customRadius, 1); }); }); + + describe('initially disabled ripple', () => { + let controller: RippleContainerWithInputBindings; + let rippleComponent: MdRipple; + + beforeEach(() => { + fixture = TestBed.createComponent(RippleContainerWithInputBindings); + controller = fixture.debugElement.componentInstance; + controller.disabled = true; + fixture.detectChanges(); + + rippleComponent = controller.ripple; + rippleElement = fixture.debugElement.nativeElement.querySelector('[md-ripple]'); + }); + + it('initially does not create background', () => { + rippleBackground = rippleElement.querySelector('.md-ripple-background'); + expect(rippleBackground).toBeNull(); + }); + + it('creates background when enabled', () => { + rippleBackground = rippleElement.querySelector('.md-ripple-background'); + expect(rippleBackground).toBeNull(); + + controller.disabled = false; + fixture.detectChanges(); + rippleBackground = rippleElement.querySelector('.md-ripple-background'); + expect(rippleBackground).toBeTruthy(); + }); + + it('creates background when manually activated', () => { + rippleBackground = rippleElement.querySelector('.md-ripple-background'); + expect(rippleBackground).toBeNull(); + + rippleComponent.start(); + rippleBackground = rippleElement.querySelector('.md-ripple-background'); + expect(rippleBackground).toBeTruthy(); + }); + }); }); @Component({ diff --git a/src/lib/core/ripple/ripple.ts b/src/lib/core/ripple/ripple.ts index cc4cde1aee59..652d18399d73 100644 --- a/src/lib/core/ripple/ripple.ts +++ b/src/lib/core/ripple/ripple.ts @@ -76,6 +76,9 @@ export class MdRipple implements OnInit, OnDestroy, OnChanges { if (!this.trigger) { this._rippleRenderer.setTriggerElementToHost(); } + if (!this.disabled) { + this._rippleRenderer.initializeIfNeeded(); + } } /** TODO: internal */ @@ -91,12 +94,16 @@ export class MdRipple implements OnInit, OnDestroy, OnChanges { if (changedInputs.indexOf('trigger') !== -1) { this._rippleRenderer.setTriggerElement(this.trigger); } + if (!this.disabled) { + this._rippleRenderer.initializeIfNeeded(); + } } /** * Responds to the start of a ripple animation trigger by fading the background in. */ start() { + this._rippleRenderer.initializeIfNeeded(); this._rippleRenderer.fadeInRippleBackground(this.backgroundColor); } From d1dcd3c008fd5a591afa13856c13221bdddc8fe5 Mon Sep 17 00:00:00 2001 From: Brian Nenninger Date: Sun, 13 Nov 2016 13:42:04 -0500 Subject: [PATCH 2/2] Rename method and add comments. --- src/lib/core/ripple/ripple-renderer.ts | 9 ++++++--- src/lib/core/ripple/ripple.ts | 6 +++--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/lib/core/ripple/ripple-renderer.ts b/src/lib/core/ripple/ripple-renderer.ts index ec7ce4fd1076..f85b9d420954 100644 --- a/src/lib/core/ripple/ripple-renderer.ts +++ b/src/lib/core/ripple/ripple-renderer.ts @@ -45,12 +45,15 @@ export class RippleRenderer { constructor(_elementRef: ElementRef, private _eventHandlers: Map void>) { this._rippleElement = _elementRef.nativeElement; - // It might be nice to delay creating the background until it's needed, but doing this in - // fadeInRippleBackground causes the first click event to not be handled reliably. + // The background div is created in createBackgroundIfNeeded when the ripple becomes enabled. + // This avoids creating unneeded divs when the ripple is always disabled. this._backgroundDiv = null; } - initializeIfNeeded() { + /** + * Creates the div for the ripple background, if it doesn't already exist. + */ + createBackgroundIfNeeded() { if (!this._backgroundDiv) { this._backgroundDiv = document.createElement('div'); this._backgroundDiv.classList.add('md-ripple-background'); diff --git a/src/lib/core/ripple/ripple.ts b/src/lib/core/ripple/ripple.ts index 652d18399d73..31d1e7b07731 100644 --- a/src/lib/core/ripple/ripple.ts +++ b/src/lib/core/ripple/ripple.ts @@ -77,7 +77,7 @@ export class MdRipple implements OnInit, OnDestroy, OnChanges { this._rippleRenderer.setTriggerElementToHost(); } if (!this.disabled) { - this._rippleRenderer.initializeIfNeeded(); + this._rippleRenderer.createBackgroundIfNeeded(); } } @@ -95,7 +95,7 @@ export class MdRipple implements OnInit, OnDestroy, OnChanges { this._rippleRenderer.setTriggerElement(this.trigger); } if (!this.disabled) { - this._rippleRenderer.initializeIfNeeded(); + this._rippleRenderer.createBackgroundIfNeeded(); } } @@ -103,7 +103,7 @@ export class MdRipple implements OnInit, OnDestroy, OnChanges { * Responds to the start of a ripple animation trigger by fading the background in. */ start() { - this._rippleRenderer.initializeIfNeeded(); + this._rippleRenderer.createBackgroundIfNeeded(); this._rippleRenderer.fadeInRippleBackground(this.backgroundColor); }