Skip to content

Commit

Permalink
fix(select): emitting change event twice for reset values (#13598)
Browse files Browse the repository at this point in the history
Fixes `mat-select` emitting its change event twice when a reset value is selected, as well as when it's selected twice in a row. This PR covers #10859 which would've introduced another issue.

Fixes #10675.
Fixes #13579.
  • Loading branch information
crisbeto committed Jul 29, 2020
1 parent a388cc3 commit 77b11f4
Show file tree
Hide file tree
Showing 2 changed files with 124 additions and 5 deletions.
115 changes: 115 additions & 0 deletions src/material/select/select.spec.ts
Expand Up @@ -2910,6 +2910,44 @@ describe('MatSelect', () => {
}));
});

describe('with reset option and a form control', () => {
let fixture: ComponentFixture<SelectWithResetOptionAndFormControl>;
let options: HTMLElement[];

beforeEach(fakeAsync(() => {
configureMatSelectTestingModule([SelectWithResetOptionAndFormControl]);
fixture = TestBed.createComponent(SelectWithResetOptionAndFormControl);
fixture.detectChanges();
fixture.debugElement.query(By.css('.mat-select-trigger'))!.nativeElement.click();
fixture.detectChanges();
options = Array.from(overlayContainerElement.querySelectorAll('mat-option'));
}));

it('should set the select value', fakeAsync(() => {
fixture.componentInstance.control.setValue('a');
fixture.detectChanges();
expect(fixture.componentInstance.select.value).toBe('a');
}));

it('should reset the control value', fakeAsync(() => {
fixture.componentInstance.control.setValue('a');
fixture.detectChanges();

options[0].click();
fixture.detectChanges();
flush();
expect(fixture.componentInstance.control.value).toBe(undefined);
}));

it('should reflect the value in the form control', fakeAsync(() => {
options[1].click();
fixture.detectChanges();
flush();
expect(fixture.componentInstance.select.value).toBe('a');
expect(fixture.componentInstance.control.value).toBe('a');
}));
});

describe('without Angular forms', () => {
beforeEach(async(() => configureMatSelectTestingModule([
BasicSelectWithoutForms,
Expand Down Expand Up @@ -3187,6 +3225,63 @@ describe('MatSelect', () => {
.toBeFalsy('Expected no value after tabbing away.');
}));

it('should emit once when a reset value is selected', fakeAsync(() => {
const fixture = TestBed.createComponent(BasicSelectWithoutForms);
const instance = fixture.componentInstance;
const spy = jasmine.createSpy('change spy');

instance.selectedFood = 'sandwich-2';
instance.foods[0].value = null;
fixture.detectChanges();

const subscription = instance.select.selectionChange.subscribe(spy);

fixture.debugElement.query(By.css('.mat-select-trigger')).nativeElement.click();
fixture.detectChanges();
flush();

(overlayContainerElement.querySelector('mat-option') as HTMLElement).click();
fixture.detectChanges();
flush();

expect(spy).toHaveBeenCalledTimes(1);

subscription.unsubscribe();
}));

it('should not emit the change event multiple times when a reset option is ' +
'selected twice in a row', fakeAsync(() => {
const fixture = TestBed.createComponent(BasicSelectWithoutForms);
const instance = fixture.componentInstance;
const spy = jasmine.createSpy('change spy');

instance.foods[0].value = null;
fixture.detectChanges();

const subscription = instance.select.selectionChange.subscribe(spy);

fixture.debugElement.query(By.css('.mat-select-trigger')).nativeElement.click();
fixture.detectChanges();
flush();

(overlayContainerElement.querySelector('mat-option') as HTMLElement).click();
fixture.detectChanges();
flush();

expect(spy).not.toHaveBeenCalled();

fixture.debugElement.query(By.css('.mat-select-trigger')).nativeElement.click();
fixture.detectChanges();
flush();

(overlayContainerElement.querySelector('mat-option') as HTMLElement).click();
fixture.detectChanges();
flush();

expect(spy).not.toHaveBeenCalled();

subscription.unsubscribe();
}));

});

Expand Down Expand Up @@ -5315,3 +5410,23 @@ class MultiSelectWithLotsOfOptions {
this.value = [];
}
}


@Component({
selector: 'basic-select-with-reset',
template: `
<mat-form-field>
<mat-select [formControl]="control">
<mat-option>Reset</mat-option>
<mat-option value="a">A</mat-option>
<mat-option value="b">B</mat-option>
<mat-option value="c">C</mat-option>
</mat-select>
</mat-form-field>
`
})
class SelectWithResetOptionAndFormControl {
@ViewChild(MatSelect) select: MatSelect;
@ViewChildren(MatOption) options: QueryList<MatOption>;
control = new FormControl();
}
14 changes: 9 additions & 5 deletions src/material/select/select.ts
Expand Up @@ -442,7 +442,10 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit,
get value(): any { return this._value; }
set value(newValue: any) {
if (newValue !== this._value) {
this.writeValue(newValue);
if (this.options) {
this._setSelectionByValue(newValue);
}

this._value = newValue;
}
}
Expand Down Expand Up @@ -676,9 +679,7 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit,
* @param value New value to be written to the model.
*/
writeValue(value: any): void {
if (this.options) {
this._setSelectionByValue(value);
}
this.value = value;
}

/**
Expand Down Expand Up @@ -1003,7 +1004,10 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit,
if (option.value == null && !this._multiple) {
option.deselect();
this._selectionModel.clear();
this._propagateChanges(option.value);

if (this.value != null) {
this._propagateChanges(option.value);
}
} else {
if (wasSelected !== option.selected) {
option.selected ? this._selectionModel.select(option) :
Expand Down

0 comments on commit 77b11f4

Please sign in to comment.