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

Accessibility improvements - form fields #717

Merged

Conversation

robinmolen
Copy link
Contributor

@robinmolen robinmolen commented Oct 2, 2024

Closes open-formulieren/open-forms#4716

These are some accessibility changes regarding chapters 1.3.1 and 1.3.5 of the accessibility rapport

@robinmolen robinmolen linked an issue Oct 2, 2024 that may be closed by this pull request
3 tasks
@robinmolen robinmolen force-pushed the feature/4716-accessibility-improvements-form-fields branch from f90eb32 to 36b37e4 Compare October 2, 2024 14:54
Copy link

codecov bot commented Oct 2, 2024

Codecov Report

Attention: Patch coverage is 74.15730% with 23 lines in your changes missing coverage. Please review.

Project coverage is 56.82%. Comparing base (3619d68) to head (5d34ffa).
Report is 16 commits behind head on main.

Files with missing lines Patch % Lines
src/formio/components/Select.js 0.00% 5 Missing ⚠️
src/formio/components/Email.js 0.00% 4 Missing ⚠️
src/formio/components/Radio.js 0.00% 3 Missing ⚠️
src/formio/utils.js 85.71% 1 Missing and 1 partial ⚠️
src/components/forms/Checkbox/Checkbox.js 0.00% 0 Missing and 1 partial ⚠️
src/components/forms/DateField/DateField.js 0.00% 0 Missing and 1 partial ⚠️
src/components/forms/NumberField/NumberField.js 0.00% 0 Missing and 1 partial ⚠️
src/components/forms/RadioField/RadioField.js 0.00% 1 Missing ⚠️
src/components/forms/SelectField/SelectField.js 0.00% 0 Missing and 1 partial ⚠️
src/components/forms/TextField/TextField.js 0.00% 0 Missing and 1 partial ⚠️
... and 3 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #717      +/-   ##
==========================================
+ Coverage   55.21%   56.82%   +1.60%     
==========================================
  Files         229      230       +1     
  Lines        4167     4255      +88     
  Branches      747      772      +25     
==========================================
+ Hits         2301     2418     +117     
+ Misses       1660     1625      -35     
- Partials      206      212       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@sergei-maertens sergei-maertens left a comment

Choose a reason for hiding this comment

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

more review for the formio components will follow!

src/components/forms/Checkbox/Checkbox.js Outdated Show resolved Hide resolved
src/components/forms/EmailField/EmailField.js Outdated Show resolved Hide resolved
src/components/forms/RadioField/RadioField.js Outdated Show resolved Hide resolved
@sergei-maertens sergei-maertens marked this pull request as draft October 8, 2024 13:15
@robinmolen robinmolen marked this pull request as ready for review October 10, 2024 07:55
@robinmolen robinmolen marked this pull request as draft October 10, 2024 08:44
@robinmolen robinmolen marked this pull request as ready for review October 10, 2024 09:43
Copy link
Member

@sergei-maertens sergei-maertens left a comment

Choose a reason for hiding this comment

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

I'd like to see some (Jest) tests that assert the aria-describedby behaves as expected. See all the annotations about missing test coverage. I'm also very interested in how multiple: true components behave!

src/components/forms/ValidationErrors.js Outdated Show resolved Hide resolved
src/formio/utils.js Outdated Show resolved Hide resolved
src/formio/components/FileField.js Outdated Show resolved Hide resolved
src/formio/templates/file.ejs Show resolved Hide resolved
@robinmolen
Copy link
Contributor Author

robinmolen commented Oct 10, 2024

Formio doesn't validate inputs inside a component with the multiple property individually. Meaning: if we have a textfield component with the multiple property, where the first input is valid and the second invalid, we cannot determine in the code which input is the valid/invalid one. This means we cannot add things like aria-invalid="true" to a specific input inside the component (because, again, we don't know which input is invalid).

So to prevent some weird issues for screenreader users, we cannot make components with the multiple property accessible.

The accessibility change to link the error message (with aria-describedby) and to add aria-invalid="true" to the input won't happen for most components with the multiple property.

robinmolen added a commit that referenced this pull request Oct 10, 2024
@robinmolen robinmolen force-pushed the feature/4716-accessibility-improvements-form-fields branch from d582477 to 0bcf3f8 Compare October 10, 2024 14:35
src/formio/templates/file.ejs Outdated Show resolved Hide resolved
src/formio/templates/file.ejs Outdated Show resolved Hide resolved
src/formio/templates/file.ejs Outdated Show resolved Hide resolved
src/formio/utils.js Outdated Show resolved Hide resolved
@robinmolen robinmolen force-pushed the feature/4716-accessibility-improvements-form-fields branch 3 times, most recently from 35b90c8 to 5ce6b99 Compare October 17, 2024 07:59
src/formio/components/Checkbox.spec.js Outdated Show resolved Hide resolved
src/formio/components/Checkbox.spec.js Outdated Show resolved Hide resolved
src/formio/components/jest-util.js Outdated Show resolved Hide resolved
@robinmolen robinmolen force-pushed the feature/4716-accessibility-improvements-form-fields branch from 5ce6b99 to 2a2ddb6 Compare October 22, 2024 13:21
sergei-maertens pushed a commit that referenced this pull request Oct 23, 2024
@sergei-maertens sergei-maertens force-pushed the feature/4716-accessibility-improvements-form-fields branch from 2a2ddb6 to 2774e03 Compare October 23, 2024 20:05
Copy link
Member

@sergei-maertens sergei-maertens left a comment

Choose a reason for hiding this comment

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

Can be merged once #722 is merged!

…ld be undefined

It's better if this element hasn't got an id, then when multiple elements have the same id
- Replaced clickable icons with buttons
- Proper textual alternatives and translations
- Changed naming conventions for element ids
- Replaced <b> with css
- All icons now have 'aria-hidden="true"'
@sergei-maertens sergei-maertens force-pushed the feature/4716-accessibility-improvements-form-fields branch from 2774e03 to 5d34ffa Compare October 24, 2024 10:15
@sergei-maertens
Copy link
Member

The test failures are known issues and will be fixed via #723

@sergei-maertens sergei-maertens merged commit 65b168a into main Oct 24, 2024
11 of 15 checks passed
@sergei-maertens sergei-maertens deleted the feature/4716-accessibility-improvements-form-fields branch October 24, 2024 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Accessibility improvements - Form fields
2 participants