Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 17, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 61 additions & 2 deletions src/lib/list/list.spec.ts
Original file line number Diff line number Diff line change
@@ -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({
Expand Down Expand Up @@ -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.');
}));

});


Expand Down
53 changes: 49 additions & 4 deletions src/lib/list/list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ import {
Optional,
QueryList,
ViewEncapsulation,
OnChanges,
OnDestroy,
ChangeDetectorRef,
} from '@angular/core';
import {
CanDisableRipple,
Expand All @@ -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 */
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -94,6 +114,14 @@ export class MatList extends _MatListMixinBase implements CanDisableRipple {

return null;
}

ngOnChanges() {
this._stateChanges.next();
}

ngOnDestroy() {
this._stateChanges.complete();
}
}

/**
Expand Down Expand Up @@ -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;
Expand All @@ -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 ||
Expand Down
15 changes: 11 additions & 4 deletions tools/public_api_guard/lib/list.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down