fix(list): disableRipple on list input not taking effect after init (#14836)

Fixes the `disableRipple` on the list not having an effect if it's changed through the input after init.

Fixes #14824.

Author: crisbeto

src/lib/list/list.spec.ts

@@ -1,10 +1,13 @@
-import {async, TestBed} from '@angular/core/testing';
+import {async, TestBed, fakeAsync, tick} from '@angular/core/testing';
 import {Component, QueryList, ViewChildren} from '@angular/core';
+import {defaultRippleAnimationConfig} from '@angular/material/core';
+import {dispatchMouseEvent} from '@angular/cdk/testing';
 import {By} from '@angular/platform-browser';
 import {MatListItem, MatListModule} from './index';
 
-
 describe('MatList', () => {
+  // Default ripple durations used for testing.
+  const {enterDuration, exitDuration} = defaultRippleAnimationConfig;
 
   beforeEach(async(() => {
     TestBed.configureTestingModule({
@@ -207,6 +210,62 @@ describe('MatList', () => {
     expect(items.every(item => item._isRippleDisabled())).toBe(true);
   });
 
+  it('should disable item ripples when list ripples are disabled via the input in nav list',
+    fakeAsync(() => {
+      const fixture = TestBed.createComponent(NavListWithOneAnchorItem);
+      fixture.detectChanges();
+
+      const rippleTarget = fixture.nativeElement.querySelector('.mat-list-item-content');
+
+      dispatchMouseEvent(rippleTarget, 'mousedown');
+      dispatchMouseEvent(rippleTarget, 'mouseup');
+
+      expect(rippleTarget.querySelectorAll('.mat-ripple-element').length)
+          .toBe(1, 'Expected ripples to be enabled by default.');
+
+      // Wait for the ripples to go away.
+      tick(enterDuration + exitDuration);
+      expect(rippleTarget.querySelectorAll('.mat-ripple-element').length)
+          .toBe(0, 'Expected ripples to go away.');
+
+      fixture.componentInstance.disableListRipple = true;
+      fixture.detectChanges();
+
+      dispatchMouseEvent(rippleTarget, 'mousedown');
+      dispatchMouseEvent(rippleTarget, 'mouseup');
+
+      expect(rippleTarget.querySelectorAll('.mat-ripple-element').length)
+          .toBe(0, 'Expected no ripples after list ripples are disabled.');
+    }));
+
+  it('should disable item ripples when list ripples are disabled via the input in an action list',
+    fakeAsync(() => {
+      const fixture = TestBed.createComponent(ActionListWithoutType);
+      fixture.detectChanges();
+
+      const rippleTarget = fixture.nativeElement.querySelector('.mat-list-item-content');
+
+      dispatchMouseEvent(rippleTarget, 'mousedown');
+      dispatchMouseEvent(rippleTarget, 'mouseup');
+
+      expect(rippleTarget.querySelectorAll('.mat-ripple-element').length)
+          .toBe(1, 'Expected ripples to be enabled by default.');
+
+      // Wait for the ripples to go away.
+      tick(enterDuration + exitDuration);
+      expect(rippleTarget.querySelectorAll('.mat-ripple-element').length)
+          .toBe(0, 'Expected ripples to go away.');
+
+      fixture.componentInstance.disableListRipple = true;
+      fixture.detectChanges();
+
+      dispatchMouseEvent(rippleTarget, 'mousedown');
+      dispatchMouseEvent(rippleTarget, 'mouseup');
+
+      expect(rippleTarget.querySelectorAll('.mat-ripple-element').length)
+          .toBe(0, 'Expected no ripples after list ripples are disabled.');
+    }));
+
 });
 
 

src/lib/list/list.ts

@@ -17,6 +17,9 @@ import {
   Optional,
   QueryList,
   ViewEncapsulation,
+  OnChanges,
+  OnDestroy,
+  ChangeDetectorRef,
 } from '@angular/core';
 import {
   CanDisableRipple,
@@ -25,6 +28,8 @@ import {
   setLines,
   mixinDisableRipple,
 } from '@angular/material/core';
+import {Subject} from 'rxjs';
+import {takeUntil} from 'rxjs/operators';
 
 // Boilerplate for applying mixins to MatList.
 /** @docs-private */
@@ -52,7 +57,19 @@ export const _MatListItemMixinBase: CanDisableRippleCtor & typeof MatListItemBas
   encapsulation: ViewEncapsulation.None,
   changeDetection: ChangeDetectionStrategy.OnPush,
 })
-export class MatNavList extends _MatListMixinBase implements CanDisableRipple {}
+export class MatNavList extends _MatListMixinBase implements CanDisableRipple, OnChanges,
+  OnDestroy {
+  /** Emits when the state of the list changes. */
+  _stateChanges = new Subject<void>();
+
+  ngOnChanges() {
+    this._stateChanges.next();
+  }
+
+  ngOnDestroy() {
+    this._stateChanges.complete();
+  }
+}
 
 @Component({
   moduleId: module.id,
@@ -67,7 +84,10 @@ export class MatNavList extends _MatListMixinBase implements CanDisableRipple {}
   encapsulation: ViewEncapsulation.None,
   changeDetection: ChangeDetectionStrategy.OnPush,
 })
-export class MatList extends _MatListMixinBase implements CanDisableRipple {
+export class MatList extends _MatListMixinBase implements CanDisableRipple, OnChanges, OnDestroy {
+  /** Emits when the state of the list changes. */
+  _stateChanges = new Subject<void>();
+
   /**
    * @deprecated _elementRef parameter to be made required.
    * @breaking-change 8.0.0
@@ -94,6 +114,14 @@ export class MatList extends _MatListMixinBase implements CanDisableRipple {
 
     return null;
   }
+
+  ngOnChanges() {
+    this._stateChanges.next();
+  }
+
+  ngOnDestroy() {
+    this._stateChanges.complete();
+  }
 }
 
 /**
@@ -143,17 +171,20 @@ export class MatListSubheaderCssMatStyler {}
   changeDetection: ChangeDetectionStrategy.OnPush,
 })
 export class MatListItem extends _MatListItemMixinBase implements AfterContentInit,
-    CanDisableRipple {
+    CanDisableRipple, OnDestroy {
   private _isInteractiveList: boolean = false;
   private _list?: MatNavList | MatList;
+  private _destroyed = new Subject<void>();
 
   @ContentChildren(MatLine) _lines: QueryList<MatLine>;
   @ContentChild(MatListAvatarCssMatStyler) _avatar: MatListAvatarCssMatStyler;
   @ContentChild(MatListIconCssMatStyler) _icon: MatListIconCssMatStyler;
 
   constructor(private _element: ElementRef<HTMLElement>,
               @Optional() navList?: MatNavList,
-              @Optional() list?: MatList) {
+              @Optional() list?: MatList,
+              // @breaking-change 8.0.0 `_changeDetectorRef` to be made into a required parameter.
+              _changeDetectorRef?: ChangeDetectorRef) {
     super();
     this._isInteractiveList = !!(navList || (list && list._getListType() === 'action-list'));
     this._list = navList || list;
@@ -165,12 +196,26 @@ export class MatListItem extends _MatListItemMixinBase implements AfterContentIn
     if (element.nodeName.toLowerCase() === 'button' && !element.hasAttribute('type')) {
       element.setAttribute('type', 'button');
     }
+
+    // @breaking-change 8.0.0 Remove null check for _changeDetectorRef.
+    if (this._list && _changeDetectorRef) {
+      // React to changes in the state of the parent list since
+      // some of the item's properties depend on it (e.g. `disableRipple`).
+      this._list._stateChanges.pipe(takeUntil(this._destroyed)).subscribe(() => {
+        _changeDetectorRef.markForCheck();
+      });
+    }
   }
 
   ngAfterContentInit() {
     setLines(this._lines, this._element);
   }
 
+  ngOnDestroy() {
+    this._destroyed.next();
+    this._destroyed.complete();
+  }
+
   /** Whether this list item should show a ripple effect when clicked. */
   _isRippleDisabled() {
     return !this._isInteractiveList || this.disableRipple ||

tools/public_api_guard/lib/list.d.ts

@@ -8,9 +8,12 @@ export declare const _MatSelectionListMixinBase: CanDisableRippleCtor & typeof M
 
 export declare const MAT_SELECTION_LIST_VALUE_ACCESSOR: any;
 
-export declare class MatList extends _MatListMixinBase implements CanDisableRipple {
+export declare class MatList extends _MatListMixinBase implements CanDisableRipple, OnChanges, OnDestroy {
+    _stateChanges: Subject<void>;
     constructor(_elementRef?: ElementRef<HTMLElement> | undefined);
     _getListType(): 'list' | 'action-list' | null;
+    ngOnChanges(): void;
+    ngOnDestroy(): void;
 }
 
 export declare class MatListAvatarCssMatStyler {
@@ -22,14 +25,15 @@ export declare class MatListBase {
 export declare class MatListIconCssMatStyler {
 }
 
-export declare class MatListItem extends _MatListItemMixinBase implements AfterContentInit, CanDisableRipple {
+export declare class MatListItem extends _MatListItemMixinBase implements AfterContentInit, CanDisableRipple, OnDestroy {
     _avatar: MatListAvatarCssMatStyler;
     _icon: MatListIconCssMatStyler;
     _lines: QueryList<MatLine>;
-    constructor(_element: ElementRef<HTMLElement>, navList?: MatNavList, list?: MatList);
+    constructor(_element: ElementRef<HTMLElement>, navList?: MatNavList, list?: MatList, _changeDetectorRef?: ChangeDetectorRef);
     _getHostElement(): HTMLElement;
     _isRippleDisabled(): boolean;
     ngAfterContentInit(): void;
+    ngOnDestroy(): void;
 }
 
 export declare class MatListItemBase {
@@ -71,7 +75,10 @@ export declare class MatListOptionBase {
 export declare class MatListSubheaderCssMatStyler {
 }
 
-export declare class MatNavList extends _MatListMixinBase implements CanDisableRipple {
+export declare class MatNavList extends _MatListMixinBase implements CanDisableRipple, OnChanges, OnDestroy {
+    _stateChanges: Subject<void>;
+    ngOnChanges(): void;
+    ngOnDestroy(): void;
 }
 
 export declare class MatSelectionList extends _MatSelectionListMixinBase implements FocusableOption, CanDisableRipple, AfterContentInit, ControlValueAccessor, OnDestroy {