From eb3f6da624775bda2068b1d8a08e1a9327d44d1c Mon Sep 17 00:00:00 2001 From: Derek Burgman Date: Thu, 7 Sep 2017 02:14:47 -0500 Subject: [PATCH 1/5] fix(selection-list): fix md-list-option value coercion and selection event emitters * md-list-option value is no longer coerced to a boolean value * md-list-option EventEmitters selectedChange and deselected now emit an event when an option is selected/deselected Fixes #6864 --- src/lib/list/selection-list.spec.ts | 108 +++++++++++++++++++++++++++- src/lib/list/selection-list.ts | 37 +++++----- 2 files changed, 127 insertions(+), 18 deletions(-) diff --git a/src/lib/list/selection-list.spec.ts b/src/lib/list/selection-list.spec.ts index 8ff20613a8c3..292182b7966a 100644 --- a/src/lib/list/selection-list.spec.ts +++ b/src/lib/list/selection-list.spec.ts @@ -4,7 +4,7 @@ import {createKeyboardEvent, dispatchFakeEvent} from '@angular/cdk/testing'; import {Component, DebugElement} from '@angular/core'; import {async, ComponentFixture, inject, TestBed} from '@angular/core/testing'; import {By} from '@angular/platform-browser'; -import {MatListModule, MatListOption, MatSelectionList} from './index'; +import {MatListModule, MatListOption, MatSelectionList, MatSelectionListOptionEvent} from './index'; describe('MatSelectionList', () => { @@ -483,9 +483,88 @@ describe('MatSelectionList', () => { expect(listItemContent.nativeElement.classList).toContain('mat-list-item-content-reverse'); }); }); -}); + describe('with multiple values', () => { + let fixture: ComponentFixture; + let listOption: DebugElement[]; + let listItemEl: DebugElement; + let selectionList: DebugElement; + + beforeEach(async(() => { + TestBed.configureTestingModule({ + imports: [MatListModule], + declarations: [ + SelectionListWithMultipleValues + ], + }); + + TestBed.compileComponents(); + })); + + beforeEach(async(() => { + fixture = TestBed.createComponent(SelectionListWithMultipleValues); + listOption = fixture.debugElement.queryAll(By.directive(MatListOption)); + listItemEl = fixture.debugElement.query(By.css('.mat-list-item')); + selectionList = fixture.debugElement.query(By.directive(MatSelectionList)); + fixture.detectChanges(); + })); + + it('should have a value for each item', () => { + expect(listOption[0].componentInstance.value).toBe(1); + expect(listOption[1].componentInstance.value).toBe('a'); + expect(listOption[2].componentInstance.value).toBe(true); + }); + + }); + + describe('with option selected events', () => { + let fixture: ComponentFixture; + let testComponent: SelectionListWithOptionEvents; + let listOption: DebugElement[]; + let selectionList: DebugElement; + + beforeEach(async(() => { + TestBed.configureTestingModule({ + imports: [MatListModule], + declarations: [ + SelectionListWithOptionEvents + ], + }); + + TestBed.compileComponents(); + })); + + beforeEach(async(() => { + fixture = TestBed.createComponent(SelectionListWithOptionEvents); + testComponent = fixture.debugElement.componentInstance; + listOption = fixture.debugElement.queryAll(By.directive(MatListOption)); + selectionList = fixture.debugElement.query(By.directive(MatSelectionList)); + fixture.detectChanges(); + })); + + it('should trigger the selected and deselected events when clicked in succession.', () => { + + let selected: boolean = false; + + spyOn(testComponent, 'onOptionSelectionChange') + .and.callFake((event: MatSelectionListOptionEvent) => { + selected = event.selected; + }); + + listOption[0].nativeElement.click(); + expect(testComponent.onOptionSelectionChange).toHaveBeenCalledTimes(1); + expect(selected).toBe(true); + + listOption[0].nativeElement.click(); + expect(testComponent.onOptionSelectionChange).toHaveBeenCalledTimes(2); + expect(selected).toBe(false); + }); + + }); + +}); + @Component({template: ` @@ -579,3 +658,28 @@ class SelectionListWithTabindexBinding { tabIndex: number; disabled: boolean; } + +@Component({template: ` + + + 1 + + + a + + + true + +`}) +class SelectionListWithMultipleValues { +} + +@Component({template: ` + + + Inbox + +`}) +class SelectionListWithOptionEvents { + onOptionSelectionChange: (event?: MatSelectionListOptionEvent) => void = () => {}; +} diff --git a/src/lib/list/selection-list.ts b/src/lib/list/selection-list.ts index 4e68a34d5c0d..99401705e1da 100644 --- a/src/lib/list/selection-list.ts +++ b/src/lib/list/selection-list.ts @@ -52,8 +52,9 @@ export class MatListOptionBase {} export const _MatListOptionMixinBase = mixinDisableRipple(MatListOptionBase); /** Event emitted by a selection-list whenever the state of an option is changed. */ -export interface MatSelectionListOptionEvent { +export class MatSelectionListOptionEvent { option: MatListOption; + selected: boolean; } /** @@ -86,7 +87,6 @@ export interface MatSelectionListOptionEvent { export class MatListOption extends _MatListOptionMixinBase implements AfterContentInit, OnInit, OnDestroy, FocusableOption, CanDisableRipple { private _lineSetter: MatLineSetter; - private _selected: boolean = false; private _disabled: boolean = false; /** Whether the option has focus. */ @@ -97,34 +97,29 @@ export class MatListOption extends _MatListOptionMixinBase /** Whether the label should appear before or after the checkbox. Defaults to 'after' */ @Input() checkboxPosition: 'before' | 'after' = 'after'; - /** Value of the option */ - @Input() value: any; - /** Whether the option is disabled. */ @Input() get disabled() { return (this.selectionList && this.selectionList.disabled) || this._disabled; } set disabled(value: any) { this._disabled = coerceBooleanProperty(value); } + /** Value of the option */ + @Input() value: any; + /** Whether the option is selected. */ @Input() - get selected() { return this._selected; } + get selected() { return this.selectionList.selectedOptions.isSelected(this); } set selected(value: boolean) { const isSelected = coerceBooleanProperty(value); - if (isSelected !== this._selected) { - const selectionModel = this.selectionList.selectedOptions; - - this._selected = isSelected; - isSelected ? selectionModel.select(this) : selectionModel.deselect(this); + if (isSelected !== this.selected) { + this.selectionList.selectedOptions.toggle(this); this._changeDetector.markForCheck(); + this.selectionChange.emit(this._createChangeEvent()); } } - /** Emitted when the option is selected. */ - @Output() selectChange = new EventEmitter(); - - /** Emitted when the option is deselected. */ - @Output() deselected = new EventEmitter(); + /** Emitted when the option is selected or deselected. */ + @Output() selectionChange = new EventEmitter(); constructor(private _renderer: Renderer2, private _element: ElementRef, @@ -174,6 +169,16 @@ export class MatListOption extends _MatListOptionMixinBase this.selectionList._setFocusedOption(this); } + /** Creates a selection event object from the specified option. */ + private _createChangeEvent(option: MatListOption = this): MatSelectionListOptionEvent { + let event = new MatSelectionListOptionEvent(); + + event.option = option; + event.selected = option.selected; + + return event; + } + /** Retrieves the DOM element of the component host. */ _getHostElement(): HTMLElement { return this._element.nativeElement; From 390cb1dc12af05bfd621aa1a986b16c432e3216f Mon Sep 17 00:00:00 2001 From: Derek Burgman Date: Thu, 12 Oct 2017 12:38:59 -0500 Subject: [PATCH 2/5] review feedback changes --- src/lib/list/selection-list.spec.ts | 28 ++++++++++++++-------------- src/lib/list/selection-list.ts | 14 ++++++++------ 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/src/lib/list/selection-list.spec.ts b/src/lib/list/selection-list.spec.ts index 292182b7966a..0292332ca7c4 100644 --- a/src/lib/list/selection-list.spec.ts +++ b/src/lib/list/selection-list.spec.ts @@ -586,17 +586,17 @@ class SelectionListWithListOptions { } @Component({template: ` - - + + Inbox (disabled selection-option) - + Starred - + Sent Mail - + Drafts `}) @@ -604,17 +604,17 @@ class SelectionListWithCheckboxPositionAfter { } @Component({template: ` - - + + Inbox (disabled selection-option) - + Starred - + Sent Mail - + Drafts `}) @@ -638,8 +638,8 @@ class SelectionListWithSelectedOption { } @Component({template: ` - - + + Inbox `}) @@ -660,7 +660,7 @@ class SelectionListWithTabindexBinding { } @Component({template: ` - + 1 @@ -675,7 +675,7 @@ class SelectionListWithMultipleValues { } @Component({template: ` - + Inbox diff --git a/src/lib/list/selection-list.ts b/src/lib/list/selection-list.ts index 99401705e1da..c91dac53f45c 100644 --- a/src/lib/list/selection-list.ts +++ b/src/lib/list/selection-list.ts @@ -51,16 +51,18 @@ export const _MatSelectionListMixinBase = export class MatListOptionBase {} export const _MatListOptionMixinBase = mixinDisableRipple(MatListOptionBase); -/** Event emitted by a selection-list whenever the state of an option is changed. */ +/** Change event object emitted by MatListOption */ export class MatSelectionListOptionEvent { - option: MatListOption; + /** The source MatListOption of the event. */ + source: MatListOption; + /** The new `selected` value of the option. */ selected: boolean; } /** * Component for list-options of selection-list. Each list-option can automatically * generate a checkbox and can put current item into the selectionModel of selection-list - * if the current item is checked. + * if the current item is selected. */ @Component({ moduleId: module.id, @@ -107,7 +109,7 @@ export class MatListOption extends _MatListOptionMixinBase /** Whether the option is selected. */ @Input() - get selected() { return this.selectionList.selectedOptions.isSelected(this); } + get selected(): boolean { return this.selectionList.selectedOptions.isSelected(this); } set selected(value: boolean) { const isSelected = coerceBooleanProperty(value); @@ -171,9 +173,9 @@ export class MatListOption extends _MatListOptionMixinBase /** Creates a selection event object from the specified option. */ private _createChangeEvent(option: MatListOption = this): MatSelectionListOptionEvent { - let event = new MatSelectionListOptionEvent(); + const event = new MatSelectionListOptionEvent(); - event.option = option; + event.source = option; event.selected = option.selected; return event; From 801c92cea58129f18640680535507fe447ec6ac4 Mon Sep 17 00:00:00 2001 From: Derek Burgman Date: Thu, 12 Oct 2017 12:40:34 -0500 Subject: [PATCH 3/5] review feedback changes --- src/lib/list/selection-list.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/list/selection-list.ts b/src/lib/list/selection-list.ts index c91dac53f45c..8d303390458d 100644 --- a/src/lib/list/selection-list.ts +++ b/src/lib/list/selection-list.ts @@ -111,7 +111,7 @@ export class MatListOption extends _MatListOptionMixinBase @Input() get selected(): boolean { return this.selectionList.selectedOptions.isSelected(this); } set selected(value: boolean) { - const isSelected = coerceBooleanProperty(value); + const isSelected: boolean = coerceBooleanProperty(value); if (isSelected !== this.selected) { this.selectionList.selectedOptions.toggle(this); From bc14f661ea825ee2ce5aa81b2430b8a7dc3c7703 Mon Sep 17 00:00:00 2001 From: Derek Burgman Date: Thu, 12 Oct 2017 12:56:41 -0500 Subject: [PATCH 4/5] review feedback changes --- src/lib/list/selection-list.spec.ts | 6 +++--- src/lib/list/selection-list.ts | 14 ++++++++------ 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/lib/list/selection-list.spec.ts b/src/lib/list/selection-list.spec.ts index 0292332ca7c4..ee71934b54c5 100644 --- a/src/lib/list/selection-list.spec.ts +++ b/src/lib/list/selection-list.spec.ts @@ -4,7 +4,7 @@ import {createKeyboardEvent, dispatchFakeEvent} from '@angular/cdk/testing'; import {Component, DebugElement} from '@angular/core'; import {async, ComponentFixture, inject, TestBed} from '@angular/core/testing'; import {By} from '@angular/platform-browser'; -import {MatListModule, MatListOption, MatSelectionList, MatSelectionListOptionEvent} from './index'; +import {MatListModule, MatListOption, MatSelectionList, MatListOptionChange} from './index'; describe('MatSelectionList', () => { @@ -548,7 +548,7 @@ describe('MatSelectionList', () => { let selected: boolean = false; spyOn(testComponent, 'onOptionSelectionChange') - .and.callFake((event: MatSelectionListOptionEvent) => { + .and.callFake((event: MatListOptionChange) => { selected = event.selected; }); @@ -681,5 +681,5 @@ class SelectionListWithMultipleValues { `}) class SelectionListWithOptionEvents { - onOptionSelectionChange: (event?: MatSelectionListOptionEvent) => void = () => {}; + onOptionSelectionChange: (event?: MatListOptionChange) => void = () => {}; } diff --git a/src/lib/list/selection-list.ts b/src/lib/list/selection-list.ts index 8d303390458d..f88f52881124 100644 --- a/src/lib/list/selection-list.ts +++ b/src/lib/list/selection-list.ts @@ -52,7 +52,7 @@ export class MatListOptionBase {} export const _MatListOptionMixinBase = mixinDisableRipple(MatListOptionBase); /** Change event object emitted by MatListOption */ -export class MatSelectionListOptionEvent { +export class MatListOptionChange { /** The source MatListOption of the event. */ source: MatListOption; /** The new `selected` value of the option. */ @@ -101,8 +101,10 @@ export class MatListOption extends _MatListOptionMixinBase /** Whether the option is disabled. */ @Input() - get disabled() { return (this.selectionList && this.selectionList.disabled) || this._disabled; } - set disabled(value: any) { this._disabled = coerceBooleanProperty(value); } + get disabled(): boolean { + return (this.selectionList && this.selectionList.disabled) || this._disabled; + } + set disabled(value: boolean) { this._disabled = coerceBooleanProperty(value); } /** Value of the option */ @Input() value: any; @@ -121,7 +123,7 @@ export class MatListOption extends _MatListOptionMixinBase } /** Emitted when the option is selected or deselected. */ - @Output() selectionChange = new EventEmitter(); + @Output() selectionChange = new EventEmitter(); constructor(private _renderer: Renderer2, private _element: ElementRef, @@ -172,8 +174,8 @@ export class MatListOption extends _MatListOptionMixinBase } /** Creates a selection event object from the specified option. */ - private _createChangeEvent(option: MatListOption = this): MatSelectionListOptionEvent { - const event = new MatSelectionListOptionEvent(); + private _createChangeEvent(option: MatListOption = this): MatListOptionChange { + const event = new MatListOptionChange(); event.source = option; event.selected = option.selected; From efab749e2d5671ff870f8d0a96fd620526d74e4f Mon Sep 17 00:00:00 2001 From: Derek Burgman Date: Thu, 12 Oct 2017 13:08:41 -0500 Subject: [PATCH 5/5] review feedback changes --- src/lib/list/selection-list.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/list/selection-list.ts b/src/lib/list/selection-list.ts index f88f52881124..26b8d0146732 100644 --- a/src/lib/list/selection-list.ts +++ b/src/lib/list/selection-list.ts @@ -113,7 +113,7 @@ export class MatListOption extends _MatListOptionMixinBase @Input() get selected(): boolean { return this.selectionList.selectedOptions.isSelected(this); } set selected(value: boolean) { - const isSelected: boolean = coerceBooleanProperty(value); + const isSelected = coerceBooleanProperty(value); if (isSelected !== this.selected) { this.selectionList.selectedOptions.toggle(this);