Skip to content

Commit

Permalink
fix(chips): incorrectly handling disabled state (#12659)
Browse files Browse the repository at this point in the history
* Fixes the form field displaying as disabled, but the user still being able to interact with the chip list.
* Fixes the chip list still being focusable while it is disabled.
* Fixes the individual chips not being disabled when the list is disabled.
* Fixes the chip input not being disabled when the list is disabled.

Fixes #11089.
  • Loading branch information
crisbeto authored and jelbourn committed Aug 21, 2018
1 parent 9a59c2a commit 65ad6ab
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 3 deletions.
15 changes: 14 additions & 1 deletion src/lib/chips/chip-input.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@ import {Directionality} from '@angular/cdk/bidi';
import {ENTER, COMMA} from '@angular/cdk/keycodes';
import {PlatformModule} from '@angular/cdk/platform';
import {createKeyboardEvent} from '@angular/cdk/testing';
import {Component, DebugElement} from '@angular/core';
import {Component, DebugElement, ViewChild} from '@angular/core';
import {async, ComponentFixture, TestBed} from '@angular/core/testing';
import {By} from '@angular/platform-browser';
import {NoopAnimationsModule} from '@angular/platform-browser/animations';
import {MatFormFieldModule} from '@angular/material/form-field';
import {MatChipInput, MatChipInputEvent} from './chip-input';
import {MatChipsModule} from './index';
import {MAT_CHIPS_DEFAULT_OPTIONS, MatChipsDefaultOptions} from './chip-default-options';
import {MatChipList} from './chip-list';


describe('MatChipInput', () => {
Expand Down Expand Up @@ -82,6 +83,17 @@ describe('MatChipInput', () => {
expect(label.textContent).toContain('or don\'t');
});

it('should become disabled if the chip list is disabled', () => {
expect(inputNativeElement.hasAttribute('disabled')).toBe(false);
expect(chipInputDirective.disabled).toBe(false);

fixture.componentInstance.chipListInstance.disabled = true;
fixture.detectChanges();

expect(inputNativeElement.getAttribute('disabled')).toBe('true');
expect(chipInputDirective.disabled).toBe(true);
});

});

describe('[addOnBlur]', () => {
Expand Down Expand Up @@ -186,6 +198,7 @@ describe('MatChipInput', () => {
`
})
class TestChipInput {
@ViewChild(MatChipList) chipListInstance: MatChipList;
addOnBlur: boolean = false;
placeholder = '';

Expand Down
7 changes: 7 additions & 0 deletions src/lib/chips/chip-input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ let nextUniqueId = 0;
'(focus)': '_focus()',
'(input)': '_onInput()',
'[id]': 'id',
'[attr.disabled]': 'disabled || null',
'[attr.placeholder]': 'placeholder || null',
}
})
Expand Down Expand Up @@ -81,6 +82,12 @@ export class MatChipInput implements OnChanges {
/** Unique id for the input. */
@Input() id: string = `mat-chip-list-input-${nextUniqueId++}`;

/** Whether the input is disabled. */
@Input()
get disabled(): boolean { return this._disabled || (this._chipList && this._chipList.disabled); }
set disabled(value: boolean) { this._disabled = coerceBooleanProperty(value); }
private _disabled: boolean = false;

/** Whether the input is empty. */
get empty(): boolean { return !this._inputElement.value; }

Expand Down
35 changes: 35 additions & 0 deletions src/lib/chips/chip-list.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,20 @@ describe('MatChipList', () => {

expect(chipsValid).toBe(true);
});

it('should toggle the chips disabled state based on whether it is disabled', () => {
expect(chips.toArray().every(chip => chip.disabled)).toBe(false);

chipListInstance.disabled = true;
fixture.detectChanges();

expect(chips.toArray().every(chip => chip.disabled)).toBe(true);

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

expect(chips.toArray().every(chip => chip.disabled)).toBe(false);
});
});

describe('with selected chips', () => {
Expand Down Expand Up @@ -114,6 +128,27 @@ describe('MatChipList', () => {
expect(manager.activeItemIndex).toBe(lastIndex);
});

it('should be able to become focused when disabled', () => {
expect(chipListInstance.focused).toBe(false, 'Expected list to not be focused.');

chipListInstance.disabled = true;
fixture.detectChanges();

chipListInstance.focus();
fixture.detectChanges();

expect(chipListInstance.focused).toBe(false, 'Expected list to continue not to be focused');
});

it('should remove the tabindex from the list if it is disabled', () => {
expect(chipListNativeElement.getAttribute('tabindex')).toBeTruthy();

chipListInstance.disabled = true;
fixture.detectChanges();

expect(chipListNativeElement.hasAttribute('tabindex')).toBeFalsy();
});

describe('on chip destroy', () => {
it('should focus the next item', () => {
let array = chips.toArray();
Expand Down
15 changes: 13 additions & 2 deletions src/lib/chips/chip-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export class MatChipListChange {
template: `<div class="mat-chip-list-wrapper"><ng-content></ng-content></div>`,
exportAs: 'matChipList',
host: {
'[attr.tabindex]': '_tabIndex',
'[attr.tabindex]': 'disabled ? null : _tabIndex',
'[attr.aria-describedby]': '_ariaDescribedby || null',
'[attr.aria-required]': 'required.toString()',
'[attr.aria-disabled]': 'disabled.toString()',
Expand Down Expand Up @@ -261,7 +261,13 @@ export class MatChipList extends _MatChipListMixinBase implements MatFormFieldCo
*/
@Input()
get disabled(): boolean { return this.ngControl ? !!this.ngControl.disabled : this._disabled; }
set disabled(value: boolean) { this._disabled = coerceBooleanProperty(value); }
set disabled(value: boolean) {
this._disabled = coerceBooleanProperty(value);

if (this.chips) {
this.chips.forEach(chip => chip.disabled = this._disabled);
}
}
protected _disabled: boolean = false;

/** Orientation of the chip list. */
Expand All @@ -275,6 +281,7 @@ export class MatChipList extends _MatChipListMixinBase implements MatFormFieldCo
get selectable(): boolean { return this._selectable; }
set selectable(value: boolean) {
this._selectable = coerceBooleanProperty(value);

if (this.chips) {
this.chips.forEach(chip => chip.chipListSelectable = this._selectable);
}
Expand Down Expand Up @@ -441,6 +448,10 @@ export class MatChipList extends _MatChipListMixinBase implements MatFormFieldCo
* are no eligible chips.
*/
focus(): void {
if (this.disabled) {
return;
}

// TODO: ARIA says this should focus the first `selected` chip if any are selected.
// Focus on first element if there's no chipInput inside chip-list
if (this._chipInput && this._chipInput.focused) {
Expand Down

0 comments on commit 65ad6ab

Please sign in to comment.