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(storefront): BCTHEME-1128 Form Input Error Message and Accessibility. #2235

Closed

Conversation

bc-yevhenii-buliuk
Copy link
Contributor

What?

This Pr fixes the accessibility of form fields. By adding the role="status" and aria-live="polite" attributes to the error messages, as well as the "aria-describedby" attribute associated with the "id", the screen reader is able to read form fields and error messages on those fields.
Also added the ability to set focus to the first field with an error when submitting an empty form.

Tickets / Documentation

BCTHEME-1128

Screen Recording

before fixes

1128_bugs.mov

after fixes

1128_fixed.mov

@huntario
Copy link
Collaborator

The focus on the latest element is a nice touch.

I've been looking into this and related issues on the PRs below. In my opinion, we don't seem to get much for the added complexity of 'announceInputErrorMessage' and we should work towards removing it. Assuming the HTML elements have appropriate Aria attributes, it should work without 'announceInputErrorMessage' which appears to not have been working for some time now (maybe never?). Is it ideal for code adding accessibility attributes to be tied to the form validation library?

That's the approach I've taken in these:
BCTHEME-1092 - #2230
BCTHEME-1093 - #2234
BCTHEME-1094 - #2233

I will of course defer to whatever you and your team wish to do. I'll watch this PR and adapt mine above based on how this one moves forward.

Whatever approach we take we'll want to also consider the methods within - assets/js/theme/account.js - related to the forms on the various logged in customer pages, but not necessarily in this PR.

@bc-yevhenii-buliuk
Copy link
Contributor Author

@huntario After discussing this task within the team, we decided to support the idea of ​​separating the form validation logic from the accessibility logic. Indeed, we can avoid using the "announceInputErrorMessage" function which adds attributes to improve accessibility during form validation and set the required accessibility attributes for input fields as implemented in your PR.
It would also be nice to implement logic to set focus to the first non-valid input field when submitting the form.
We expect that this approach will be implemented in all your other PRs that solve this problem.
Thus, I close this PR so as not to duplicate another solution. Thank you for discussing and paying attention to this issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants