fix(ripple): don't create background div until ripple becomes enabled (#1849)

Author: dozingcat

src/lib/core/ripple/ripple-renderer.ts

@@ -45,11 +45,20 @@ export class RippleRenderer {
 
   constructor(_elementRef: ElementRef, private _eventHandlers: Map<string, (e: Event) => 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);
+    }
   }
 
   /**

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({

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);
   }