Skip to content

Commit

Permalink
fix(selection-list): model not updated when option is selected progra…
Browse files Browse the repository at this point in the history 
…mmatically

Fixes the list selection model not being updated when an option in the list is selected programmatically.

Fixes angular#7318.
  • Loading branch information
@crisbeto
crisbeto committed Sep 26, 2017
1 parent 3571f68 commit d3249f2
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 32 deletions.
7 changes: 4 additions & 3 deletions src/demo-app/list/list-demo.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,10 @@ <h2>Selection list</h2>
<md-selection-list #groceries>
<h3 md-subheader>Groceries</h3>

<md-list-option *ngFor="let item of items" [value]="item">
{{item}}
</md-list-option>
<md-list-option value="bananas">Bananas</md-list-option>
<md-list-option selected value="oranges">Oranges</md-list-option>
<md-list-option value="apples">Apples</md-list-option>
<md-list-option value="strawberries">Strawberries</md-list-option>
</md-selection-list>

<p>Selected: {{groceries.selectedOptions.selected.length}}</p>
Expand Down
66 changes: 40 additions & 26 deletions src/lib/list/selection-list.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {MdListModule, MdListOption, MdSelectionList} from './index';
describe('MdSelectionList', () => {
describe('with list option', () => {
let fixture: ComponentFixture<SelectionListWithListOptions>;
let listOption: DebugElement[];
let listOptions: DebugElement[];
let listItemEl: DebugElement;
let selectionList: DebugElement;

Expand All @@ -30,7 +30,7 @@ describe('MdSelectionList', () => {

beforeEach(async(() => {
fixture = TestBed.createComponent(SelectionListWithListOptions);
listOption = fixture.debugElement.queryAll(By.directive(MdListOption));
listOptions = fixture.debugElement.queryAll(By.directive(MdListOption));
listItemEl = fixture.debugElement.query(By.css('.mat-list-item'));
selectionList = fixture.debugElement.query(By.directive(MdSelectionList));
fixture.detectChanges();
Expand All @@ -39,11 +39,11 @@ describe('MdSelectionList', () => {
it('should add and remove focus class on focus/blur', () => {
expect(listItemEl.nativeElement.classList).not.toContain('mat-list-item-focus');

listOption[0].componentInstance._handleFocus();
listOptions[0].componentInstance._handleFocus();
fixture.detectChanges();
expect(listItemEl.nativeElement.className).toContain('mat-list-item-focus');

listOption[0].componentInstance._handleBlur();
listOptions[0].componentInstance._handleBlur();
fixture.detectChanges();
expect(listItemEl.nativeElement.className).not.toContain('mat-list-item-focus');
});
Expand All @@ -52,33 +52,33 @@ describe('MdSelectionList', () => {
const optionValues = ['inbox', 'starred', 'sent-mail', 'drafts'];

optionValues.forEach((optionValue, index) => {
expect(listOption[index].componentInstance.value).toBe(optionValue);
expect(listOptions[index].componentInstance.value).toBe(optionValue);
});
});

it('should be able to dispatch one selected item', () => {
let testListItem = listOption[2].injector.get<MdListOption>(MdListOption);
let testListItem = listOptions[2].injector.get<MdListOption>(MdListOption);
let selectList = selectionList.injector.get<MdSelectionList>(MdSelectionList).selectedOptions;

expect(selectList.selected.length).toBe(0);
expect(listOption[2].nativeElement.getAttribute('aria-selected')).toBe('false');
expect(listOptions[2].nativeElement.getAttribute('aria-selected')).toBe('false');

testListItem.toggle();
fixture.detectChanges();

expect(listOption[2].nativeElement.getAttribute('aria-selected')).toBe('true');
expect(listOption[2].nativeElement.getAttribute('aria-disabled')).toBe('false');
expect(listOptions[2].nativeElement.getAttribute('aria-selected')).toBe('true');
expect(listOptions[2].nativeElement.getAttribute('aria-disabled')).toBe('false');
expect(selectList.selected.length).toBe(1);
});

it('should be able to dispatch multiple selected items', () => {
let testListItem = listOption[2].injector.get<MdListOption>(MdListOption);
let testListItem2 = listOption[1].injector.get<MdListOption>(MdListOption);
let testListItem = listOptions[2].injector.get<MdListOption>(MdListOption);
let testListItem2 = listOptions[1].injector.get<MdListOption>(MdListOption);
let selectList = selectionList.injector.get<MdSelectionList>(MdSelectionList).selectedOptions;

expect(selectList.selected.length).toBe(0);
expect(listOption[2].nativeElement.getAttribute('aria-selected')).toBe('false');
expect(listOption[1].nativeElement.getAttribute('aria-selected')).toBe('false');
expect(listOptions[2].nativeElement.getAttribute('aria-selected')).toBe('false');
expect(listOptions[1].nativeElement.getAttribute('aria-selected')).toBe('false');

testListItem.toggle();
fixture.detectChanges();
Expand All @@ -87,14 +87,14 @@ describe('MdSelectionList', () => {
fixture.detectChanges();

expect(selectList.selected.length).toBe(2);
expect(listOption[2].nativeElement.getAttribute('aria-selected')).toBe('true');
expect(listOption[1].nativeElement.getAttribute('aria-selected')).toBe('true');
expect(listOption[1].nativeElement.getAttribute('aria-disabled')).toBe('false');
expect(listOption[2].nativeElement.getAttribute('aria-disabled')).toBe('false');
expect(listOptions[2].nativeElement.getAttribute('aria-selected')).toBe('true');
expect(listOptions[1].nativeElement.getAttribute('aria-selected')).toBe('true');
expect(listOptions[1].nativeElement.getAttribute('aria-disabled')).toBe('false');
expect(listOptions[2].nativeElement.getAttribute('aria-disabled')).toBe('false');
});

it('should be able to deselect an option', () => {
let testListItem = listOption[2].injector.get<MdListOption>(MdListOption);
let testListItem = listOptions[2].injector.get<MdListOption>(MdListOption);
let selectList = selectionList.injector.get<MdSelectionList>(MdSelectionList).selectedOptions;

expect(selectList.selected.length).toBe(0);
Expand All @@ -111,11 +111,11 @@ describe('MdSelectionList', () => {
});

it('should not allow selection of disabled items', () => {
let testListItem = listOption[0].injector.get<MdListOption>(MdListOption);
let testListItem = listOptions[0].injector.get<MdListOption>(MdListOption);
let selectList = selectionList.injector.get<MdSelectionList>(MdSelectionList).selectedOptions;

expect(selectList.selected.length).toBe(0);
expect(listOption[0].nativeElement.getAttribute('aria-disabled')).toBe('true');
expect(listOptions[0].nativeElement.getAttribute('aria-disabled')).toBe('true');

testListItem._handleClick();
fixture.detectChanges();
Expand All @@ -124,18 +124,18 @@ describe('MdSelectionList', () => {
});

it('should be able to un-disable disabled items', () => {
let testListItem = listOption[0].injector.get<MdListOption>(MdListOption);
let testListItem = listOptions[0].injector.get<MdListOption>(MdListOption);

expect(listOption[0].nativeElement.getAttribute('aria-disabled')).toBe('true');
expect(listOptions[0].nativeElement.getAttribute('aria-disabled')).toBe('true');

testListItem.disabled = false;
fixture.detectChanges();

expect(listOption[0].nativeElement.getAttribute('aria-disabled')).toBe('false');
expect(listOptions[0].nativeElement.getAttribute('aria-disabled')).toBe('false');
});

it('should be able to use keyboard select with SPACE', () => {
let testListItem = listOption[1].nativeElement as HTMLElement;
let testListItem = listOptions[1].nativeElement as HTMLElement;
let SPACE_EVENT: KeyboardEvent =
createKeyboardEvent('keydown', SPACE, testListItem);
let selectList = selectionList.injector.get<MdSelectionList>(MdSelectionList).selectedOptions;
Expand All @@ -153,7 +153,7 @@ describe('MdSelectionList', () => {
});

it('should focus previous item when press UP ARROW', () => {
let testListItem = listOption[2].nativeElement as HTMLElement;
let testListItem = listOptions[2].nativeElement as HTMLElement;
let UP_EVENT: KeyboardEvent =
createKeyboardEvent('keydown', UP_ARROW, testListItem);
let options = selectionList.componentInstance.options;
Expand All @@ -172,7 +172,7 @@ describe('MdSelectionList', () => {
});

it('should focus next item when press DOWN ARROW', () => {
let testListItem = listOption[2].nativeElement as HTMLElement;
let testListItem = listOptions[2].nativeElement as HTMLElement;
let DOWN_EVENT: KeyboardEvent =
createKeyboardEvent('keydown', DOWN_ARROW, testListItem);
let options = selectionList.componentInstance.options;
Expand Down Expand Up @@ -212,6 +212,20 @@ describe('MdSelectionList', () => {

expect(list.options.toArray().every(option => option.selected)).toBe(false);
});

it('should update the list value when an item is selected programmatically', () => {
const list: MdSelectionList = selectionList.componentInstance;

expect(list.selectedOptions.isEmpty()).toBe(true);

listOptions[0].componentInstance.selected = true;
listOptions[2].componentInstance.selected = true;
fixture.detectChanges();

expect(list.selectedOptions.isEmpty()).toBe(false);
expect(list.selectedOptions.isSelected(listOptions[0].componentInstance)).toBe(true);
expect(list.selectedOptions.isSelected(listOptions[2].componentInstance)).toBe(true);
});
});

describe('with single option', () => {
Expand Down
14 changes: 11 additions & 3 deletions src/lib/list/selection-list.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,17 @@ export class MdListOption extends _MdListOptionMixinBase
/** Whether the option is selected. */
@Input()
get selected() { return this._selected; }
set selected(value: boolean) { this._selected = coerceBooleanProperty(value); }
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);
this._changeDetector.markForCheck();
}
}

/** Emitted when the option is focused. */
onFocus = new EventEmitter<MdSelectionListOptionEvent>();
Expand Down Expand Up @@ -146,8 +156,6 @@ export class MdListOption extends _MdListOptionMixinBase
/** Toggles the selection state of the option. */
toggle(): void {
this.selected = !this.selected;
this.selectionList.selectedOptions.toggle(this);
this._changeDetector.markForCheck();
}

/** Allows for programmatic focusing of the option. */
Expand Down

0 comments on commit d3249f2

Please sign in to comment.