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

[SDPA-5684] Added a visually hidden list of all validation errors for screenreaders. #1078

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

elNico
Copy link
Contributor

@elNico elNico commented Oct 22, 2021

Motivation and Context

JIRA issue: https://digital-engagement.atlassian.net/browse/SDPA-5684

Changed

  1. Add all submission errors to a visually hidden list.
  2. Make sure the list gets read out by screenreaders with priority.
  3. Note: This only makes sense in combination with meaningful field validation messages (see screenshots), which are in [SDPA-5526] Added field labels to form validation required messages #1074

Screenshots

Without PR 1074
Screen Shot 2021-10-22 at 4 35 38 pm (2)

With PR 1074
Screen Shot 2021-10-22 at 3 33 02 pm (2)

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Improvement/refactoring (non-breaking change that doesn't add any features but makes things better)

Checklist

  • I've added relevant changes to the documentation.
  • I have added tests to cover my changes (if not applicable, please state why)
  • My change requires a template update for create-ripple-app.
  • I have added template update script for next release.

@elNico elNico requested a review from dylankelly October 22, 2021 06:46
@elNico elNico self-assigned this Oct 22, 2021
&__sr-only {
@include rpl_visually_hidden;
ul {
// Avoid screenreader reading out "bullet".
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this go in the rpl_visually_hidden mixin?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dylankelly I'm not really sure whether I understand. Can you explain it a bit more? I'm just using the standard rpl_visually_hidden mixin to visually hide the messages. You mean using the rpl-visually-hidden class instead? I wrapped it in the custom _sr-only just because it needed the extra UL treatment. I guess it could be reshuffled if that's what you mean.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah maybe i didnt think that through first. What I was probably intending was a list reset mixin that included this.

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