Skip to content

Commit

Permalink
fix(material/form-field): avoid touching the DOM on each state change
Browse files Browse the repository at this point in the history
Currently we set the `aria-describedby` every time the state of the form control changes. This is excessive, because it only needs to happen if the error state or `userAriaDescribedBy` change.
  • Loading branch information
crisbeto committed Nov 14, 2024
1 parent 5dc2de2 commit d62c236
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 4 deletions.
18 changes: 16 additions & 2 deletions src/material/form-field/form-field.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import {AbstractControlDirective} from '@angular/forms';
import {ThemePalette} from '@angular/material/core';
import {_IdGenerator} from '@angular/cdk/a11y';
import {Subject, Subscription, merge} from 'rxjs';
import {takeUntil} from 'rxjs/operators';
import {map, pairwise, takeUntil, filter, startWith} from 'rxjs/operators';
import {MAT_ERROR, MatError} from './directives/error';
import {
FLOATING_LABEL_PARENT,
Expand Down Expand Up @@ -328,6 +328,7 @@ export class MatFormField
private _previousControl: MatFormFieldControl<unknown> | null = null;
private _stateChanges: Subscription | undefined;
private _valueChanges: Subscription | undefined;
private _describedByChanges: Subscription | undefined;

private _injector = inject(Injector);

Expand Down Expand Up @@ -377,6 +378,7 @@ export class MatFormField
ngOnDestroy() {
this._stateChanges?.unsubscribe();
this._valueChanges?.unsubscribe();
this._describedByChanges?.unsubscribe();
this._destroyed.next();
this._destroyed.complete();
}
Expand Down Expand Up @@ -426,10 +428,22 @@ export class MatFormField
this._stateChanges?.unsubscribe();
this._stateChanges = control.stateChanges.subscribe(() => {
this._updateFocusState();
this._syncDescribedByIds();
this._changeDetectorRef.markForCheck();
});

// Updating the `aria-describedby` touches the DOM. Only do it if it actually needs to change.
this._describedByChanges?.unsubscribe();
this._describedByChanges = control.stateChanges
.pipe(
startWith([undefined, undefined] as const),
map(() => [control.errorState, control.userAriaDescribedBy] as const),
pairwise(),
filter(([[prevErrorState, prevDescribedBy], [currentErrorState, currentDescribedBy]]) => {
return prevErrorState !== currentErrorState || prevDescribedBy !== currentDescribedBy;
}),
)
.subscribe(() => this._syncDescribedByIds());

this._valueChanges?.unsubscribe();

// Run change detection if the value changes.
Expand Down
6 changes: 4 additions & 2 deletions src/material/input/input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -551,10 +551,12 @@ export class MatInput
* @docs-private
*/
setDescribedByIds(ids: string[]) {
const element = this._elementRef.nativeElement;

if (ids.length) {
this._elementRef.nativeElement.setAttribute('aria-describedby', ids.join(' '));
element.setAttribute('aria-describedby', ids.join(' '));
} else {
this._elementRef.nativeElement.removeAttribute('aria-describedby');
element.removeAttribute('aria-describedby');
}
}

Expand Down

0 comments on commit d62c236

Please sign in to comment.