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

[SCREENREADER]: Month, Day, Year combo box pattern not announcing LEGEND to assistive devices #10142

Merged
merged 4 commits into from
Jun 13, 2019

Conversation

accbjt
Copy link
Contributor

@accbjt accbjt commented May 29, 2019

Description

Original: usds/us-forms-system#265

Mentioned: https://github.com/department-of-veterans-affairs/vets.gov-team/issues/15740

Testing done

Local Testing

Screenshots

Acceptance criteria

  • Screen Reader should speak the legend and date and time in the form

Definition of done

  • Events are logged appropriately
  • Documentation has been updated, if applicable
  • A link has been provided to the originating GitHub issue (or connected to it via ZenHub)
  • No sensitive information (i.e. PII/credentials/internal URLs/etc.) is captured in logging, hardcoded, or specs

@va-bot va-bot temporarily deployed to vetsgov-pr-10142 May 29, 2019 22:52 Inactive
@accbjt
Copy link
Contributor Author

accbjt commented May 29, 2019

@1Copenut I tried to test it on on my Mac but could not get it to work. I do not have a computer that has Jaws available so can you please test this and make sure it's working. I did what you recommended.

@jbalboni
Copy link
Contributor

@accbjt I think Trevor is on vacation for the next week or so, we may need to test this as best we can and leave the ticket open for him to validate when he gets back.

@@ -87,6 +87,7 @@ export default class DateWidget extends React.Component {
Month
</label>
<select
aria-describedby={this.props.id}
Copy link
Contributor

Choose a reason for hiding this comment

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

The id isn't a value that would be useful for a user to hear, I don't think. I think we'd want the label, but labels might not be passed to widgets. So we might need to figure out how to pass labels to widgets or convert this to a field instead of a widget.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally we want the <legend> to have a relevant ID and point to that. I'd like the select to read out its own label "Month" in this case, pause, then read out the legend text. Having that additional context for what date we're asking users to put in (service start date, service end date, birth date) is critical for screen readers, especially when we have multiple date fieldsets on the page.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From my understanding the id is connected to the legend id and that is what it should read out. I think aria-describedby is suppose to read out the element which the id is attached to. I never got this to work on Chrome so I wasn't sure if it worked the way @1Copenut wanted it to. @1Copenut were you able to try it out on JAWS?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah yeah, looks like I misunderstood. This would actually need to be the id with -label appended to it.

@1Copenut
Copy link
Contributor

1Copenut commented Jun 5, 2019

@accbjt I did some research and documented findings for different screen reader + browser pairings here: https://github.com/department-of-veterans-affairs/vets.gov-team/blob/master/Practice%20Areas/Accessibility/RESEARCH-screenreader-fieldset-legend-label.md

Might be worth a quick glance to compare your findings on VoiceOver. I'll give this a look as soon as I can. Today's my first day back and I've got a few pressing items to look into.

@accbjt
Copy link
Contributor Author

accbjt commented Jun 7, 2019

I'm moving to the VSP contract so I am going to put this back in the Ready and un-assign myself. @jbalboni @1Copenut what should I do with this branch? I don't want it to get stale and diverge from master. I might be able to pick it up again on VSP but I'm not sure.

@jbalboni
Copy link
Contributor

jbalboni commented Jun 8, 2019

@accbjt Leave it for now, I will try to finish it up next week.

@accbjt
Copy link
Contributor Author

accbjt commented Jun 8, 2019

@jbalboni Great, Thanks

@va-bot va-bot temporarily deployed to vetsgov-pr-10142 June 10, 2019 14:26 Inactive
@jbalboni
Copy link
Contributor

@1Copenut This should be ready for you to review. I verified that works as expected in VoiceOver; the legend text is read out after the specific date field label.

@jbalboni jbalboni self-assigned this Jun 10, 2019
@va-vfs-bot va-vfs-bot temporarily deployed to master/16447-screenreader June 10, 2019 14:51 Inactive
Copy link
Contributor

@1Copenut 1Copenut left a comment

Choose a reason for hiding this comment

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

👍 LGTM. I tested with the big three screen readers in their native environments. All are now reading out the <legend> text as expected. This is a significant improvement in the handling of our multi-label form groups.

@jbalboni jbalboni merged commit 50e2805 into master Jun 13, 2019
@jbalboni jbalboni deleted the 16447-screenreader branch June 13, 2019 13:39
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.

5 participants