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

Add VoiceOver accessibility to contrast-checker #14649

Merged
merged 6 commits into from
Apr 4, 2019

Conversation

AmartyaU
Copy link
Contributor

@AmartyaU AmartyaU commented Mar 26, 2019

Addresses #10581 (comment)

Description

Added VoiceOver accessibility functionality to contrast-checker

How has this been tested?

Changed background and text color options in color settings and heard warning being announced in addition to it being displayed in all cases

Tested with VoiceOver on Mac in a Chrome browser

Types of changes

New feature (non-breaking change which adds functionality)

Testing

Insert Paragraph block and start playing with its colors with VoiceOver enabled.

Screen Shot 2019-03-28 at 14 16 39

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

@AmartyaU AmartyaU changed the title Fixes #10581 https://github.com/WordPress/gutenberg/issues/10581#issuecomment-474761908 Fixes #10581 Mar 26, 2019
@AmartyaU AmartyaU changed the title Fixes #10581 Add VoiceOver accessibility functionality to contrast-checker Mar 26, 2019
@AmartyaU AmartyaU changed the title Add VoiceOver accessibility functionality to contrast-checker Add VoiceOver accessibility to contrast-checker Mar 26, 2019
@gziolo gziolo added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Accessibility Feedback Need input from accessibility User Experience (UX) labels Mar 28, 2019
@gziolo
Copy link
Member

gziolo commented Mar 28, 2019

I tested it with VoiceOver enabled. It's very aggressive when you try to use custom colors. Every time I pick a new color which doesn't meet criteria triggers the announcement even if this is exactly the same message. It might be a better idea to trigger speak with 3 messages only when they change:

  • no violation - no message
  • violation fixed - message informing that color combination is good
  • try a brighter color message
  • try a darker color message

@afercia
Copy link
Contributor

afercia commented Mar 28, 2019

Wondering it the message second part is actually useful, both visually and as an audible message:

Try using a brighter background color and/or a darker text color.
Try using a darker background color and/or a brighter text color.

What is the value added by these sentences? They add cognitive load and make the entire message very long. There's a lot to absorb visually and I'd argue most users won't even get the difference between the two sentences:

Screenshot 2019-03-28 at 15 05 58

Wouldn't the first part be enough? I'd consider to display and speak() this simplified version:

This color combination may be hard for people to read.

Not sure there's the need for a "violation fixed" message.

@AmartyaU
Copy link
Contributor Author

I agree. When I was initially testing, it took me some time to realize both error messages were different.

I also think error messages shouldn't be this long, because we are expecting a long span of attention from users which users don't usually have.

On the other hand, just having "This color combination may be hard for people to read" can induce learned helplessness in users (repeated failure or errors combined with inability to form consistent explanation may induce frustration in users).

@afercia
Copy link
Contributor

afercia commented Mar 28, 2019

Yeo I'd agree there's a need to find a good balance. Not sure it can help, but for the alt text description we've tried to balance the need for a short message and the need to provide sufficient information by linking to a proper resource:

Screenshot 2019-03-28 at 15 38 32

@afercia
Copy link
Contributor

afercia commented Mar 29, 2019

Discussed during today's accessibility bug-scrub, agree to recommend to simplify the messages.

@afercia afercia removed the Needs Accessibility Feedback Need input from accessibility label Mar 29, 2019
@gziolo gziolo added the [Type] Enhancement A suggestion for improvement. label Mar 29, 2019
@gziolo
Copy link
Member

gziolo commented Mar 29, 2019

This color combination may be hard for people to read.

Let's update it as suggested by @afercia and it should be ready to land :)

We also need to have a similar notice with VoiceOver support for Heading level validation as described in the parent issue. I'm saying, in case you are interested in opening another PR.

@AmartyaU AmartyaU requested a review from ellatrix as a code owner March 29, 2019 16:30
@AmartyaU
Copy link
Contributor Author

Thank you for the feedback @afercia @gziolo! I updated the messages in speak. I can also try my hand at the parent issue.

Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

It works great, thanks @AmartyaU for working on it. Nice job 💯

@gziolo gziolo added this to the 5.5 (Gutenberg) milestone Apr 4, 2019
@gziolo gziolo merged commit 988439a into WordPress:master Apr 4, 2019
@AmartyaU AmartyaU deleted the fix-VoiceOver branch April 4, 2019 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants