Skip to content

Commit

Permalink
fix(list): add ripples to action list items (#13799)
Browse files Browse the repository at this point in the history 
Sets up ripples for the items inside an action list and makes it a little easier to set ripples up in the future, if we add other list types.

Fixes #13795.
  • Loading branch information
@crisbeto @jelbourn
crisbeto authored and jelbourn committed Nov 3, 2018
1 parent 235add9 commit 76044e8
Show file tree
Hide file tree
Showing 2 changed files with 101 additions and 29 deletions.
85 changes: 63 additions & 22 deletions src/lib/list/list.spec.ts
View file
Expand Up @@ -29,62 +29,62 @@ describe('MatList', () => {
}));

it('should not apply any additional class to a list without lines', () => {
let fixture = TestBed.createComponent(ListWithOneItem);
let listItem = fixture.debugElement.query(By.css('mat-list-item'));
const fixture = TestBed.createComponent(ListWithOneItem);
const listItem = fixture.debugElement.query(By.css('mat-list-item'));
fixture.detectChanges();
expect(listItem.nativeElement.className).toBe('mat-list-item');
});

it('should apply mat-2-line class to lists with two lines', () => {
let fixture = TestBed.createComponent(ListWithTwoLineItem);
const fixture = TestBed.createComponent(ListWithTwoLineItem);
fixture.detectChanges();

let listItems = fixture.debugElement.children[0].queryAll(By.css('mat-list-item'));
const listItems = fixture.debugElement.children[0].queryAll(By.css('mat-list-item'));
expect(listItems[0].nativeElement.className).toContain('mat-2-line');
expect(listItems[1].nativeElement.className).toContain('mat-2-line');
});

it('should apply mat-3-line class to lists with three lines', () => {
let fixture = TestBed.createComponent(ListWithThreeLineItem);
const fixture = TestBed.createComponent(ListWithThreeLineItem);
fixture.detectChanges();

let listItems = fixture.debugElement.children[0].queryAll(By.css('mat-list-item'));
const listItems = fixture.debugElement.children[0].queryAll(By.css('mat-list-item'));
expect(listItems[0].nativeElement.className).toContain('mat-3-line');
expect(listItems[1].nativeElement.className).toContain('mat-3-line');
});

it('should apply mat-multi-line class to lists with more than 3 lines', () => {
let fixture = TestBed.createComponent(ListWithManyLines);
const fixture = TestBed.createComponent(ListWithManyLines);
fixture.detectChanges();

let listItems = fixture.debugElement.children[0].queryAll(By.css('mat-list-item'));
const listItems = fixture.debugElement.children[0].queryAll(By.css('mat-list-item'));
expect(listItems[0].nativeElement.className).toContain('mat-multi-line');
expect(listItems[1].nativeElement.className).toContain('mat-multi-line');
});

it('should apply a class to list items with avatars', () => {
let fixture = TestBed.createComponent(ListWithAvatar);
const fixture = TestBed.createComponent(ListWithAvatar);
fixture.detectChanges();

let listItems = fixture.debugElement.children[0].queryAll(By.css('mat-list-item'));
const listItems = fixture.debugElement.children[0].queryAll(By.css('mat-list-item'));
expect(listItems[0].nativeElement.className).toContain('mat-list-item-with-avatar');
expect(listItems[1].nativeElement.className).not.toContain('mat-list-item-with-avatar');
});

it('should not clear custom classes provided by user', () => {
let fixture = TestBed.createComponent(ListWithItemWithCssClass);
const fixture = TestBed.createComponent(ListWithItemWithCssClass);
fixture.detectChanges();

let listItems = fixture.debugElement.children[0].queryAll(By.css('mat-list-item'));
const listItems = fixture.debugElement.children[0].queryAll(By.css('mat-list-item'));
expect(listItems[0].nativeElement.classList.contains('test-class')).toBe(true);
});

