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

feature(date-picker): added aria-labels to year and month select elem… #2636

Merged
merged 4 commits into from
Aug 28, 2018

Conversation

RonnBlack
Copy link
Contributor

…ents

Before submitting a pull request, please make sure you have at least performed the following:

  • [] read and followed the CONTRIBUTING.md guide.
  • [] built and tested the changes locally.
  • [] added/updated any applicable tests.
  • [] added/updated any applicable API documentation.
  • [] added/updated any applicable demos.

@pkozlowski-opensource pkozlowski-opensource added this to the 3.1.0 milestone Aug 24, 2018
@pkozlowski-opensource
Copy link
Member

@RonnBlack based on the discussion in #2543 it doesn't seem like aria labels alone are enough...
Did you tests with screen readers and found different results in your testing?

@rblack-a3digital
Copy link

Thanks for pointing out the discussion. I hadn't seen this.
I was only basing my changes on recommendations from WAVE (Web Accessibility Evaluation Tool) and I didn't actually test with a screen reader. I'll update the code to add the title property and update the pull request.

@pkozlowski-opensource
Copy link
Member

. I'll update the code to add the title property and update the pull request.

To be frank we are still unsure if adding this property is the right thing to do as it will have visual impact. We need more solid testing with different screen readers.

@RonnBlack
Copy link
Contributor Author

RonnBlack commented Aug 25, 2018

I added titles to the select elements and tested in Chrome, Firefox, and Edge. I saw no display issues on Windows 10 or Mac OS High Sierra (10.13.6). You mentioned that there would be visual impact... do you know of something specific?

I also tested with Voice Over on Mac and Narrator on Windows and the drop downs seemed to behave as I would expect. It looks like the pull request shows the additional changes I made. If that isn't the case let me know.

@benouat
Copy link
Member

benouat commented Aug 28, 2018

You mentioned that there would be visual impact... do you know of something specific?

I think @pkozlowski-opensource was mentioning the fact that browsers add a native native yellow tooltip on elements with title attribute when you hover them and wait for some milliseconds.

@RonnBlack
Copy link
Contributor Author

Yes the hover behavior is there but the next and previous month buttons also have that behavior.
In my testing with screen readers it seemed to work fine with just the aria-label even before I added the title. The only problem I noticed is that the Microsoft Narrator didn't announce the label on the Month select but it did properly announce it on the Year. Once title was added it was fine with both. Mac Voice over didn't have a problem either way but IMHO the way it announced what you were on was more confusing (perhaps there is some control over this)

Copy link
Member

@maxokorokov maxokorokov left a comment

Choose a reason for hiding this comment

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

It actually doesn't shock me to have titles on select boxes and we already have them on previous/next arrows.

Thanks, @RonnBlack !

screen shot 2018-08-28 at 17 22 59
screen shot 2018-08-28 at 17 28 48

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.

None yet

5 participants