Skip to content

Commit

Permalink
fix(selection-list): disabling list doesn't disable ripples of options
Browse files Browse the repository at this point in the history
* Fixes that dynamically changing the `[disabled]` binding does not update the state of the list options (e.g. ripple disabled state; checkbox disabled styling)

Fixes #9952
  • Loading branch information
devversion committed Jun 28, 2018
1 parent d96adbe commit 6fbd3e8
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 5 deletions.
21 changes: 20 additions & 1 deletion src/lib/list/selection-list.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
ViewChildren,
} from '@angular/core';
import {async, ComponentFixture, fakeAsync, TestBed, tick, flush} from '@angular/core/testing';
import {MatRipple} from '@angular/material/core';
import {By} from '@angular/platform-browser';
import {
MatListModule,
Expand Down Expand Up @@ -626,6 +627,23 @@ describe('MatSelectionList without forms', () => {

expect(selectList.selected.length).toBe(0);
});

it('should update state of options if list state has changed', () => {
// To verify that the template of the list options has been re-rendered after the disabled
// property of the selection list has been updated, the ripple directive can be used.
// Inspecting the host classes of the options doesn't work because those update as part
// of the parent template (of the selection-list).
const listOptionRipple = listOption[2].query(By.directive(MatRipple)).injector.get(MatRipple);

expect(listOptionRipple.disabled)
.toBe(true, 'Expected ripples of list option to be disabled');

fixture.componentInstance.disabled = false;
fixture.detectChanges();

expect(listOptionRipple.disabled)
.toBe(false, 'Expected ripples of list option to be enabled');
});
});

describe('with checkbox position after', () => {
Expand Down Expand Up @@ -977,7 +995,7 @@ class SelectionListWithCheckboxPositionAfter {
}

@Component({template: `
<mat-selection-list id="selection-list-3" [disabled]=true>
<mat-selection-list id="selection-list-3" [disabled]="disabled">
<mat-list-option checkboxPosition="after">
Inbox (disabled selection-option)
</mat-list-option>
Expand All @@ -992,6 +1010,7 @@ class SelectionListWithCheckboxPositionAfter {
</mat-list-option>
</mat-selection-list>`})
class SelectionListWithListDisabled {
disabled: boolean = true;
}

@Component({template: `
Expand Down
32 changes: 28 additions & 4 deletions src/lib/list/selection-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,9 @@ import {
ViewEncapsulation,
} from '@angular/core';
import {
CanDisable,
CanDisableRipple,
MatLine,
MatLineSetter,
mixinDisabled,
mixinDisableRipple,
} from '@angular/material/core';
import {ControlValueAccessor, NG_VALUE_ACCESSOR} from '@angular/forms';
Expand All @@ -43,7 +41,7 @@ import {Subscription} from 'rxjs';

/** @docs-private */
export class MatSelectionListBase {}
export const _MatSelectionListMixinBase = mixinDisableRipple(mixinDisabled(MatSelectionListBase));
export const _MatSelectionListMixinBase = mixinDisableRipple(MatSelectionListBase);

/** @docs-private */
export class MatListOptionBase {}
Expand Down Expand Up @@ -238,6 +236,15 @@ export class MatListOption extends _MatListOptionMixinBase
this._changeDetector.markForCheck();
return true;
}

/**
* Notifies Angular that the option needs to be checked in the next change detection run. Mainly
* used to trigger an update of the list option if the disabled state of the selection list
* changed.
*/
_markForCheck() {
this._changeDetector.markForCheck();
}
}


Expand Down Expand Up @@ -265,7 +272,7 @@ export class MatListOption extends _MatListOptionMixinBase
changeDetection: ChangeDetectionStrategy.OnPush
})
export class MatSelectionList extends _MatSelectionListMixinBase implements FocusableOption,
CanDisable, CanDisableRipple, AfterContentInit, ControlValueAccessor, OnDestroy {
CanDisableRipple, AfterContentInit, ControlValueAccessor, OnDestroy {

/** The FocusKeyManager which handles focus. */
_keyManager: FocusKeyManager<MatListOption>;
Expand All @@ -287,6 +294,22 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements Focu
*/
@Input() compareWith: (o1: any, o2: any) => boolean;

/** Whether the selection list is disabled. */
@Input()
get disabled(): boolean { return this._disabled; }
set disabled(value: boolean) {
this._disabled = coerceBooleanProperty(value);

// The `MatSelectionList` and `MatListOption` are using the `OnPush` change detection
// strategy. Therefore the options will not check for any changes if the `MatSelectionList`
// changed its state. Since we know that a change to `disabled` property of the list affects
// the state of the options, we manually mark each option for check.
if (this.options) {
this.options.forEach(option => option._markForCheck());
}
}
private _disabled: boolean = false;

/** The currently selected options. */
selectedOptions: SelectionModel<MatListOption> = new SelectionModel<MatListOption>(true);

Expand All @@ -296,6 +319,7 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements Focu
/** Used for storing the values that were assigned before the options were initialized. */
private _tempValues: string[]|null;

/** Subscription to sync value changes in the SelectionModel back to the SelectionList. */
private _modelChanges = Subscription.EMPTY;

/** View to model callback that should be called if the list or its options lost focus. */
Expand Down

0 comments on commit 6fbd3e8

Please sign in to comment.