Skip to content

Commit 54c0b00

Browse files
crisbetojosephperrott
authored andcommitted
fix(list): selection list always firing change event for selectAll and deselectAll (#11029)
1 parent 385f96d commit 54c0b00

File tree

2 files changed

+52
-9
lines changed

2 files changed

+52
-9
lines changed

src/lib/list/selection-list.spec.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -751,6 +751,30 @@ describe('MatSelectionList with forms', () => {
751751
expect(fixture.componentInstance.selectedOptions).toEqual(['opt1']);
752752
}));
753753

754+
it('should not dispatch the model change event if nothing changed using selectAll', () => {
755+
expect(fixture.componentInstance.modelChangeSpy).not.toHaveBeenCalled();
756+
757+
selectionListDebug.componentInstance.selectAll();
758+
fixture.detectChanges();
759+
760+
expect(fixture.componentInstance.modelChangeSpy).toHaveBeenCalledTimes(1);
761+
762+
selectionListDebug.componentInstance.selectAll();
763+
fixture.detectChanges();
764+
765+
expect(fixture.componentInstance.modelChangeSpy).toHaveBeenCalledTimes(1);
766+
});
767+
768+
it('should not dispatch the model change event if nothing changed using selectAll', () => {
769+
expect(fixture.componentInstance.modelChangeSpy).not.toHaveBeenCalled();
770+
771+
selectionListDebug.componentInstance.deselectAll();
772+
fixture.detectChanges();
773+
774+
expect(fixture.componentInstance.modelChangeSpy).not.toHaveBeenCalled();
775+
});
776+
777+
754778
});
755779

756780
describe('and formControl', () => {
@@ -968,13 +992,14 @@ class SelectionListWithTabindexBinding {
968992

969993
@Component({
970994
template: `
971-
<mat-selection-list [(ngModel)]="selectedOptions">
995+
<mat-selection-list [(ngModel)]="selectedOptions" (ngModelChange)="modelChangeSpy()">
972996
<mat-list-option value="opt1">Option 1</mat-list-option>
973997
<mat-list-option value="opt2">Option 2</mat-list-option>
974998
<mat-list-option value="opt3" *ngIf="renderLastOption">Option 3</mat-list-option>
975999
</mat-selection-list>`
9761000
})
9771001
class SelectionListWithModel {
1002+
modelChangeSpy = jasmine.createSpy('model change spy');
9781003
selectedOptions: string[] = [];
9791004
renderLastOption = true;
9801005
}

src/lib/list/selection-list.ts

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -221,10 +221,10 @@ export class MatListOption extends _MatListOptionMixinBase
221221
return this._element.nativeElement;
222222
}
223223

224-
/** Sets the selected state of the option. */
225-
_setSelected(selected: boolean) {
224+
/** Sets the selected state of the option. Returns whether the value has changed. */
225+
_setSelected(selected: boolean): boolean {
226226
if (selected === this._selected) {
227-
return;
227+
return false;
228228
}
229229

230230
this._selected = selected;
@@ -236,6 +236,7 @@ export class MatListOption extends _MatListOptionMixinBase
236236
}
237237

238238
this._changeDetector.markForCheck();
239+
return true;
239240
}
240241
}
241242

@@ -302,7 +303,6 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements Focu
302303

303304
constructor(private _element: ElementRef, @Attribute('tabindex') tabIndex: string) {
304305
super();
305-
306306
this.tabIndex = parseInt(tabIndex) || 0;
307307
}
308308

@@ -346,14 +346,12 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements Focu
346346

347347
/** Selects all of the options. */
348348
selectAll() {
349-
this.options.forEach(option => option._setSelected(true));
350-
this._reportValueChange();
349+
this._setAllOptionsSelected(true);
351350
}
352351

353352
/** Deselects all of the options. */
354353
deselectAll() {
355-
this.options.forEach(option => option._setSelected(false));
356-
this._reportValueChange();
354+
this._setAllOptionsSelected(false);
357355
}
358356

359357
/** Sets the focused option of the selection-list. */
@@ -479,6 +477,26 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements Focu
479477
}
480478
}
481479

480+
/**
481+
* Sets the selected state on all of the options
482+
* and emits an event if anything changed.
483+
*/
484+
private _setAllOptionsSelected(isSelected: boolean) {
485+
// Keep track of whether anything changed, because we only want to
486+
// emit the changed event when something actually changed.
487+
let hasChanged = false;
488+
489+
this.options.forEach(option => {
490+
if (option._setSelected(isSelected)) {
491+
hasChanged = true;
492+
}
493+
});
494+
495+
if (hasChanged) {
496+
this._reportValueChange();
497+
}
498+
}
499+
482500
/**
483501
* Utility to ensure all indexes are valid.
484502
* @param index The index to be checked.

0 commit comments

Comments
 (0)