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

adds aria-{invalid,describedby} annotations for WTForms validation errors #6240

Merged
merged 2 commits into from
Mar 2, 2022

Conversation

cfm
Copy link
Member

@cfm cfm commented Jan 28, 2022

Status

Ready for review

Description of Changes

Closes #6238 (towards #5972) by adding aria-invalid and aria-describedby annotations where possible for validated WTForms-based forms. This is admittedly a pretty minor improvement; see #6238 for details on all the places where even this improvement is either not applicable or not possible in the current interfaces of the Source and Journalist Interfaces.

Testing

  1. Log in to the Journalist Interface as an administrator.
  2. Begin adding a user and enter invalid values for each field.
  3. Click Add User.
  4. When the form comes back with validation errors, view source and observe that each invalid field is marked aria-invalid and has an aria-describedby annotation pointing to its validation errors. (These annotations will also be used by assistive tech.)

Deployment

No deployment considerations.

Checklist

If you made changes to the server application code:

  • Linting (make lint) and tests (make test) pass in the development container

@cfm cfm changed the title adds ARIA-{INVALID,DESCRIBEDBY} annotations for WTForms validation errors adds aria-{invalid,describedby} annotations for WTForms validation errors Jan 28, 2022
@cfm cfm added the a11y Issues related to accessibility label Jan 28, 2022
@cfm cfm force-pushed the 6238-semantic-form-errors branch from c7eee30 to 777d7b6 Compare January 28, 2022 23:58
@cfm cfm marked this pull request as ready for review January 28, 2022 23:59
@cfm cfm requested a review from a team as a code owner January 28, 2022 23:59
@codecov-commenter
Copy link

codecov-commenter commented Jan 29, 2022

Codecov Report

Merging #6240 (d72e0dc) into develop (37f969e) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #6240   +/-   ##
========================================
  Coverage    84.09%   84.10%           
========================================
  Files           60       60           
  Lines         4220     4222    +2     
  Branches       509      509           
========================================
+ Hits          3549     3551    +2     
  Misses         550      550           
  Partials       121      121           
Impacted Files Coverage Δ
securedrop/journalist_app/__init__.py 90.17% <100.00%> (+0.08%) ⬆️
securedrop/source_app/__init__.py 90.62% <100.00%> (+0.09%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 37f969e...d72e0dc. Read the comment docs.

@cfm
Copy link
Member Author

cfm commented Feb 22, 2022

Thanks for reviewing, @legoktm. I'll revise this week.

@cfm cfm self-assigned this Mar 2, 2022
@cfm cfm force-pushed the 6238-semantic-form-errors branch from 777d7b6 to d72e0dc Compare March 2, 2022 19:07
@cfm
Copy link
Member Author

cfm commented Mar 2, 2022

Thanks for the review, @legoktm. Revised accordingly, including the suggested minor refactoring to Jinja's do expression.

@cfm cfm requested a review from legoktm March 2, 2022 19:35
@cfm cfm removed their assignment Mar 2, 2022
Copy link
Member

@legoktm legoktm left a comment

Choose a reason for hiding this comment

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

LGTM, test plan still checks out.

@legoktm legoktm merged commit 2236d3d into develop Mar 2, 2022
@legoktm legoktm deleted the 6238-semantic-form-errors branch March 2, 2022 22:43
@zenmonkeykstop zenmonkeykstop mentioned this pull request Mar 17, 2022
34 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a11y Issues related to accessibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

check that form errors are semantically linked to their triggering inputs (WCAG 3.3.1)
3 participants