From a8835d293431632c0add9db5196b8bd1bfc28409 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Wed, 24 Oct 2018 22:47:59 +0200 Subject: [PATCH] fix(list): add ripples to action list items 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. --- src/lib/list/list.spec.ts | 85 +++++++++++++++++++++++++++++---------- src/lib/list/list.ts | 45 +++++++++++++++++---- 2 files changed, 101 insertions(+), 29 deletions(-) diff --git a/src/lib/list/list.spec.ts b/src/lib/list/list.spec.ts index 4f8f2c062516..689bd3675098 100644 --- a/src/lib/list/list.spec.ts +++ b/src/lib/list/list.spec.ts @@ -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'); @@ -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 = fixture.debugElement.componentInstance.listItems; @@ -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; @@ -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(); @@ -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; @@ -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); + }); + }); @@ -205,13 +244,15 @@ class NavListWithOneAnchorItem extends BaseTestList { } @Component({template: ` - - `}) class ActionListWithoutType extends BaseTestList { @ViewChildren(MatListItem) listItems: QueryList; + disableListRipple = false; + disableItemRipple = false; } @Component({template: ` diff --git a/src/lib/list/list.ts b/src/lib/list/list.ts index aa0f0546c1fb..a66b29ee2aa8 100644 --- a/src/lib/list/list.ts +++ b/src/lib/list/list.ts @@ -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) { + 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. @@ -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; @ContentChild(MatListAvatarCssMatStyler) _avatar: MatListAvatarCssMatStyler; @ContentChild(MatListIconCssMatStyler) _icon: MatListIconCssMatStyler; constructor(private _element: ElementRef, - @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