Skip to content

Commit

Permalink
fix(material/chips): chip set overwriting disabled state (#29795)
Browse files Browse the repository at this point in the history
The chip set has been set up in a way where it syncs its state to the chips, instead of the other way around which we follow in other components. This means that if its `disabled` state changes later, it can ovewrite the state that the user explicitly set on the chip.

These changes make the logic a bit more robust by writing to a different field.

Fixes #29783.
  • Loading branch information
crisbeto authored Sep 30, 2024
1 parent 030cdd6 commit 86ebb9b
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 8 deletions.
23 changes: 23 additions & 0 deletions src/material/chips/chip-listbox.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,15 @@ describe('MatChipListbox', () => {
expect(chipListboxNativeElement.hasAttribute('role')).toBe(false);
expect(chipListboxNativeElement.hasAttribute('aria-required')).toBe(false);
});

it('should toggle the chips disabled state based on whether it is disabled', fakeAsync(() => {
fixture.destroy();
TestBed.resetTestingModule();
const disabledFixture = createComponent(IndividuallyDisabledChipInsideForm);
disabledFixture.detectChanges();
flush();
expect(disabledFixture.componentInstance.chip.disabled).toBe(true);
}));
});

describe('with selected chips', () => {
Expand Down Expand Up @@ -1043,3 +1052,17 @@ class FalsyBasicChipListbox {
@ViewChild(MatChipListbox) chipListbox: MatChipListbox;
@ViewChildren(MatChipOption) chips: QueryList<MatChipOption>;
}

// Based on #29783.
@Component({
template: `
<form>
<mat-chip-listbox name="test" [ngModel]="null">
<mat-chip-option value="1" disabled>Hello</mat-chip-option>
</mat-chip-listbox>
</form>
`,
})
class IndividuallyDisabledChipInsideForm {
@ViewChild(MatChipOption) chip: MatChipOption;
}
10 changes: 4 additions & 6 deletions src/material/chips/chip-set.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,12 +158,10 @@ export class MatChipSet implements AfterViewInit, OnDestroy {

/** Syncs the chip-set's state with the individual chips. */
protected _syncChipsState() {
if (this._chips) {
this._chips.forEach(chip => {
chip.disabled = this._disabled;
chip._changeDetectorRef.markForCheck();
});
}
this._chips?.forEach(chip => {
chip._chipListDisabled = this._disabled;
chip._changeDetectorRef.markForCheck();
});
}

/** Dummy method for subclasses to override. Base chip set cannot be focused. */
Expand Down
11 changes: 10 additions & 1 deletion src/material/chips/chip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,9 @@ export class MatChip implements OnInit, AfterViewInit, AfterContentInit, DoCheck
/** Id of a span that contains this chip's aria description. */
_ariaDescriptionId = `${this.id}-aria-description`;

/** Whether the chip list is disabled. */
_chipListDisabled: boolean = false;

private _textElement!: HTMLElement;

/**
Expand Down Expand Up @@ -201,7 +204,13 @@ export class MatChip implements OnInit, AfterViewInit, AfterContentInit, DoCheck

/** Whether the chip is disabled. */
@Input({transform: booleanAttribute})
disabled: boolean = false;
get disabled(): boolean {
return this._disabled || this._chipListDisabled;
}
set disabled(value: boolean) {
this._disabled = value;
}
private _disabled = false;

/** Emitted when a chip is to be removed. */
@Output() readonly removed: EventEmitter<MatChipEvent> = new EventEmitter<MatChipEvent>();
Expand Down
4 changes: 3 additions & 1 deletion tools/public_api_guard/material/chips.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,11 @@ export class MatChip implements OnInit, AfterViewInit, AfterContentInit, DoCheck
protected basicChipAttrName: string;
// (undocumented)
_changeDetectorRef: ChangeDetectorRef;
_chipListDisabled: boolean;
color?: string | null;
readonly destroyed: EventEmitter<MatChipEvent>;
disabled: boolean;
get disabled(): boolean;
set disabled(value: boolean);
disableRipple: boolean;
// (undocumented)
protected _document: Document;
Expand Down

0 comments on commit 86ebb9b

Please sign in to comment.