From d7a54ef9dceaf3d68e5a936b02586a7694e8c67b Mon Sep 17 00:00:00 2001 From: Brian Nenninger Date: Tue, 15 Nov 2016 14:22:21 -0500 Subject: [PATCH] fix(ripple): don't create background div until ripple becomes enabled (#1849) --- src/lib/core/ripple/ripple-renderer.ts | 19 +++++++++---- src/lib/core/ripple/ripple.spec.ts | 39 ++++++++++++++++++++++++++ src/lib/core/ripple/ripple.ts | 7 +++++ 3 files changed, 60 insertions(+), 5 deletions(-) diff --git a/src/lib/core/ripple/ripple-renderer.ts b/src/lib/core/ripple/ripple-renderer.ts index 396c630c8cb9..f85b9d420954 100644 --- a/src/lib/core/ripple/ripple-renderer.ts +++ b/src/lib/core/ripple/ripple-renderer.ts @@ -45,11 +45,20 @@ 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. - this._backgroundDiv = document.createElement('div'); - this._backgroundDiv.classList.add('md-ripple-background'); - this._rippleElement.appendChild(this._backgroundDiv); + // 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; + } + + /** + * 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'); + 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..31d1e7b07731 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.createBackgroundIfNeeded(); + } } /** 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.createBackgroundIfNeeded(); + } } /** * Responds to the start of a ripple animation trigger by fading the background in. */ start() { + this._rippleRenderer.createBackgroundIfNeeded(); this._rippleRenderer.fadeInRippleBackground(this.backgroundColor); }