Skip to content

Commit

Permalink
fix(material/list): Do not rely on input binding order (#17501)
Browse files Browse the repository at this point in the history
* fix(material/list): Do not rely on input binding order

This change ensures that setting `value` does not clear the `selected`
input until after the first change detection cycle. Ivy sets inputs
based on the order they appear in the templates.

Fixes #17500

* fixup! fix(material/list): Do not rely on input binding order
  • Loading branch information
atscott authored and mmalerba committed Oct 26, 2019
1 parent 0248ca9 commit 4301fb0
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 14 deletions.
19 changes: 19 additions & 0 deletions src/material/list/selection-list.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ describe('MatSelectionList without forms', () => {
SelectionListWithListDisabled,
SelectionListWithOnlyOneOption,
SelectionListWithIndirectChildOptions,
SelectionListWithSelectedOptionAndValue,
],
});

Expand Down Expand Up @@ -594,6 +595,14 @@ describe('MatSelectionList without forms', () => {
.toBe(0, 'Expected no ripples after list ripples are disabled.');
}));

it('can bind both selected and value at the same time', () => {
const componentFixture = TestBed.createComponent(SelectionListWithSelectedOptionAndValue);
componentFixture.detectChanges();
const listItemEl = componentFixture.debugElement.query(By.directive(MatListOption))!;
expect(listItemEl.componentInstance.selected).toBe(true);
expect(listItemEl.componentInstance.value).toBe(componentFixture.componentInstance.itemValue);
});

});

describe('with list option selected', () => {
Expand Down Expand Up @@ -1277,6 +1286,16 @@ class SelectionListWithDisabledOption {
class SelectionListWithSelectedOption {
}

@Component({
template: `
<mat-selection-list>
<mat-list-option [selected]="true" [value]="itemValue">Item</mat-list-option>
</mat-selection-list>`
})
class SelectionListWithSelectedOptionAndValue {
itemValue = 'item1';
}

@Component({template: `
<mat-selection-list id="selection-list-4">
<mat-list-option checkboxPosition="after" class="test-focus" id="123">
Expand Down
36 changes: 22 additions & 14 deletions src/material/list/selection-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ import {FocusableOption, FocusKeyManager} from '@angular/cdk/a11y';
import {coerceBooleanProperty} from '@angular/cdk/coercion';
import {SelectionModel} from '@angular/cdk/collections';
import {
SPACE,
A,
DOWN_ARROW,
END,
ENTER,
hasModifierKey,
HOME,
END,
SPACE,
UP_ARROW,
DOWN_ARROW,
A,
hasModifierKey,
} from '@angular/cdk/keycodes';
import {
AfterContentInit,
Expand All @@ -32,25 +32,27 @@ import {
forwardRef,
Inject,
Input,
OnChanges,
OnDestroy,
OnInit,
Output,
QueryList,
SimpleChanges,
ViewChild,
ViewEncapsulation,
SimpleChanges,
OnChanges,
} from '@angular/core';
import {ControlValueAccessor, NG_VALUE_ACCESSOR} from '@angular/forms';
import {
CanDisableRipple, CanDisableRippleCtor,
CanDisableRipple,
CanDisableRippleCtor,
MatLine,
setLines,
mixinDisableRipple,
setLines,
ThemePalette,
} from '@angular/material/core';
import {ControlValueAccessor, NG_VALUE_ACCESSOR} from '@angular/forms';
import {Subject} from 'rxjs';
import {takeUntil} from 'rxjs/operators';

import {MatListAvatarCssMatStyler, MatListIconCssMatStyler} from './list';


Expand Down Expand Up @@ -114,9 +116,9 @@ export class MatSelectionListChange {
encapsulation: ViewEncapsulation.None,
changeDetection: ChangeDetectionStrategy.OnPush,
})
export class MatListOption extends _MatListOptionMixinBase
implements AfterContentInit, OnDestroy, OnInit, FocusableOption, CanDisableRipple {

export class MatListOption extends _MatListOptionMixinBase implements AfterContentInit, OnDestroy,
OnInit, FocusableOption,
CanDisableRipple {
private _selected = false;
private _disabled = false;
private _hasFocus = false;
Expand All @@ -137,11 +139,16 @@ export class MatListOption extends _MatListOptionMixinBase
set color(newValue: ThemePalette) { this._color = newValue; }
private _color: ThemePalette;

/**
* This is set to true after the first OnChanges cycle so we don't clear the value of `selected`
* in the first cycle.
*/
private _inputsInitialized = false;
/** Value of the option */
@Input()
get value(): any { return this._value; }
set value(newValue: any) {
if (this.selected && newValue !== this.value) {
if (this.selected && newValue !== this.value && this._inputsInitialized) {
this.selected = false;
}

Expand Down Expand Up @@ -200,6 +207,7 @@ export class MatListOption extends _MatListOptionMixinBase
this._changeDetector.markForCheck();
}
});
this._inputsInitialized = true;
}

ngAfterContentInit() {
Expand Down

0 comments on commit 4301fb0

Please sign in to comment.