Skip to content

Commit

Permalink
fix(material/input): only set as aria-invalid if the input isn't empty
Browse files Browse the repository at this point in the history 
Only sets `aria-invalid` on a `MatInput` if it is invalid and it has a value, otherwise it'll likely
overlap with `aria-required` and cause more noise for users. Furthermore, it may be
confusing if the user lands on an input that they haven't interacted with, but it's already
invalid.

Fixes angular#18140.
  • Loading branch information
@crisbeto
crisbeto committed Jan 15, 2021
1 parent 7348086 commit 42556d6
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 14 deletions.
33 changes: 28 additions & 5 deletions src/material-experimental/mdc-input/input.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -896,7 +896,7 @@ describe('MatMdcInput with forms', () => {
let fixture: ComponentFixture<MatInputWithFormErrorMessages>;
let testComponent: MatInputWithFormErrorMessages;
let containerEl: HTMLElement;
let inputEl: HTMLElement;
let inputEl: HTMLInputElement;

beforeEach(fakeAsync(() => {
fixture = createComponent(MatInputWithFormErrorMessages);
Expand All @@ -917,6 +917,7 @@ describe('MatMdcInput with forms', () => {
expect(testComponent.formControl.invalid).toBe(true, 'Expected form control to be invalid');
expect(containerEl.querySelectorAll('mat-error').length).toBe(0, 'Expected no error message');

inputEl.value = 'not valid';
testComponent.formControl.markAsTouched();
fixture.detectChanges();
flush();
Expand Down Expand Up @@ -949,6 +950,7 @@ describe('MatMdcInput with forms', () => {
expect(testComponent.formControl.invalid).toBe(true, 'Expected form control to be invalid');
expect(containerEl.querySelectorAll('mat-error').length).toBe(0, 'Expected no error message');

inputEl.value = 'not valid';
dispatchFakeEvent(fixture.debugElement.query(By.css('form'))!.nativeElement, 'submit');
fixture.detectChanges();
flush();
Expand Down Expand Up @@ -981,6 +983,7 @@ describe('MatMdcInput with forms', () => {
expect(component.formGroupDirective.submitted)
.toBe(false, 'Expected form not to have been submitted');

inputEl.value = 'not valid';
dispatchFakeEvent(groupFixture.debugElement.query(By.css('form'))!.nativeElement, 'submit');
groupFixture.detectChanges();
flush();
Expand All @@ -1007,7 +1010,7 @@ describe('MatMdcInput with forms', () => {
expect(containerEl.querySelectorAll('mat-hint').length)
.toBe(0, 'Expected no hints to be shown.');

testComponent.formControl.setValue('something');
testComponent.formControl.setValue('valid value');
fixture.detectChanges();
flush();

Expand Down Expand Up @@ -1059,6 +1062,26 @@ describe('MatMdcInput with forms', () => {
expect(errorIds).toBeTruthy('errors should be shown');
expect(describedBy).toBe(errorIds);
}));

it('should not set `aria-invalid` to true if the input is empty', fakeAsync(() => {
// Submit the form since it's the one that triggers the default error state matcher.
dispatchFakeEvent(fixture.nativeElement.querySelector('form'), 'submit');
fixture.detectChanges();
flush();

expect(testComponent.formControl.invalid).toBe(true, 'Expected form control to be invalid');
expect(inputEl.value).toBeFalsy();
expect(inputEl.getAttribute('aria-invalid'))
.toBe('false', 'Expected aria-invalid to be set to "false".');

inputEl.value = 'not valid';
fixture.detectChanges();

expect(testComponent.formControl.invalid).toBe(true, 'Expected form control to be invalid');
expect(inputEl.getAttribute('aria-invalid'))
.toBe('true', 'Expected aria-invalid to be set to "true".');
}));

});

describe('custom error behavior', () => {
Expand Down Expand Up @@ -1524,7 +1547,7 @@ class MatInputMissingMatInputTestController {}
})
class MatInputWithFormErrorMessages {
@ViewChild('form') form: NgForm;
formControl = new FormControl('', Validators.required);
formControl = new FormControl('', [Validators.required, Validators.pattern(/valid value/)]);
renderError = true;
}

Expand All @@ -1543,7 +1566,7 @@ class MatInputWithFormErrorMessages {
})
class MatInputWithCustomErrorStateMatcher {
formGroup = new FormGroup({
name: new FormControl('', Validators.required)
name: new FormControl('', [Validators.required, Validators.pattern(/valid value/)])
});

errorState = false;
Expand All @@ -1567,7 +1590,7 @@ class MatInputWithCustomErrorStateMatcher {
class MatInputWithFormGroupErrorMessages {
@ViewChild(FormGroupDirective) formGroupDirective: FormGroupDirective;
formGroup = new FormGroup({
name: new FormControl('', Validators.required)
name: new FormControl('', [Validators.required, Validators.pattern(/valid value/)])
});
}

Expand Down
6 changes: 4 additions & 2 deletions src/material-experimental/mdc-input/input.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,10 @@ import {MatInput as BaseMatInput} from '@angular/material/input';
'[required]': 'required',
'[attr.placeholder]': 'placeholder',
'[attr.readonly]': 'readonly && !_isNativeSelect || null',
'[attr.aria-invalid]': 'errorState',
'[attr.aria-required]': 'required.toString()',
// Only mark the input as invalid for assistive technology if it has a value since the
// state usually overlaps with `aria-required` when the input is empty and can be redundant.
'[attr.aria-invalid]': 'errorState && !empty',
'[attr.aria-required]': 'required',
},
providers: [{provide: MatFormFieldControl, useExisting: MatInput}],
})
Expand Down
33 changes: 28 additions & 5 deletions src/material/input/input.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1067,7 +1067,7 @@ describe('MatInput with forms', () => {
let fixture: ComponentFixture<MatInputWithFormErrorMessages>;
let testComponent: MatInputWithFormErrorMessages;
let containerEl: HTMLElement;
let inputEl: HTMLElement;
let inputEl: HTMLInputElement;

beforeEach(fakeAsync(() => {
fixture = createComponent(MatInputWithFormErrorMessages);
Expand All @@ -1088,6 +1088,7 @@ describe('MatInput with forms', () => {
expect(testComponent.formControl.invalid).toBe(true, 'Expected form control to be invalid');
expect(containerEl.querySelectorAll('mat-error').length).toBe(0, 'Expected no error message');

inputEl.value = 'not valid';
testComponent.formControl.markAsTouched();
fixture.detectChanges();
flush();
Expand All @@ -1105,6 +1106,7 @@ describe('MatInput with forms', () => {
expect(testComponent.formControl.invalid).toBe(true, 'Expected form control to be invalid');
expect(containerEl.querySelectorAll('mat-error').length).toBe(0, 'Expected no error message');

inputEl.value = 'not valid';
dispatchFakeEvent(fixture.debugElement.query(By.css('form'))!.nativeElement, 'submit');
fixture.detectChanges();
flush();
Expand Down Expand Up @@ -1137,6 +1139,7 @@ describe('MatInput with forms', () => {
expect(component.formGroupDirective.submitted)
.toBe(false, 'Expected form not to have been submitted');

inputEl.value = 'not valid';
dispatchFakeEvent(groupFixture.debugElement.query(By.css('form'))!.nativeElement, 'submit');
groupFixture.detectChanges();
flush();
Expand All @@ -1163,7 +1166,7 @@ describe('MatInput with forms', () => {
expect(containerEl.querySelectorAll('mat-hint').length)
.toBe(0, 'Expected no hints to be shown.');

testComponent.formControl.setValue('something');
testComponent.formControl.setValue('valid value');
fixture.detectChanges();
flush();

Expand Down Expand Up @@ -1215,6 +1218,26 @@ describe('MatInput with forms', () => {
expect(errorIds).toBeTruthy('errors should be shown');
expect(describedBy).toBe(errorIds);
}));

it('should not set `aria-invalid` to true if the input is empty', fakeAsync(() => {
// Submit the form since it's the one that triggers the default error state matcher.
dispatchFakeEvent(fixture.nativeElement.querySelector('form'), 'submit');
fixture.detectChanges();
flush();

expect(testComponent.formControl.invalid).toBe(true, 'Expected form control to be invalid');
expect(inputEl.value).toBeFalsy();
expect(inputEl.getAttribute('aria-invalid'))
.toBe('false', 'Expected aria-invalid to be set to "false".');

inputEl.value = 'not valid';
fixture.detectChanges();

expect(testComponent.formControl.invalid).toBe(true, 'Expected form control to be invalid');
expect(inputEl.getAttribute('aria-invalid'))
.toBe('true', 'Expected aria-invalid to be set to "true".');
}));

});

describe('custom error behavior', () => {
Expand Down Expand Up @@ -2005,7 +2028,7 @@ class MatInputMissingMatInputTestController {}
})
class MatInputWithFormErrorMessages {
@ViewChild('form') form: NgForm;
formControl = new FormControl('', Validators.required);
formControl = new FormControl('', [Validators.required, Validators.pattern(/valid value/)]);
renderError = true;
}

Expand All @@ -2024,7 +2047,7 @@ class MatInputWithFormErrorMessages {
})
class MatInputWithCustomErrorStateMatcher {
formGroup = new FormGroup({
name: new FormControl('', Validators.required)
name: new FormControl('', [Validators.required, Validators.pattern(/valid value/)])
});

errorState = false;
Expand All @@ -2048,7 +2071,7 @@ class MatInputWithCustomErrorStateMatcher {
class MatInputWithFormGroupErrorMessages {
@ViewChild(FormGroupDirective) formGroupDirective: FormGroupDirective;
formGroup = new FormGroup({
name: new FormControl('', Validators.required)
name: new FormControl('', [Validators.required, Validators.pattern(/valid value/)])
});
}

Expand Down
6 changes: 4 additions & 2 deletions src/material/input/input.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,10 @@ const _MatInputMixinBase: CanUpdateErrorStateCtor & typeof MatInputBase =
'[disabled]': 'disabled',
'[required]': 'required',
'[attr.readonly]': 'readonly && !_isNativeSelect || null',
'[attr.aria-invalid]': 'errorState',
'[attr.aria-required]': 'required.toString()',
// Only mark the input as invalid for assistive technology if it has a value since the
// state usually overlaps with `aria-required` when the input is empty and can be redundant.
'[attr.aria-invalid]': 'errorState && !empty',
'[attr.aria-required]': 'required',
},
providers: [{provide: MatFormFieldControl, useExisting: MatInput}],
})
Expand Down

0 comments on commit 42556d6

Please sign in to comment.