Skip to content

Commit 7d7a7ae

Browse files
committed
fix(input): placeholder being read out twice by some screen readers
Currently we hide the native placeholder using CSS while we mirror its value in the form field label, however because the value is still in the DOM, some screen readers will read it out twice: once for the label and once for the attribute. These changes remove the attribute from the DOM if it isn't supposed to be shown. Fixes #9721.
1 parent b823dbd commit 7d7a7ae

File tree

3 files changed

+30
-10
lines changed

3 files changed

+30
-10
lines changed

src/material/chips/chip-list.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ export class MatChipList extends _MatChipListMixinBase implements MatFormFieldCo
251251
* @docs-private
252252
*/
253253
get empty(): boolean {
254-
return (!this._chipInput || this._chipInput.empty) && this.chips.length === 0;
254+
return (!this._chipInput || this._chipInput.empty) && (!this.chips || this.chips.length === 0);
255255
}
256256

257257
/**

src/material/input/input.spec.ts

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -355,22 +355,31 @@ describe('MatInput without forms', () => {
355355
let fixture = createComponent(MatInputPlaceholderAttrTestComponent);
356356
fixture.detectChanges();
357357

358-
let inputEl = fixture.debugElement.query(By.css('input')).nativeElement;
359-
360358
expect(fixture.debugElement.query(By.css('label'))).toBeNull();
361-
expect(inputEl.placeholder).toBe('');
362359

363360
fixture.componentInstance.placeholder = 'Other placeholder';
364361
fixture.detectChanges();
365362

366363
let labelEl = fixture.debugElement.query(By.css('label'));
367364

368-
expect(inputEl.placeholder).toBe('Other placeholder');
369365
expect(labelEl).not.toBeNull();
370366
expect(labelEl.nativeElement.textContent).toMatch('Other placeholder');
371367
expect(labelEl.nativeElement.textContent).not.toMatch(/\*/g);
372368
}));
373369

370+
it('should not render the native placeholder when its value is mirrored in the label',
371+
fakeAsync(() => {
372+
const fixture = TestBed.createComponent(MatInputPlaceholderAttrTestComponent);
373+
fixture.componentInstance.placeholder = 'Enter a name';
374+
fixture.detectChanges();
375+
376+
const inputEl = fixture.debugElement.query(By.css('input')).nativeElement;
377+
const labelEl = fixture.debugElement.query(By.css('label'));
378+
379+
expect(inputEl.hasAttribute('placeholder')).toBe(false);
380+
expect(labelEl.nativeElement.textContent).toContain('Enter a name');
381+
}));
382+
374383
it('supports placeholder element', fakeAsync(() => {
375384
let fixture = createComponent(MatInputPlaceholderElementTestComponent);
376385
fixture.detectChanges();
@@ -895,7 +904,7 @@ describe('MatInput without forms', () => {
895904
expect(container.classList).toContain('mat-form-field-hide-placeholder');
896905
expect(container.classList).not.toContain('mat-form-field-should-float');
897906
expect(label.textContent.trim()).toBe('Label');
898-
expect(input.getAttribute('placeholder')).toBe('Placeholder');
907+
expect(input.hasAttribute('placeholder')).toBe(false);
899908

900909
input.value = 'Value';
901910
fixture.detectChanges();

src/material/input/input.ts

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import {
2929
ErrorStateMatcher,
3030
mixinErrorState,
3131
} from '@angular/material/core';
32-
import {MatFormFieldControl} from '@angular/material/form-field';
32+
import {MatFormFieldControl, MatFormField} from '@angular/material/form-field';
3333
import {Subject} from 'rxjs';
3434
import {getMatInputUnsupportedTypeError} from './input-errors';
3535
import {MAT_INPUT_VALUE_ACCESSOR} from './input-value-accessor';
@@ -76,7 +76,7 @@ const _MatInputMixinBase: CanUpdateErrorStateCtor & typeof MatInputBase =
7676
// Native input properties that are overwritten by Angular inputs need to be synced with
7777
// the native input element. Otherwise property bindings for those don't work.
7878
'[attr.id]': 'id',
79-
'[attr.placeholder]': 'placeholder',
79+
'[attr.placeholder]': '_getNativePlaceholderValue()',
8080
'[disabled]': 'disabled',
8181
'[required]': 'required',
8282
'[attr.readonly]': 'readonly && !_isNativeSelect || null',
@@ -231,8 +231,9 @@ export class MatInput extends _MatInputMixinBase implements MatFormFieldControl<
231231
_defaultErrorStateMatcher: ErrorStateMatcher,
232232
@Optional() @Self() @Inject(MAT_INPUT_VALUE_ACCESSOR) inputValueAccessor: any,
233233
private _autofillMonitor: AutofillMonitor,
234-
ngZone: NgZone) {
235-
234+
ngZone: NgZone,
235+
// @breaking-change 8.0.0 `_formField` parameter to be made required.
236+
@Optional() private _formField?: MatFormField) {
236237
super(_defaultErrorStateMatcher, _parentForm, _parentFormGroup, ngControl);
237238

238239
const element = this._elementRef.nativeElement;
@@ -332,6 +333,16 @@ export class MatInput extends _MatInputMixinBase implements MatFormFieldControl<
332333
// FormsModule or ReactiveFormsModule, because Angular forms also listens to input events.
333334
}
334335

336+
/** Determines the value of the native `placeholder` attribute that should be used in the DOM. */
337+
_getNativePlaceholderValue() {
338+
// If we're hiding the native placeholder, it should also be cleared from the DOM, otherwise
339+
// screen readers will read it out twice: once from the label and once from the attribute.
340+
// TODO: can be removed once we get rid of the `legacy` style for the form field, because it's
341+
// the only one that supports promoting the placeholder to a label.
342+
const formField = this._formField;
343+
return (!formField || !formField._hideControlPlaceholder()) ? this.placeholder : null;
344+
}
345+
335346
/** Does some manual dirty checking on the native input `value` property. */
336347
protected _dirtyCheckNativeValue() {
337348
const newValue = this._elementRef.nativeElement.value;

0 commit comments

Comments
 (0)