From 5f68dd8dbdeac85a646b8318be193aa8aed94c22 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Sat, 11 May 2019 16:12:57 -0400 Subject: [PATCH] fix(list): form control cleared when list is destroyed Currently we remove values from the selection model if the associated list option is destroyed, however that means that when the entire list is destroyed, the form control would be cleared. In most cases this is fine since it's very likely that the entire view was destroyed, however it becomes problematic when it's only the list being destroyed. These changes add an extra check to ensure that we're not clearing the model on destroy. Fixes #15994. --- src/material/list/selection-list.spec.ts | 20 +++++++++++++++++++- src/material/list/selection-list.ts | 13 +++++++++++-- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/src/material/list/selection-list.spec.ts b/src/material/list/selection-list.spec.ts index ab54217289ff..13c4e1e271cb 100644 --- a/src/material/list/selection-list.spec.ts +++ b/src/material/list/selection-list.spec.ts @@ -1081,6 +1081,23 @@ describe('MatSelectionList with forms', () => { expect(listOptions[1].selected).toBe(true, 'Expected second option to be selected.'); expect(listOptions[2].selected).toBe(true, 'Expected third option to be selected.'); }); + + it('should not clear the form control when the list is destroyed', fakeAsync(() => { + const option = listOptions[1]; + + option.selected = true; + fixture.detectChanges(); + + expect(fixture.componentInstance.formControl.value).toEqual(['opt2']); + + fixture.componentInstance.renderList = false; + fixture.detectChanges(); + tick(); + fixture.detectChanges(); + + expect(fixture.componentInstance.formControl.value).toEqual(['opt2']); + })); + }); describe('preselected values', () => { @@ -1264,7 +1281,7 @@ class SelectionListWithModel { @Component({ template: ` - + Option 1 Option 2 Option 3 @@ -1273,6 +1290,7 @@ class SelectionListWithModel { }) class SelectionListWithFormControl { formControl = new FormControl(); + renderList = true; } diff --git a/src/material/list/selection-list.ts b/src/material/list/selection-list.ts index c126e4c7eb32..3074ec73fa79 100644 --- a/src/material/list/selection-list.ts +++ b/src/material/list/selection-list.ts @@ -200,7 +200,9 @@ export class MatListOption extends _MatListOptionMixinBase if (this.selected) { // We have to delay this until the next tick in order // to avoid changed after checked errors. - Promise.resolve().then(() => this.selected = false); + Promise.resolve().then(() => { + this.selected = false; + }); } const hadFocus = this._hasFocus; @@ -366,6 +368,9 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements Focu /** 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; + constructor(private _element: ElementRef, @Attribute('tabindex') tabIndex: string) { super(); this.tabIndex = parseInt(tabIndex) || 0; @@ -412,6 +417,7 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements Focu } ngOnDestroy() { + this._destroyed = true; this._modelChanges.unsubscribe(); } @@ -495,7 +501,10 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements Focu /** Reports a value change to the ControlValueAccessor */ _reportValueChange() { - if (this.options) { + // 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()); } }