diff --git a/app/components/memorable_date_component.html.erb b/app/components/memorable_date_component.html.erb index 8d9db6cc2f8..b9251c68619 100644 --- a/app/components/memorable_date_component.html.erb +++ b/app/components/memorable_date_component.html.erb @@ -15,7 +15,7 @@ }, false, ) %> - + <%= f.simple_fields_for name do |p| %> <%= p.input( :month, @@ -33,7 +33,6 @@ maxLength: 2, aria: { invalid: false, - describedby: "validated-field-error-#{unique_id}", labelledby: [ "memorable-date-month-label-#{unique_id}", "memorable-date-month-hint-#{unique_id}", @@ -41,7 +40,7 @@ }, value: month, }, - error: { id: "validated-field-error-#{unique_id}" }, + error: false, required: required, ) %> <% end %> @@ -49,7 +48,7 @@ <%= t('in_person_proofing.form.state_id.date_hint.month') %> - + <%= f.simple_fields_for name do |p| %> <%= p.input( :day, @@ -67,7 +66,6 @@ maxLength: 2, aria: { invalid: false, - describedby: "validated-field-error-#{unique_id}", labelledby: [ "memorable-date-day-label-#{unique_id}", "memorable-date-day-hint-#{unique_id}", @@ -75,7 +73,7 @@ }, value: day, }, - error: { id: "validated-field-error-#{unique_id}" }, + error: false, required: required, ) %> <% end %> @@ -83,7 +81,7 @@ <%= t('in_person_proofing.form.state_id.date_hint.day') %> - + <%= f.simple_fields_for name do |p| %> <%= p.input( :year, @@ -101,7 +99,6 @@ maxLength: 4, aria: { invalid: false, - describedby: "validated-field-error-#{unique_id}", labelledby: [ "memorable-date-year-label-#{unique_id}", "memorable-date-year-hint-#{unique_id}", @@ -109,7 +106,7 @@ }, value: year, }, - error: { id: "validated-field-error-#{unique_id}" }, + error: false, required: required, ) %> <% end %> @@ -119,4 +116,4 @@ <% end -%> - + diff --git a/app/components/validated_field_component.html.erb b/app/components/validated_field_component.html.erb index b685769e86a..88a77b187e1 100644 --- a/app/components/validated_field_component.html.erb +++ b/app/components/validated_field_component.html.erb @@ -1,4 +1,4 @@ - +<%= content_tag(:'lg-validated-field', 'error-id': error_id) do %> <%= content_tag( :script, error_messages.to_json, @@ -18,17 +18,11 @@ class: [*tag_options.dig(:input_html, :class), 'validated-field__input'], aria: { invalid: false, - describedby: [ - *tag_options.dig(:input_html, :aria, :describedby), - "validated-field-error-#{unique_id}", - "validated-field-hint-#{unique_id}", - ], + describedby: aria_describedby_idrefs, }, }, - hint_html: { - id: "validated-field-hint-#{unique_id}", - }, - error_html: { id: "validated-field-error-#{unique_id}" }, + hint_html: { id: hint_id }, + error_html: { id: error_id }, ), ) %> - +<% end %> diff --git a/app/components/validated_field_component.rb b/app/components/validated_field_component.rb index a6abb1a9795..53eef78a625 100644 --- a/app/components/validated_field_component.rb +++ b/app/components/validated_field_component.rb @@ -19,8 +19,31 @@ def error_messages }.compact end + def aria_describedby_idrefs + idrefs = [*tag_options.dig(:input_html, :aria, :describedby)] + idrefs << error_id if has_errors? + idrefs << hint_id if has_hint? + idrefs + end + private + def has_errors? + form.object.respond_to?(:errors) && form.object.errors.key?(name) + end + + def has_hint? + tag_options.key?(:hint) + end + + def error_id + "validated-field-error-#{unique_id}" + end + + def hint_id + "validated-field-hint-#{unique_id}" + end + def value_missing_error_message case input_type when :boolean diff --git a/app/javascript/packages/memorable-date/index.spec.ts b/app/javascript/packages/memorable-date/index.spec.ts index 9710df4eea4..745cf1c31d9 100644 --- a/app/javascript/packages/memorable-date/index.spec.ts +++ b/app/javascript/packages/memorable-date/index.spec.ts @@ -58,11 +58,9 @@ describe('MemorableDateElement', () => { let submitButton; function expectErrorToEqual(text: string) { - if (errorMessageElement.style.display !== 'none') { - expect(errorMessageElement.textContent).to.equal(text); - } else { - expect('').to.equal(text); - } + // Improvement idea: Assert that the computed accessible description of the input includes text. + expect(errorMessageElement.textContent).to.equal(text); + expect(errorMessageElement.classList.contains('display-none')).to.equal(!text); } beforeEach(() => { @@ -72,41 +70,38 @@ describe('MemorableDateElement', () => {
This is an arbitrary element to click
- + - + - + - + `; @@ -233,9 +228,8 @@ describe('MemorableDateElement', () => { function itHidesValidationErrorsOnTyping() { it('hides validation errors on typing', async () => { const expectNoVisibleError = () => { - expect(errorMessageElement).to.satisfy( - (element: HTMLDivElement) => element.style.display === 'none' || !element.textContent, - ); + expect(errorMessageElement.classList.contains('display-none')).to.be.true(); + expect(errorMessageElement.textContent).to.be.empty(); expect(Array.from(monthInput.classList)).not.to.contain('usa-input--error'); expect(monthInput.getAttribute('aria-invalid')).to.equal('false'); expect(Array.from(dayInput.classList)).not.to.contain('usa-input--error'); @@ -245,7 +239,7 @@ describe('MemorableDateElement', () => { }; const expectVisibleError = () => { - expect(errorMessageElement.style.display).not.to.equal('none'); + expect(errorMessageElement.classList.contains('display-none')).to.be.false(); expect(errorMessageElement.textContent).not.to.be.empty(); expect(Array.from(monthInput.classList)).to.contain('usa-input--error'); expect(monthInput.getAttribute('aria-invalid')).to.equal('true'); diff --git a/app/javascript/packages/validated-field/validated-field-element.spec.ts b/app/javascript/packages/validated-field/validated-field-element.spec.ts index 64638448e19..d22b9341cb1 100644 --- a/app/javascript/packages/validated-field/validated-field-element.spec.ts +++ b/app/javascript/packages/validated-field/validated-field-element.spec.ts @@ -1,6 +1,7 @@ import sinon from 'sinon'; import { getByRole, getByText } from '@testing-library/dom'; import userEvent from '@testing-library/user-event'; +import { computeAccessibleDescription } from 'dom-accessibility-api'; import './validated-field-element'; describe('ValidatedFieldElement', () => { @@ -8,10 +9,14 @@ describe('ValidatedFieldElement', () => { function createAndConnectElement({ hasInitialError = false, errorInsideField = true } = {}) { const element = document.createElement('lg-validated-field'); - const errorMessageId = ++idCounter; - const errorHtml = hasInitialError - ? `
Invalid value
` - : ''; + const errorMessageId = `validated-field-error-${++idCounter}`; + element.setAttribute('error-id', errorMessageId); + const errorHtml = + hasInitialError || !errorInsideField + ? `` + : ''; element.innerHTML = `
- + + Required Field { return element; } + it('does not have an error message by default', () => { + const element = createAndConnectElement(); + + expect(element.querySelector('.usa-error-message')).to.not.exist(); + }); + + it('does not have an error message while the value is valid', async () => { + const element = createAndConnectElement(); + + const input = getByRole(element, 'textbox'); + await userEvent.type(input, '5'); + + input.closest('form')!.checkValidity(); + + expect(element.querySelector('.usa-error-message')).to.not.exist(); + }); + it('shows error state and focuses on form validation', () => { const element = createAndConnectElement(); @@ -54,9 +77,10 @@ describe('ValidatedFieldElement', () => { expect(input.classList.contains('usa-input--error')).to.be.true(); expect(input.getAttribute('aria-invalid')).to.equal('true'); expect(document.activeElement).to.equal(input); - const message = getByText(element, 'This field is required'); - expect(message).to.be.ok(); - expect(message.id).to.equal('validated-field-error-1'); + expect(form.querySelector('.usa-error-message:not(.display-none)')).to.exist(); + expect(computeAccessibleDescription(document.activeElement!)).to.equal( + 'Required Field This field is required', + ); }); it('shows custom validity as message content', () => { @@ -84,7 +108,8 @@ describe('ValidatedFieldElement', () => { expect(input.classList.contains('usa-input--error')).to.be.false(); expect(input.getAttribute('aria-invalid')).to.equal('false'); - expect(getByText(element, 'This field is required').style.display).to.equal('none'); + expect(form.querySelector('.usa-error-message:not(.display-none)')).not.to.exist(); + expect(computeAccessibleDescription(document.activeElement!)).to.equal('Required Field'); }); it('focuses the first element with an error', () => { @@ -114,34 +139,57 @@ describe('ValidatedFieldElement', () => { expect(input.classList.contains('usa-input--error')).to.be.false(); expect(input.getAttribute('aria-invalid')).to.equal('false'); expect(() => getByText(element, 'Invalid value')).to.throw(); + expect(form.querySelector('.usa-error-message:not(.display-none)')).not.to.exist(); }); }); context('with error message element pre-rendered in the DOM', () => { it('reuses the error message element from inside the tag', () => { const element = createAndConnectElement({ hasInitialError: true, errorInsideField: true }); + const input = getByRole(element, 'textbox'); - expect(() => getByText(element, 'Invalid value')).not.to.throw(); - expect(() => getByText(element, 'This field is required')).to.throw(); + expect(computeAccessibleDescription(input)).to.equal('Required Field Invalid value'); const form = element.parentNode as HTMLFormElement; form.checkValidity(); + expect(computeAccessibleDescription(input)).to.equal('Required Field This field is required'); expect(() => getByText(element, 'Invalid value')).to.throw(); - expect(() => getByText(element, 'This field is required')).not.to.throw(); + expect(form.querySelector('.usa-error-message:not(.display-none)')).to.exist(); }); it('reuses the error message element from outside the tag', () => { const element = createAndConnectElement({ hasInitialError: true, errorInsideField: false }); + const input = getByRole(element, 'textbox'); const form = element.parentNode as HTMLFormElement; - expect(() => getByText(form, 'Invalid value')).not.to.throw(); - expect(() => getByText(form, 'This field is required')).to.throw(); + expect(computeAccessibleDescription(input)).to.equal('Required Field Invalid value'); form.checkValidity(); + expect(computeAccessibleDescription(input)).to.equal('Required Field This field is required'); expect(() => getByText(form, 'Invalid value')).to.throw(); - expect(() => getByText(form, 'This field is required')).not.to.throw(); + expect(form.querySelector('.usa-error-message:not(.display-none)')).to.exist(); + }); + + it('links input to external error message element when input is invalid', () => { + const element = createAndConnectElement({ hasInitialError: false, errorInsideField: false }); + const form = element.parentNode as HTMLFormElement; + + form.checkValidity(); + + const input = getByRole(element, 'textbox'); + expect(computeAccessibleDescription(input)).to.equal('Required Field This field is required'); + expect(form.querySelector('.usa-error-message:not(.display-none)')).to.exist(); + }); + + it('clears error message when field becomes valid', async () => { + const element = createAndConnectElement({ hasInitialError: true }); + const input = getByRole(element, 'textbox'); + await userEvent.type(input, '5'); + + expect(computeAccessibleDescription(input)).to.equal('Required Field'); + expect(element.querySelector('.usa-error-message:not(.display-none)')).not.to.exist(); }); }); diff --git a/app/javascript/packages/validated-field/validated-field-element.ts b/app/javascript/packages/validated-field/validated-field-element.ts index 7dc129b5d16..39b2713fa35 100644 --- a/app/javascript/packages/validated-field/validated-field-element.ts +++ b/app/javascript/packages/validated-field/validated-field-element.ts @@ -25,13 +25,10 @@ class ValidatedFieldElement extends HTMLElement { errorMessage: HTMLElement | null; - descriptorId?: string | null; - connectedCallback() { this.input = this.querySelector('.validated-field__input'); this.inputWrapper = this.querySelector('.validated-field__input-wrapper'); - this.descriptorId = this.input?.getAttribute('aria-describedby'); - this.errorMessage = this.getErrorElement(); + this.errorMessage = this.ownerDocument.getElementById(this.errorId); try { Object.assign( this.errorStrings, @@ -44,6 +41,14 @@ class ValidatedFieldElement extends HTMLElement { this.input?.addEventListener('invalid', (event) => this.toggleErrorMessage(event)); } + get errorId(): string { + return this.getAttribute('error-id')!; + } + + get descriptorIdRefs(): string[] { + return this?.input?.getAttribute('aria-describedby')?.split(' ').filter(Boolean) || []; + } + /** * Handles an invalid event, rendering or hiding an error message based on the input's current * validity. @@ -68,12 +73,12 @@ class ValidatedFieldElement extends HTMLElement { */ setErrorMessage(message?: string | null) { if (message) { - this.getOrCreateErrorMessageElement().textContent = message; - if (this.errorMessage?.style.display === 'none') { - this.errorMessage.style.display = ''; - } + this.getOrCreateErrorMessageElement(); + this.errorMessage!.textContent = message; + this.errorMessage!.classList.remove('display-none'); } else if (this.errorMessage) { - this.errorMessage.style.display = 'none'; + this.errorMessage.textContent = ''; + this.errorMessage.classList.add('display-none'); } } @@ -85,6 +90,12 @@ class ValidatedFieldElement extends HTMLElement { setInputIsValid(isValid: boolean) { this.input?.classList.toggle('usa-input--error', !isValid); this.input?.setAttribute('aria-invalid', String(!isValid)); + + const idRefs = this.descriptorIdRefs.filter((idRef) => idRef !== this.errorId); + if (!isValid) { + idRefs.push(this.errorId); + } + this.input?.setAttribute('aria-describedby', idRefs.join(' ')); } /** @@ -119,14 +130,8 @@ class ValidatedFieldElement extends HTMLElement { if (!this.errorMessage) { this.errorMessage = this.ownerDocument.createElement('div'); this.errorMessage.classList.add('usa-error-message'); + this.errorMessage.id = this.errorId; - const descriptorId = (this.descriptorId?.split(' ') || []).find( - (id) => document.getElementById(id) === null, - ); - - if (descriptorId) { - this.errorMessage.id = descriptorId; - } if (this.input && TEXT_LIKE_INPUT_TYPES.has(this.input.type)) { this.errorMessage.style.maxWidth = `${this.input.offsetWidth}px`; } @@ -137,17 +142,6 @@ class ValidatedFieldElement extends HTMLElement { return this.errorMessage; } - private getErrorElement(): HTMLElement | null { - const describedByElementIds = this.descriptorId?.split(' ') || []; - - return ( - describedByElementIds - .map((id) => document.getElementById(id)) - .find((element) => element?.classList.contains('usa-error-message')) || - this.querySelector('.usa-error-message') - ); - } - /** * Focus on this input if it's invalid and another error element * does not have focus. diff --git a/app/javascript/packages/validated-field/validated-field.spec.tsx b/app/javascript/packages/validated-field/validated-field.spec.tsx index ccc03b55a7e..70363e6a797 100644 --- a/app/javascript/packages/validated-field/validated-field.spec.tsx +++ b/app/javascript/packages/validated-field/validated-field.spec.tsx @@ -1,5 +1,6 @@ import sinon from 'sinon'; import { render } from '@testing-library/react'; +import { computeAccessibleDescription } from 'dom-accessibility-api'; import ValidatedField, { getErrorMessages } from './validated-field'; describe('getErrorMessages', () => { @@ -77,20 +78,18 @@ describe('ValidatedField', () => { it('is described by associated error message', () => { const validate = sinon.stub().throws(new Error('oops')); - const { getByRole, baseElement, rerender } = render(); + const { getByRole, rerender } = render(); const input = getByRole('textbox') as HTMLInputElement; input.reportValidity(); - const errorMessage = baseElement.querySelector(`#${input.getAttribute('aria-describedby')}`)!; - expect(errorMessage.classList.contains('usa-error-message')).to.be.true(); - expect(errorMessage.textContent).to.equal('oops'); + expect(computeAccessibleDescription(input)).to.equal('oops'); validate.resetBehavior(); rerender(); input.reportValidity(); - expect(errorMessage.textContent).to.equal('simple_form.required.text'); + expect(computeAccessibleDescription(input)).to.equal('simple_form.required.text'); }); it('merges classNames', () => { diff --git a/app/javascript/packages/validated-field/validated-field.tsx b/app/javascript/packages/validated-field/validated-field.tsx index 788d0042258..e9fa309598c 100644 --- a/app/javascript/packages/validated-field/validated-field.tsx +++ b/app/javascript/packages/validated-field/validated-field.tsx @@ -116,7 +116,7 @@ function ValidatedField( .join(' '); return ( - + @@ -124,7 +124,6 @@ function ValidatedField( {cloneElement(input, { ...inputProps, 'aria-invalid': false, - 'aria-describedby': errorId, className: inputClasses, })}
diff --git a/spec/components/memorable_date_component_spec.rb b/spec/components/memorable_date_component_spec.rb index 1cd67dd10a8..617ceb2d25c 100644 --- a/spec/components/memorable_date_component_spec.rb +++ b/spec/components/memorable_date_component_spec.rb @@ -214,15 +214,8 @@ end end - it 'renders aria-describedby to establish connection between input and error message' do - field = rendered.at_css('input') - - expect(field.attr('aria-describedby')).to start_with('validated-field-error-') - end - it 'renders a non-visible error message element' do - expect(rendered).to_not have_css('.usa-error-message', visible: true) - expect(rendered).to have_css('.usa-error-message', visible: false) + expect(rendered).to have_css('.usa-error-message.display-none') end context 'tag options are specified' do diff --git a/spec/components/validated_field_component_spec.rb b/spec/components/validated_field_component_spec.rb index 3ad4581a286..62f48697b2f 100644 --- a/spec/components/validated_field_component_spec.rb +++ b/spec/components/validated_field_component_spec.rb @@ -25,13 +25,46 @@ render_inline(described_class.new(**options)) end - it 'renders aria-describedby to establish connection between input and error message' do + it 'does not render aria-describedby by default' do field = rendered.at_css('input') - expect(field.attr('aria-describedby').split(' ')).to include( - start_with('validated-field-hint-'), - start_with('validated-field-error-'), - ) + expect(field.attr('aria-describedby')).to be_blank + end + + context 'when form has errors for field' do + let(:error_message) { 'Field is required' } + + before do + form_object.errors.add(name, error_message, type: :blank) + end + + it 'renders descriptive relationship to error message' do + field = rendered.at_css('input') + + expect(field).to have_description error_message + end + + context 'with aria tag option' do + let(:tag_options) { { input_html: { aria: { describedby: 'foo' } } } } + + it 'merges aria-describedby with the one applied by the field' do + field = rendered.at_css('input') + + expect(field.attr('aria-describedby')).to include('validated-field-error-') + expect(field.attr('aria-describedby')).to include('foo') + end + end + end + + context 'when hint is assigned to field' do + let(:hint) { 'Example: 123456' } + let(:tag_options) { { hint: 'Example: 123456' } } + + it 'renders descriptive relationship to hint' do + field = rendered.at_css('input') + + expect(field).to have_description hint + end end describe 'error message strings' do @@ -68,17 +101,4 @@ end end end - - context 'with tag options' do - context 'with aria tag option' do - let(:tag_options) { { input_html: { aria: { describedby: 'foo' } } } } - - it 'merges aria-describedby with the one applied by the field' do - field = rendered.at_css('input') - - expect(field.attr('aria-describedby')).to include('validated-field-error-') - expect(field.attr('aria-describedby')).to include('foo') - end - end - end end diff --git a/spec/features/phone/add_phone_spec.rb b/spec/features/phone/add_phone_spec.rb index 9dc59378730..a77c6022e1f 100644 --- a/spec/features/phone/add_phone_spec.rb +++ b/spec/features/phone/add_phone_spec.rb @@ -111,7 +111,7 @@ find('span', text: 'United States').click end expect(page).to have_content(t('two_factor_authentication.otp_delivery_preference.title')) - expect(page).to_not have_css('.usa-error-message') + expect(page).to have_css('.usa-error-message', text: '', visible: false) expect(hidden_select.value).to eq('US') click_continue expect(page.find(':focus')).to match_css('.phone-input__number')