From 42556d635eec7d3735b4f9596acea10af9192edb Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Fri, 15 Jan 2021 21:28:34 +0100 Subject: [PATCH] fix(material/input): only set as aria-invalid if the input isn't empty 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 #18140. --- .../mdc-input/input.spec.ts | 33 ++++++++++++++++--- src/material-experimental/mdc-input/input.ts | 6 ++-- src/material/input/input.spec.ts | 33 ++++++++++++++++--- src/material/input/input.ts | 6 ++-- 4 files changed, 64 insertions(+), 14 deletions(-) diff --git a/src/material-experimental/mdc-input/input.spec.ts b/src/material-experimental/mdc-input/input.spec.ts index 5e1509db874a..19749729ee05 100644 --- a/src/material-experimental/mdc-input/input.spec.ts +++ b/src/material-experimental/mdc-input/input.spec.ts @@ -896,7 +896,7 @@ describe('MatMdcInput with forms', () => { let fixture: ComponentFixture; let testComponent: MatInputWithFormErrorMessages; let containerEl: HTMLElement; - let inputEl: HTMLElement; + let inputEl: HTMLInputElement; beforeEach(fakeAsync(() => { fixture = createComponent(MatInputWithFormErrorMessages); @@ -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(); @@ -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(); @@ -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(); @@ -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(); @@ -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', () => { @@ -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; } @@ -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; @@ -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/)]) }); } diff --git a/src/material-experimental/mdc-input/input.ts b/src/material-experimental/mdc-input/input.ts index f2aedb13d52c..534491dbb4e6 100644 --- a/src/material-experimental/mdc-input/input.ts +++ b/src/material-experimental/mdc-input/input.ts @@ -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}], }) diff --git a/src/material/input/input.spec.ts b/src/material/input/input.spec.ts index 21b3970dcb36..6864c2012b34 100644 --- a/src/material/input/input.spec.ts +++ b/src/material/input/input.spec.ts @@ -1067,7 +1067,7 @@ describe('MatInput with forms', () => { let fixture: ComponentFixture; let testComponent: MatInputWithFormErrorMessages; let containerEl: HTMLElement; - let inputEl: HTMLElement; + let inputEl: HTMLInputElement; beforeEach(fakeAsync(() => { fixture = createComponent(MatInputWithFormErrorMessages); @@ -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(); @@ -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(); @@ -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(); @@ -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(); @@ -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', () => { @@ -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; } @@ -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; @@ -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/)]) }); } diff --git a/src/material/input/input.ts b/src/material/input/input.ts index c08560d26cd8..226294044ac3 100644 --- a/src/material/input/input.ts +++ b/src/material/input/input.ts @@ -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}], })