Skip to content

Commit

Permalink
fix(material/select): option active styling not removed when value is…
Browse files Browse the repository at this point in the history
… changed programmatically (#20649)

If an option is selected by the user and the value is changed programmatically afterwards, we weren't removing the
active state from the previous option. This seems to be a regression from #19970.

These changes resolve the issue and preserve the fix from #19970 by resetting the
active styling manually.
  • Loading branch information
crisbeto committed Oct 2, 2020
1 parent 7beb952 commit c4078aa
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 2 deletions.
33 changes: 33 additions & 0 deletions src/material-experimental/mdc-select/select.spec.ts
Expand Up @@ -1098,6 +1098,39 @@ describe('MDC-based MatSelect', () => {
expect(options[1].getAttribute('aria-disabled')).toEqual('false');
expect(options[2].getAttribute('aria-disabled')).toEqual('false');
}));

it('should remove the active state from options that have been deselected while closed',
fakeAsync(() => {
let activeOptions = options.filter(option => {
return option.classList.contains('mat-mdc-option-active');
});
expect(activeOptions).toEqual([options[0]],
'Expected first option to have active styles.');

options[1].click();
fixture.detectChanges();
trigger.click();
fixture.detectChanges();
flush();

activeOptions = options.filter(option => {
return option.classList.contains('mat-mdc-option-active');
});
expect(activeOptions).toEqual([options[1]],
'Expected only selected option to be marked as active after it is clicked.');

fixture.componentInstance.control.setValue(fixture.componentInstance.foods[7].value);
fixture.detectChanges();
trigger.click();
fixture.detectChanges();
flush();

activeOptions = options.filter(option => {
return option.classList.contains('mat-mdc-option-active');
});
expect(activeOptions).toEqual([options[7]],
'Expected only selected option to be marked as active after the value has changed.');
}));
});

describe('for option groups', () => {
Expand Down
27 changes: 27 additions & 0 deletions src/material/select/select.spec.ts
Expand Up @@ -1066,6 +1066,33 @@ describe('MatSelect', () => {
expect(options[1].getAttribute('aria-disabled')).toEqual('false');
expect(options[2].getAttribute('aria-disabled')).toEqual('false');
}));

it('should remove the active state from options that have been deselected while closed',
fakeAsync(() => {
let activeOptions = options.filter(option => option.classList.contains('mat-active'));
expect(activeOptions).toEqual([options[0]],
'Expected first option to have active styles.');

options[1].click();
fixture.detectChanges();
trigger.click();
fixture.detectChanges();
flush();

activeOptions = options.filter(option => option.classList.contains('mat-active'));
expect(activeOptions).toEqual([options[1]],
'Expected only selected option to be marked as active after it is clicked.');

fixture.componentInstance.control.setValue(fixture.componentInstance.foods[7].value);
fixture.detectChanges();
trigger.click();
fixture.detectChanges();
flush();

activeOptions = options.filter(option => option.classList.contains('mat-active'));
expect(activeOptions).toEqual([options[7]],
'Expected only selected option to be marked as active after the value has changed.');
}));
});

describe('for option groups', () => {
Expand Down
5 changes: 3 additions & 2 deletions src/material/select/select.ts
Expand Up @@ -813,16 +813,17 @@ export abstract class _MatSelectBase<C> extends _MatSelectMixinBase implements A
* found with the designated value, the select trigger is cleared.
*/
private _setSelectionByValue(value: any | any[]): void {
this._selectionModel.selected.forEach(option => option.setInactiveStyles());
this._selectionModel.clear();

if (this.multiple && value) {
if (!Array.isArray(value) && (typeof ngDevMode === 'undefined' || ngDevMode)) {
throw getMatSelectNonArrayValueError();
}

this._selectionModel.clear();
value.forEach((currentValue: any) => this._selectValue(currentValue));
this._sortValues();
} else {
this._selectionModel.clear();
const correspondingOption = this._selectValue(value);

// Shift focus to the active item. Note that we shouldn't do this in multiple
Expand Down

0 comments on commit c4078aa

Please sign in to comment.