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/input): only set as aria-invalid if the input isn't empty #21609

Merged
merged 1 commit into from
Feb 10, 2021
Merged
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
33 changes: 28 additions & 5 deletions src/material-experimental/mdc-input/input.spec.ts
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
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',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to clarify- will errorState be true when the control is touched (not pristine)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The errorState is set based on the ErrorStateMatcher. The default one will be true if the input is invalid and touched or the form has been submitted.

'[attr.aria-required]': 'required',
},
providers: [{provide: MatFormFieldControl, useExisting: MatInput}],
})
Expand Down
33 changes: 28 additions & 5 deletions src/material/input/input.spec.ts
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
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