Skip to content

fix(list): matching item not preselected if added after init #16080

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions src/material/list/selection-list.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1098,6 +1098,20 @@ describe('MatSelectionList with forms', () => {
expect(fixture.componentInstance.formControl.value).toEqual(['opt2']);
}));

it('should mark options added at a later point as selected', () => {
fixture.componentInstance.formControl.setValue(['opt4']);
fixture.detectChanges();

fixture.componentInstance.renderExtraOption = true;
fixture.detectChanges();

listOptions = fixture.debugElement.queryAll(By.directive(MatListOption))
.map(optionDebugEl => optionDebugEl.componentInstance);

expect(listOptions.length).toBe(4);
expect(listOptions[3].selected).toBe(true);
});

});

describe('preselected values', () => {
Expand Down Expand Up @@ -1285,12 +1299,14 @@ class SelectionListWithModel {
<mat-list-option value="opt1">Option 1</mat-list-option>
<mat-list-option value="opt2">Option 2</mat-list-option>
<mat-list-option value="opt3">Option 3</mat-list-option>
<mat-list-option value="opt4" *ngIf="renderExtraOption">Option 4</mat-list-option>
</mat-selection-list>
`
})
class SelectionListWithFormControl {
formControl = new FormControl();
renderList = true;
renderExtraOption = false;
}


Expand Down
57 changes: 31 additions & 26 deletions src/material/list/selection-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ import {
ThemePalette,
} from '@angular/material/core';
import {ControlValueAccessor, NG_VALUE_ACCESSOR} from '@angular/forms';
import {Subscription} from 'rxjs';
import {Subject} from 'rxjs';
import {takeUntil} from 'rxjs/operators';
import {MatListAvatarCssMatStyler, MatListIconCssMatStyler} from './list';


Expand Down Expand Up @@ -103,8 +104,8 @@ export class MatSelectionListChange {
// its theme. The accent theme palette is the default and doesn't need to be set.
'[class.mat-primary]': 'color === "primary"',
'[class.mat-warn]': 'color === "warn"',
'[attr.aria-selected]': 'selected.toString()',
'[attr.aria-disabled]': 'disabled.toString()',
'[attr.aria-selected]': 'selected',
'[attr.aria-disabled]': 'disabled',
},
templateUrl: 'list-option.html',
encapsulation: ViewEncapsulation.None,
Expand Down Expand Up @@ -177,13 +178,19 @@ export class MatListOption extends _MatListOptionMixinBase
}

ngOnInit() {
const list = this.selectionList;

if (list._value && list._value.some(value => list.compareWith(value, this._value))) {
this._setSelected(true);
}

const wasSelected = this._selected;

// List options that are selected at initialization can't be reported properly to the form
// control. This is because it takes some time until the selection-list knows about all
// available options. Also it can happen that the ControlValueAccessor has an initial value
// that should be used instead. Deferring the value change report to the next tick ensures
// that the form control value is not being overwritten.
const wasSelected = this._selected;

Promise.resolve().then(() => {
if (this._selected || wasSelected) {
this.selected = true;
Expand Down Expand Up @@ -337,7 +344,7 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements Focu
* options should appear as selected. The first argument is the value of an options. The second
* one is a value from the selected value. A boolean must be returned.
*/
@Input() compareWith: (o1: any, o2: any) => boolean;
@Input() compareWith: (o1: any, o2: any) => boolean = (a1, a2) => a1 === a2;

/** Whether the selection list is disabled. */
@Input()
Expand All @@ -359,17 +366,17 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements Focu
/** View to model callback that should be called whenever the selected options change. */
private _onChange: (value: any) => void = (_: any) => {};

/** Used for storing the values that were assigned before the options were initialized. */
private _tempValues: string[]|null;
/** Keeps track of the currently-selected value. */
_value: string[]|null;

/** Subscription to sync value changes in the SelectionModel back to the SelectionList. */
private _modelChanges = Subscription.EMPTY;
/** Emits when the list has been destroyed. */
private _destroyed = new Subject<void>();

/** View to model callback that should be called if the list or its options lost focus. */
_onTouched: () => void = () => {};

/** Whether the list has been destroyed. */
private _destroyed: boolean;
private _isDestroyed: boolean;

constructor(private _element: ElementRef<HTMLElement>, @Attribute('tabindex') tabIndex: string) {
super();
Expand All @@ -385,13 +392,12 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements Focu
.skipPredicate(() => false)
.withAllowedModifierKeys(['shiftKey']);

if (this._tempValues) {
this._setOptionsFromValues(this._tempValues);
this._tempValues = null;
if (this._value) {
this._setOptionsFromValues(this._value);
}

// Sync external changes to the model back to the options.
this._modelChanges = this.selectedOptions.onChange.subscribe(event => {
this.selectedOptions.onChange.pipe(takeUntil(this._destroyed)).subscribe(event => {
if (event.added) {
for (let item of event.added) {
item.selected = true;
Expand All @@ -417,8 +423,9 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements Focu
}

ngOnDestroy() {
this._destroyed = true;
this._modelChanges.unsubscribe();
this._destroyed.next();
this._destroyed.complete();
this._isDestroyed = true;
}

/** Focuses the selection list. */
Expand Down Expand Up @@ -504,8 +511,10 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements Focu
// Stop reporting value changes after the list has been destroyed. This avoids
// cases where the list might wrongly reset its value once it is removed, but
// the form control is still live.
if (this.options && !this._destroyed) {
this._onChange(this._getSelectedOptionValues());
if (this.options && !this._isDestroyed) {
const value = this._getSelectedOptionValues();
this._onChange(value);
this._value = value;
}
}

Expand All @@ -516,10 +525,10 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements Focu

/** Implemented as part of ControlValueAccessor. */
writeValue(values: string[]): void {
this._value = values;

if (this.options) {
this._setOptionsFromValues(values || []);
} else {
this._tempValues = values;
}
}

Expand All @@ -546,11 +555,7 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements Focu
const correspondingOption = this.options.find(option => {
// Skip options that are already in the model. This allows us to handle cases
// where the same primitive value is selected multiple times.
if (option.selected) {
return false;
}

return this.compareWith ? this.compareWith(option.value, value) : option.value === value;
return option.selected ? false : this.compareWith(option.value, value);
});

if (correspondingOption) {
Expand Down
1 change: 1 addition & 0 deletions tools/public_api_guard/material/list.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ export declare class MatNavList extends _MatListMixinBase implements CanDisableR
export declare class MatSelectionList extends _MatSelectionListMixinBase implements FocusableOption, CanDisableRipple, AfterContentInit, ControlValueAccessor, OnDestroy, OnChanges {
_keyManager: FocusKeyManager<MatListOption>;
_onTouched: () => void;
_value: string[] | null;
color: ThemePalette;
compareWith: (o1: any, o2: any) => boolean;
disabled: boolean;
Expand Down