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

Components: Speak Notice message #15745

Open
wants to merge 11 commits into
base: master
from
Open

Components: Speak Notice message #15745

wants to merge 11 commits into from

Conversation

@aduth
Copy link
Member

aduth commented May 20, 2019

Fixes #9442
Closes #15708
Related: #15594

This pull request seeks to move spoken message handling for notices from the notices store into the Notice component.

Implementation notes:

Update (2019-12-10): The commentary in this section has since received feedback at #15745 (comment) , with subsequent revisions summarized at #15745 (comment)

I could use some accessibility feedback here. Per #9442, there's a recommendation to use the role attribute for notices affected by withNotices. It's unclear whether this is appropriate for all notices. The current implementation here preserves existing behavior so far as using wp.a11y.speak, but notices (both existing and from withNotices) will default to a politeness level corresponding to the status of the message (error messages as 'assertive', success messages as 'polite'). From the reading of the relevant roles, it doesn't seem like this must be a guaranteed 1-to-1 mapping (i.e. a success notice could be a time-sensitive, assertive message?), but is implemented instead as a sensible default. Notices can be configured to override this default by assigning role="polite" or role="assertive". The role is not in-fact assigned to the rendered element, but its usage with speak is assumed to be a suitable equivalent behavior.

Feedback needed:

  • Is the default behavior sensible?
  • Is the actual behavior for role in not assigning the DOM attribute acceptable?
    • In my testing, assigning it to the element would cause a duplicate spoken message, if speak is also used.
  • Would it be better to always use role and never wp.a11y.speak?
  • Should role be exposed at all, or instead based entirely from status? Or always 'assertive'?

Testing instructions:

Verify there are no regressions in the spoken messages of notices (example: published post "Post published!" notice).

Verify that notices mounted by withNotices (or ad hoc use of the Notice component) are spoken.

@afercia

This comment has been minimized.

Copy link
Contributor

afercia commented May 25, 2019

@aduth thanks for working on this, I've finally found some time to have a look at it.

speak() vs. alert and status roles

Per #9442, there's a recommendation to use the role attribute for notices affected by withNotices

That was a suggestion: given the notices don't use speak(), a role alert or status would have worked. However, if notices are going to use speak(), they shouldn't use a role. As you pointed out, using both would cause a duplicate spoken message.

The alert and status roles are just specialized types of live regions. The speak() live regions implement the same behavior by the use of live region attributes. Quoting from the spec:

role=alert
A type of live region with important, and usually time-sensitive, information.
Elements with the role alert have an implicit aria-live value of assertive, and an implicit aria-atomic value of true.

role=status
A type of live region whose content is advisory information for the user but is not important enough to justify an alert
Elements with the role status have an implicit aria-live value of polite and an implicit aria-atomic value of true.

One difference is that the speak() live regions use explicit attributes:

aria-live="polite / assertive" aria-atomic="true" aria-relevant="additions text"

Basically, never use both speak() and alert/status roles together.

Which one should be used?

Theoretically, we should leverage existing standards and prefer role alert/status. However, there's one big caveat.

Some screen readers are able to announce elements with role alert / status that are "rendered on the fly" and added to the DOM. Not all screen readers are able to do that though. And that's not the recommended way to use these roles.

For maximum compatibility with the various browser/screen reader combos, it is expected that the elements with role alert / status are present in the DOM on page load. Typically, this is not the way the Gutenberg Notices work. They're added to the DOM. Instead, the speak() live regions are rendered when the DOM is ready and screen readers behave better: they're able to understand they have to "monitor" for changes within those elements.

References:

Using ARIA role=alert or Live Regions to Identify Errors
https://www.w3.org/WAI/WCAG21/Techniques/aria/ARIA19

The error container must be present in the DOM on page load for the error message to be spoken by most screen readers

Example:
https://www.w3.org/WAI/WCAG21/working-examples/aria-alert-identify-errors/

Using role=status to present status messages
https://www.w3.org/WAI/WCAG21/Techniques/aria/ARIA22

Check that the container destined to hold the status message has a role attribute with a value of status before the status message occurs.

Example:
https://www.w3.org/WAI/WCAG21/working-examples/aria-role-status-searchresults/

Considering all this, I'd recommend to use speak().

Code related considerations

The role is not in-fact assigned to the rendered element

Looking at the code, I was initially confused by this. It's just about naming. With this implementation, we're not setting ARIA roles, so I'd suggest to rename the prop and the getDefaultRole() function.

Seems to me what is needed here is to just determine the politeness level. Considering also that developers are not required to know what a role is, I'd suggest to:

  • rename the role prop to politeness
  • rename getDefaultRole() to getDefaultPoliteness() and make it return the politeness value

Is the default behavior sensible?

Looks good to me, as long as there's a way to override the default politeness level. As you pointed out, there's no 1-to-1 mapping between the notices type and the politeness level. For example, there may be cases where a success notice would need to be communicated with an assertive politeness level.

I'd also suggest to well document this and make very clear the difference is:

  • assertive: for important, and usually time-sensitive, information that will interrupt anything else the screen reader is announcing in that moment
  • polite: for advisory information that shouldn't interrupt what the screen reader is announcing in that moment (the "speech queue") or interrupt the current task

Worth also noting that these values are suggestion and assistive technologies may override them based on internal heuristics. For more details: https://www.w3.org/TR/wai-aria-1.1/#aria-live

Is the actual behavior for role in not assigning the DOM attribute acceptable?

Yes. I'd just rename things as commented above.

In my testing, assigning it to the element would cause a duplicate spoken message, if speak is also used.
Would it be better to always use role and never wp.a11y.speak?

See above.

Should role be exposed at all, or instead based entirely from status? Or always 'assertive'?

Not sure I fully understand, I guess the previous considerations cover this point?

Add a way to pass a custom message

While testing the ContrastChecker notice, I realized that the current implementation uses the notice content (children) as the message. This may not be always appropriate.

Note: the ContrastChecker still uses its own useEffect() / speak() which I guess should be removed.

In some cases, we're intentionally using a shorter, simpler message. For example in the ContrastChecker the visible messages are longer, e.g.:

This color combination may be hard for people to read. Try using a darker background color and/or a brighter text color.

While we've opted to use a horter, simpler, message for speak():

This color combination may be hard for people to read.

This is because the longer messages would add a lot of noise and in this cases a shorter message is preferable.

While the use of children as message seems a sensible default, seems to me we have to take into account the need for custom messages. I guess a message prop would work? If there's consensus, this part should be well explained in the documentation.

Note on current documentation:

Not sure this part is accurate:

Do use a Notice when you want to communicate a message of medium importance.

Referring to "medium importance" could be a bit misleading

Question on NoticeList

I'm not sure I understand if all this will work with the NoticeList component, but that's very likely because of my poor coding skills 🙂

@youknowriad

This comment has been minimized.

Copy link
Contributor

youknowriad commented Jun 7, 2019

Should we update this to include the snackbar component?

@youknowriad

This comment has been minimized.

Copy link
Contributor

youknowriad commented Nov 18, 2019

Would be good to refresh and land this PR :)

@aduth aduth force-pushed the update/notice-speak branch from 3e61a21 to 8ca6db1 Dec 10, 2019
@aduth aduth requested review from ellatrix and mkaz as code owners Dec 10, 2019
@aduth

This comment has been minimized.

Copy link
Member Author

aduth commented Dec 10, 2019

Apologies for my delay in revisiting this pull request, and a belated "Thank you!" to @afercia for your extended feedback.

I've had a chance to refresh this today. Specifically, this includes:

  • Rename the new Notice role prop to politeness, expecting one of the valid aria-live attribute values. This includes extended documentation on what's expected with these values (based on feedback that this be provided as a suggestion, and detailed overview of what's expected from each of 'polite' and 'assertive').
  • Include a new Notice prop spokenMessage to have full control over the message spoken in the notice.
  • Use this new spokenMessage prop in the implementation of ContrastChecker for its custom (shortened) message.
  • Implement the same speak behavior in the Snackbar component, including the same politeness, spokenMessage props from Notice. The default politeness for Snackbar is fixed to a value of 'polite', consistent with the prior behavior and in accordance with its documented usage for low-priority, non-interruptive messages. This can be overridden using the politeness prop.
Copy link
Contributor

youknowriad left a comment

This is looking good to me 👍

I'd appreciate another a11y sanity check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.