Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(material/form-field): preserve aria-describedby set externally across all form controls #30699

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions goldens/material/chips/index.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ export class MatChipGrid extends MatChipSet implements AfterContentInit, AfterVi
readonly controlType: string;
// (undocumented)
protected _defaultRole: string;
get describedByIds(): string[];
get disabled(): boolean;
set disabled(value: boolean);
get empty(): boolean;
Expand Down Expand Up @@ -250,6 +251,7 @@ export class MatChipInput implements MatChipTextControl, OnChanges, OnDestroy {
// (undocumented)
protected _chipGrid: MatChipGrid;
clear(): void;
get describedByIds(): string[];
get disabled(): boolean;
set disabled(value: boolean);
disabledInteractive: boolean;
Expand Down Expand Up @@ -514,6 +516,7 @@ export class MatChipsModule {

// @public
export interface MatChipTextControl {
readonly describedByIds?: string[];
empty: boolean;
focus(): void;
focused: boolean;
Expand Down
1 change: 1 addition & 0 deletions goldens/material/datepicker/index.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,7 @@ export class MatDateRangeInput<D> implements MatFormFieldControl<DateRange<D>>,
controlType: string;
get dateFilter(): DateFilterFn<D>;
set dateFilter(value: DateFilterFn<D>);
get describedByIds(): string[];
readonly disableAutomaticLabeling = true;
get disabled(): boolean;
set disabled(value: boolean);
Expand Down
1 change: 1 addition & 0 deletions goldens/material/form-field/index.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ export type MatFormFieldAppearance = 'fill' | 'outline';
export abstract class MatFormFieldControl<T> {
readonly autofilled?: boolean;
readonly controlType?: string;
readonly describedByIds?: string[];
readonly disableAutomaticLabeling?: boolean;
readonly disabled: boolean;
readonly empty: boolean;
Expand Down
1 change: 1 addition & 0 deletions goldens/material/input/index.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ export class MatInput implements MatFormFieldControl_2<any>, OnChanges, OnDestro
constructor(...args: unknown[]);
autofilled: boolean;
controlType: string;
get describedByIds(): string[];
protected _dirtyCheckNativeValue(): void;
get disabled(): boolean;
set disabled(value: BooleanInput);
Expand Down
1 change: 1 addition & 0 deletions goldens/material/select/index.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,7 @@ export class MatSelect implements AfterContentInit, OnChanges, OnDestroy, OnInit
customTrigger: MatSelectTrigger;
// (undocumented)
protected _defaultOptions: MatSelectConfig | null;
get describedByIds(): string[];
protected readonly _destroy: Subject<void>;
readonly disableAutomaticLabeling = true;
disabled: boolean;
Expand Down
8 changes: 8 additions & 0 deletions src/material/chips/chip-grid.ts
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,14 @@ export class MatChipGrid
this.stateChanges.next();
}

/**
* Implemented as part of MatFormFieldControl.
* @docs-private
*/
get describedByIds(): string[] {
return this._chipInput?.describedByIds || [];
}

/**
* Implemented as part of MatFormFieldControl.
* @docs-private
Expand Down
35 changes: 34 additions & 1 deletion src/material/chips/chip-input.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,38 @@ describe('MatChipInput', () => {
expect(inputNativeElement.classList).toContain('mat-mdc-chip-input');
expect(inputNativeElement.classList).toContain('mdc-text-field__input');
});

it('should set `aria-describedby` to the id of the mat-hint', () => {
expect(inputNativeElement.getAttribute('aria-describedby')).toBeNull();

fixture.componentInstance.hint = 'test';
fixture.changeDetectorRef.markForCheck();
fixture.detectChanges();
const hint = fixture.debugElement.query(By.css('mat-hint')).nativeElement;

expect(inputNativeElement.getAttribute('aria-describedby')).toBe(hint.getAttribute('id'));
expect(inputNativeElement.getAttribute('aria-describedby')).toMatch(/^mat-mdc-hint-\w+\d+$/);
});

it('should support user binding to `aria-describedby`', () => {
inputNativeElement.setAttribute('aria-describedby', 'test');
fixture.changeDetectorRef.markForCheck();
fixture.detectChanges();

expect(inputNativeElement.getAttribute('aria-describedby')).toBe('test');
});

it('should preserve aria-describedby set directly in the DOM', fakeAsync(() => {
inputNativeElement.setAttribute('aria-describedby', 'custom');
fixture.componentInstance.hint = 'test';
fixture.changeDetectorRef.markForCheck();
fixture.detectChanges();
const hint = fixture.debugElement.query(By.css('mat-hint')).nativeElement;

expect(inputNativeElement.getAttribute('aria-describedby')).toBe(
`${hint.getAttribute('id')} custom`,
);
}));
});

describe('[addOnBlur]', () => {
Expand Down Expand Up @@ -289,7 +321,7 @@ describe('MatChipInput', () => {

@Component({
template: `
<mat-form-field>
<mat-form-field [hintLabel]="hint">
<mat-chip-grid #chipGrid [required]="required">
<mat-chip-row>Hello</mat-chip-row>
<input
Expand All @@ -309,6 +341,7 @@ class TestChipInput {
placeholder = '';
required = false;
disabledInteractive = false;
hint: string;

add(_: MatChipInputEvent) {}
}
11 changes: 11 additions & 0 deletions src/material/chips/chip-input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,17 @@ export class MatChipInput implements MatChipTextControl, OnChanges, OnDestroy {
this.inputElement.value = '';
}

/**
* Implemented as part of MatChipTextControl.
* @docs-private
*/
get describedByIds(): string[] {
const element = this._elementRef.nativeElement;
const existingDescribedBy = element.getAttribute('aria-describedby');

return existingDescribedBy?.split(' ') || [];
}

setDescribedByIds(ids: string[]): void {
const element = this._elementRef.nativeElement;

Expand Down
3 changes: 3 additions & 0 deletions src/material/chips/chip-text-control.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ export interface MatChipTextControl {
/** Focuses the text control. */
focus(): void;

/** Gets the list of ids the input is described by. */
readonly describedByIds?: string[];

/** Sets the list of ids the input is described by. */
setDescribedByIds(ids: string[]): void;
}
14 changes: 13 additions & 1 deletion src/material/datepicker/date-range-input.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ describe('MatDateRangeInput', () => {
expect(rangeInput.getAttribute('aria-labelledby')).toBe(labelId);
});

it('should point the range input aria-labelledby to the form field hint element', () => {
it('should point the range input aria-describedby to the form field hint element', () => {
const fixture = createComponent(StandardRangePicker);
fixture.detectChanges();
const labelId = fixture.nativeElement.querySelector('.mat-mdc-form-field-hint').id;
Expand All @@ -179,6 +179,18 @@ describe('MatDateRangeInput', () => {
expect(rangeInput.getAttribute('aria-describedby')).toBe(labelId);
});

it('should preserve aria-describedby set directly in the DOM', fakeAsync(() => {
const fixture = createComponent(StandardRangePicker);
const rangeInput = fixture.nativeElement.querySelector('.mat-date-range-input');

rangeInput.setAttribute('aria-describedby', 'custom');
fixture.changeDetectorRef.markForCheck();
fixture.detectChanges();
const hint = fixture.nativeElement.querySelector('.mat-mdc-form-field-hint');

expect(rangeInput.getAttribute('aria-describedby')).toBe(`${hint.getAttribute('id')} custom`);
}));

it('should not set aria-labelledby if the form field does not have a label', () => {
const fixture = createComponent(RangePickerNoLabel);
fixture.detectChanges();
Expand Down
11 changes: 11 additions & 0 deletions src/material/datepicker/date-range-input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,17 @@ export class MatDateRangeInput<D>
this.ngControl = inject(ControlContainer, {optional: true, self: true}) as any;
}

/**
* Implemented as part of MatFormFieldControl.
* @docs-private
*/
get describedByIds(): string[] {
const element = this._elementRef.nativeElement;
const existingDescribedBy = element.getAttribute('aria-describedby');

return existingDescribedBy?.split(' ') || [];
}

/**
* Implemented as a part of `MatFormFieldControl`.
* @docs-private
Expand Down
3 changes: 3 additions & 0 deletions src/material/form-field/form-field-control.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ export abstract class MatFormFieldControl<T> {
*/
readonly disableAutomaticLabeling?: boolean;

/** Gets the list of element IDs that currently describe this control. */
readonly describedByIds?: string[];

/** Sets the list of element IDs that currently describe this control. */
abstract setDescribedByIds(ids: string[]): void;

Expand Down
20 changes: 19 additions & 1 deletion src/material/form-field/form-field.ts
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,9 @@ export class MatFormField
// Unique id for the hint label.
readonly _hintLabelId = this._idGenerator.getId('mat-mdc-hint-');

// Ids obtained from the error and hint fields
private _describedByIds: string[] | undefined;

/** Gets the current form field control */
get _control(): MatFormFieldControl<any> {
return this._explicitFormFieldControl || this._formFieldControl;
Expand Down Expand Up @@ -717,7 +720,22 @@ export class MatFormField
ids.push(...this._errorChildren.map(error => error.id));
}

this._control.setDescribedByIds(ids);
const existingDescribedBy = this._control.describedByIds;
let toAssign: string[];

// In some cases there might be some `aria-describedby` IDs that were assigned directly,
// like by the `AriaDescriber` (see #30011). Attempt to preserve them by taking the previous
// attribute value and filtering out the IDs that came from the previous `setDescribedByIds`
// call. Note the `|| ids` here allows us to avoid duplicating IDs on the first render.
if (existingDescribedBy) {
const exclude = this._describedByIds || ids;
toAssign = ids.concat(existingDescribedBy.filter(id => id && !exclude.includes(id)));
} else {
toAssign = ids;
}

this._control.setDescribedByIds(toAssign);
this._describedByIds = ids;
}
}

Expand Down
33 changes: 12 additions & 21 deletions src/material/input/input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,6 @@ export class MatInput
private _cleanupIosKeyup: (() => void) | undefined;
private _cleanupWebkitWheel: (() => void) | undefined;

/** `aria-describedby` IDs assigned by the form field. */
private _formFieldDescribedBy: string[] | undefined;

/** Whether the component is being rendered on the server. */
readonly _isServer: boolean;

Expand Down Expand Up @@ -554,28 +551,22 @@ export class MatInput
* Implemented as part of MatFormFieldControl.
* @docs-private
*/
setDescribedByIds(ids: string[]) {
get describedByIds(): string[] {
const element = this._elementRef.nativeElement;
const existingDescribedBy = element.getAttribute('aria-describedby');
let toAssign: string[];

// In some cases there might be some `aria-describedby` IDs that were assigned directly,
// like by the `AriaDescriber` (see #30011). Attempt to preserve them by taking the previous
// attribute value and filtering out the IDs that came from the previous `setDescribedByIds`
// call. Note the `|| ids` here allows us to avoid duplicating IDs on the first render.
if (existingDescribedBy) {
const exclude = this._formFieldDescribedBy || ids;
toAssign = ids.concat(
existingDescribedBy.split(' ').filter(id => id && !exclude.includes(id)),
);
} else {
toAssign = ids;
}

this._formFieldDescribedBy = ids;
return existingDescribedBy?.split(' ') || [];
}

/**
* Implemented as part of MatFormFieldControl.
* @docs-private
*/
setDescribedByIds(ids: string[]) {
const element = this._elementRef.nativeElement;

if (toAssign.length) {
element.setAttribute('aria-describedby', toAssign.join(' '));
if (ids.length) {
element.setAttribute('aria-describedby', ids.join(' '));
} else {
element.removeAttribute('aria-describedby');
}
Expand Down
10 changes: 10 additions & 0 deletions src/material/select/select.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,16 @@ describe('MatSelect', () => {
expect(select.getAttribute('aria-describedby')).toBe('test');
});

it('should preserve aria-describedby set directly in the DOM', fakeAsync(() => {
select.setAttribute('aria-describedby', 'custom');
fixture.componentInstance.hint = 'test';
fixture.changeDetectorRef.markForCheck();
fixture.detectChanges();
const hint = fixture.debugElement.query(By.css('mat-hint')).nativeElement;

expect(select.getAttribute('aria-describedby')).toBe(`${hint.getAttribute('id')} custom`);
}));

it('should be able to override the tabindex', () => {
fixture.componentInstance.tabIndexOverride = 3;
fixture.changeDetectorRef.markForCheck();
Expand Down
11 changes: 11 additions & 0 deletions src/material/select/select.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1449,6 +1449,17 @@ export class MatSelect
return value;
}

/**
* Implemented as part of MatFormFieldControl.
* @docs-private
*/
get describedByIds(): string[] {
const element = this._elementRef.nativeElement;
const existingDescribedBy = element.getAttribute('aria-describedby');

return existingDescribedBy?.split(' ') || [];
}

/**
* Implemented as part of MatFormFieldControl.
* @docs-private
Expand Down
Loading