it('should update classes if number of lines change', () => {
let fixture = TestBed.createComponent(ListWithDynamicNumberOfLines);
const fixture = TestBed.createComponent(ListWithDynamicNumberOfLines);
fixture.debugElement.componentInstance.showThirdLine = false;
fixture.detectChanges();

let listItem = fixture.debugElement.children[0].query(By.css('mat-list-item'));
const listItem = fixture.debugElement.children[0].query(By.css('mat-list-item'));
expect(listItem.nativeElement.classList.length).toBe(2);
expect(listItem.nativeElement.classList).toContain('mat-2-line');
expect(listItem.nativeElement.classList).toContain('mat-list-item');
Expand All @@ -95,17 +95,17 @@ describe('MatList', () => {
});

it('should add aria roles properly', () => {
let fixture = TestBed.createComponent(ListWithMultipleItems);
const fixture = TestBed.createComponent(ListWithMultipleItems);
fixture.detectChanges();

let list = fixture.debugElement.children[0];
let listItem = fixture.debugElement.children[0].query(By.css('mat-list-item'));
const list = fixture.debugElement.children[0];
const listItem = fixture.debugElement.children[0].query(By.css('mat-list-item'));
expect(list.nativeElement.getAttribute('role')).toBeNull('Expect mat-list no role');
expect(listItem.nativeElement.getAttribute('role')).toBeNull('Expect mat-list-item no role');
});

it('should not show ripples for non-nav lists', () => {
let fixture = TestBed.createComponent(ListWithOneAnchorItem);
const fixture = TestBed.createComponent(ListWithOneAnchorItem);
fixture.detectChanges();

const items: QueryList<MatListItem> = fixture.debugElement.componentInstance.listItems;
Expand All @@ -114,7 +114,7 @@ describe('MatList', () => {
});

it('should allow disabling ripples for specific nav-list items', () => {
let fixture = TestBed.createComponent(NavListWithOneAnchorItem);
const fixture = TestBed.createComponent(NavListWithOneAnchorItem);
fixture.detectChanges();

const items = fixture.componentInstance.listItems;
Expand All @@ -137,6 +137,29 @@ describe('MatList', () => {
expect(items.length).toBeGreaterThan(0);
});

it('should enable ripples for action lists by default', () => {
const fixture = TestBed.createComponent(ActionListWithoutType);
fixture.detectChanges();

const items = fixture.componentInstance.listItems;
expect(items.toArray().every(item => !item._isRippleDisabled())).toBe(true);
});

it('should allow disabling ripples for specific action list items', () => {
const fixture = TestBed.createComponent(ActionListWithoutType);
fixture.detectChanges();

const items = fixture.componentInstance.listItems.toArray();
expect(items.length).toBeGreaterThan(0);

expect(items.every(item => !item._isRippleDisabled())).toBe(true);

fixture.componentInstance.disableItemRipple = true;
fixture.detectChanges();

expect(items.every(item => item._isRippleDisabled())).toBe(true);
});

it('should set default type attribute to button for action list', () => {
const fixture = TestBed.createComponent(ActionListWithoutType);
fixture.detectChanges();
Expand All @@ -154,7 +177,7 @@ describe('MatList', () => {
});

it('should allow disabling ripples for the whole nav-list', () => {
let fixture = TestBed.createComponent(NavListWithOneAnchorItem);
const fixture = TestBed.createComponent(NavListWithOneAnchorItem);
fixture.detectChanges();

const items = fixture.componentInstance.listItems;
Expand All @@ -168,6 +191,22 @@ describe('MatList', () => {

items.forEach(item => expect(item._isRippleDisabled()).toBe(true));
});

it('should allow disabling ripples for the entire action list', () => {
const fixture = TestBed.createComponent(ActionListWithoutType);
fixture.detectChanges();

const items = fixture.componentInstance.listItems.toArray();
expect(items.length).toBeGreaterThan(0);

expect(items.every(item => !item._isRippleDisabled())).toBe(true);

fixture.componentInstance.disableListRipple = true;
fixture.detectChanges();

expect(items.every(item => item._isRippleDisabled())).toBe(true);
});

});


Expand Down Expand Up @@ -205,13 +244,15 @@ class NavListWithOneAnchorItem extends BaseTestList {
}

@Component({template: `
<mat-action-list>
<button mat-list-item>
<mat-action-list [disableRipple]="disableListRipple">
<button mat-list-item [disableRipple]="disableItemRipple">
Paprika
</button>
</mat-action-list>`})
class ActionListWithoutType extends BaseTestList {
@ViewChildren(MatListItem) listItems: QueryList<MatListItem>;
disableListRipple = false;
disableItemRipple = false;
}

@Component({template: `
Expand Down
45 changes: 38 additions & 7 deletions src/lib/list/list.ts
View file
Expand Up @@ -65,7 +65,34 @@ 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 {
/**
* @deprecated _elementRef parameter to be made required.
* @breaking-change 8.0.0
*/
constructor(private _elementRef?: ElementRef<HTMLElement>) {
super();
}

_getListType(): 'list' | 'action-list' | null {
const elementRef = this._elementRef;

// @breaking-change 8.0.0 Remove null check once _elementRef is a required param.
if (elementRef) {
const nodeName = elementRef.nativeElement.nodeName.toLowerCase();

if (nodeName === 'mat-list') {
return 'list';
}

if (nodeName === 'mat-action-list') {
return 'action-list';
}
}

return null;
}
}

/**
* Directive whose purpose is to add the mat- CSS styling to this selector.
Expand Down Expand Up @@ -115,22 +142,25 @@ export class MatListSubheaderCssMatStyler {}
})
export class MatListItem extends _MatListItemMixinBase implements AfterContentInit,
CanDisableRipple {
private _isNavList: boolean = false;
private _isInteractiveList: boolean = false;
private _list?: MatNavList | MatList;

@ContentChildren(MatLine) _lines: QueryList<MatLine>;
@ContentChild(MatListAvatarCssMatStyler) _avatar: MatListAvatarCssMatStyler;
@ContentChild(MatListIconCssMatStyler) _icon: MatListIconCssMatStyler;

constructor(private _element: ElementRef<HTMLElement>,
@Optional() private _navList: MatNavList) {
@Optional() navList?: MatNavList,
@Optional() list?: MatList) {
super();
this._isNavList = !!_navList;
this._isInteractiveList = !!(navList || (list && list._getListType() === 'action-list'));
this._list = navList || list;

// If no type attributed is specified for <button>, set it to "button".
// If a type attribute is already specified, do nothing.
const element = this._getHostElement();
if (element.nodeName && element.nodeName.toLowerCase() === 'button'
&& !element.hasAttribute('type')) {

if (element.nodeName.toLowerCase() === 'button' && !element.hasAttribute('type')) {
element.setAttribute('type', 'button');
}
}
Expand All @@ -141,7 +171,8 @@ export class MatListItem extends _MatListItemMixinBase implements AfterContentIn

/** Whether this list item should show a ripple effect when clicked. */
_isRippleDisabled() {
return !this._isNavList || this.disableRipple || this._navList.disableRipple;
return !this._isInteractiveList || this.disableRipple ||
!!(this._list && this._list.disableRipple);
}

/** Retrieves the DOM element of the component host. */
Expand Down

0 comments on commit 76044e8

Please sign in to comment.