Skip to content

Commit

Permalink
fix(material/list): dispatching model change event multiple times in …
Browse files Browse the repository at this point in the history
…single selection mode (#22376)

Fixes that the MDC-based list dispatches its `ngModelChange` event multiple times when the value changes in single selection mode.

Fixes #22276.

(cherry picked from commit 7702177)
  • Loading branch information
crisbeto authored and mmalerba committed Apr 13, 2021
1 parent bf8dc1b commit 11ff7ba
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 4 deletions.
5 changes: 4 additions & 1 deletion src/material-experimental/mdc-list/list-option.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,10 @@ export class MatListOption extends MatListItemBase implements ListOption, OnInit

if (isSelected !== this._selected) {
this._setSelected(isSelected);
this._selectionList._reportValueChange();

if (isSelected || this._selectionList.multiple) {
this._selectionList._reportValueChange();
}
}
}
private _selected = false;
Expand Down
34 changes: 33 additions & 1 deletion src/material-experimental/mdc-list/selection-list.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1105,6 +1105,34 @@ describe('MDC-based MatSelectionList with forms', () => {

expect(listOptions.map(option => option.selected)).toEqual([true, true, true, false, false]);
}));

it('should dispatch one change event per change when updating a single-selection list',
fakeAsync(() => {
fixture.destroy();
fixture = TestBed.createComponent(SelectionListWithModel);
fixture.componentInstance.multiple = false;
fixture.componentInstance.selectedOptions = ['opt3'];
fixture.detectChanges();
const options = fixture.debugElement.queryAll(By.directive(MatListOption))
.map(optionDebugEl => optionDebugEl.nativeElement);

expect(fixture.componentInstance.modelChangeSpy).not.toHaveBeenCalled();

options[0].click();
fixture.detectChanges();
tick();

expect(fixture.componentInstance.modelChangeSpy).toHaveBeenCalledTimes(1);
expect(fixture.componentInstance.selectedOptions).toEqual(['opt1']);

options[1].click();
fixture.detectChanges();
tick();

expect(fixture.componentInstance.modelChangeSpy).toHaveBeenCalledTimes(2);
expect(fixture.componentInstance.selectedOptions).toEqual(['opt2']);
}));

});

describe('and formControl', () => {
Expand Down Expand Up @@ -1408,13 +1436,17 @@ class SelectionListWithOnlyOneOption {

@Component({
template: `
<mat-selection-list [(ngModel)]="selectedOptions" (ngModelChange)="modelChangeSpy()">
<mat-selection-list
[(ngModel)]="selectedOptions"
(ngModelChange)="modelChangeSpy()"
[multiple]="multiple">
<mat-list-option *ngFor="let option of options" [value]="option">{{option}}</mat-list-option>
</mat-selection-list>`
})
class SelectionListWithModel {
modelChangeSpy = jasmine.createSpy('model change spy');
selectedOptions: string[] = [];
multiple = true;
options = ['opt1', 'opt2', 'opt3'];
}

Expand Down
33 changes: 32 additions & 1 deletion src/material/list/selection-list.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1339,6 +1339,33 @@ describe('MatSelectionList with forms', () => {
expect(selectionListDebug.nativeElement.tabIndex).toBe(-1);
});

it('should dispatch one change event per change when updating a single-selection list',
fakeAsync(() => {
fixture.destroy();
fixture = TestBed.createComponent(SelectionListWithModel);
fixture.componentInstance.multiple = false;
fixture.componentInstance.selectedOptions = ['opt3'];
fixture.detectChanges();
const options = fixture.debugElement.queryAll(By.directive(MatListOption))
.map(optionDebugEl => optionDebugEl.nativeElement);

expect(fixture.componentInstance.modelChangeSpy).not.toHaveBeenCalled();

options[0].click();
fixture.detectChanges();
tick();

expect(fixture.componentInstance.modelChangeSpy).toHaveBeenCalledTimes(1);
expect(fixture.componentInstance.selectedOptions).toEqual(['opt1']);

options[1].click();
fixture.detectChanges();
tick();

expect(fixture.componentInstance.modelChangeSpy).toHaveBeenCalledTimes(2);
expect(fixture.componentInstance.selectedOptions).toEqual(['opt2']);
}));

});

describe('and formControl', () => {
Expand Down Expand Up @@ -1640,13 +1667,17 @@ class SelectionListWithOnlyOneOption {

@Component({
template: `
<mat-selection-list [(ngModel)]="selectedOptions" (ngModelChange)="modelChangeSpy()">
<mat-selection-list
[(ngModel)]="selectedOptions"
(ngModelChange)="modelChangeSpy()"
[multiple]="multiple">
<mat-list-option *ngFor="let option of options" [value]="option">{{option}}</mat-list-option>
</mat-selection-list>`
})
class SelectionListWithModel {
modelChangeSpy = jasmine.createSpy('model change spy');
selectedOptions: string[] = [];
multiple = true;
options = ['opt1', 'opt2', 'opt3'];
}

Expand Down
5 changes: 4 additions & 1 deletion src/material/list/selection-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,10 @@ export class MatListOption extends _MatListOptionMixinBase implements AfterConte

if (isSelected !== this._selected) {
this._setSelected(isSelected);
this.selectionList._reportValueChange();

if (isSelected || this.selectionList.multiple) {
this.selectionList._reportValueChange();
}
}
}

Expand Down

0 comments on commit 11ff7ba

Please sign in to comment.