Page MenuHomePhabricator

Add support for aural-only and visual-only elements
ClosedPublic

Authored by epriestley on Apr 21 2014, 2:22 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Nov 27, 8:16 AM
Unknown Object (File)
Sun, Nov 27, 8:15 AM
Unknown Object (File)
Sun, Nov 27, 8:15 AM
Unknown Object (File)
Sun, Nov 27, 8:15 AM
Unknown Object (File)
Sun, Nov 27, 8:15 AM
Unknown Object (File)
Sun, Nov 27, 8:15 AM
Unknown Object (File)
Sun, Nov 27, 1:25 AM
Unknown Object (File)
Fri, Nov 18, 7:33 AM

Details

Summary

Ref T4843. This adds support to javelin_tag() for an aural attribute. When specified, true values mean "this content is aural-only", while false values mean "this content is not aural".

  • I've attempted to find the best modern approaches for marking this content, but the aural attribute should let us change the mechanism later.
  • Make the "beta" markers on application navigation visual only (see T4843). This information is of very low importance, the application navigation is accessed frequently, and the information is available on the application list.
  • Partially convert the main navigation. This is mostly to test things, since I want to get more concrete feedback about approaches here.
  • Add a ?__aural__=1 attribute, which renders the page with aural-only elements visible and visual-only elements colored.
Test Plan

Screen_Shot_2014-04-21_at_7.11.34_AM.png (892×1 px, 196 KB)

Diff Detail

Repository
rP Phabricator
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

epriestley retitled this revision from to Add support for aural-only and visual-only elements.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added reviewers: btrahan, chad.

Adding in Stefan, who spent good time at FB on this issue.

Stefan, if you have a few minutes, would love some guidance here if you have any.

epriestley edited edge metadata.

One of the notification/message strings was flipped.

Although this works, you might find with this implementation that you're going to be duplicating a lot of things just to get text readable to a screen reader. Instead you might consider just labeling the existing elements better so they seem less confusing to a blind user. For instance, instead of just hiding your alerts, give them labels so that they have meaning.

<a href="/notification/" class="sprite-menu alert-notifications message-unread">
  <span class="sprite-menu phabricator-main-menu-alert-icon"></span>
  <span class="phabricator-main-menu-alert-count">0</span>
</a>

This can become:

<a href="/notification/" class="sprite-menu alert-notifications message-unread">
  <span class="sprite-menu phabricator-main-menu-alert-icon">
    <span class="aural-only">Alerts: </span>
  </span>
  <span class="phabricator-main-menu-alert-count">0</span>
</a>

In this specific case, the action on the visible links is (I think?) bad for screen readers, since it pops open a dropdown that shows up in a totally different part of the document. That is, if you "click" the existing element, you don't navigate anywhere, but instead affect a visually related but aurally disconnected part of the document body. I'm not actually sure if this is a problem, but it's difficult for me to imagine it works well.

ARIA has standards to enable the understanding of these drop-downs with screen readers. You can give your link aria-haspopup="true" and aria-expanded="false". Then, when the popup opens, you should set focus within it and return focus to the link when the popup closes. Ideally you should also trap focus within the popup while it is open and give your link a role="button".

epriestley edited edge metadata.
  • Update the rest of the main menu.
  • Update the remarkup control.

The ARIA stuff for dealing with menus is really good to know, but I don't want to implement it for now because it would make this patch a lot more complex and it's difficult for me to test confidently. I don't think changing these into links makes the experience worse: the target pages have exactly the same content as the dropdowns.

There are other cases where we can't reasonably do this and we'll eventually need all the aria stuff, but I just want to fix the few most troublesome things that were called out in T4843 for now.

chad edited edge metadata.
chad added inline comments.
webroot/rsrc/css/core/core.css
140

assume you kept this out of z-index.css on purpose

This revision is now accepted and ready to land.May 1 2014, 2:12 PM

I should probably move it, I was internally conflicted over whether "aural" or "z-index" is a more specialized state.

epriestley edited edge metadata.
  • Move z-index rule.
epriestley updated this revision to Diff 21180.

Closed by commit rPe8cebb7da5a4 (authored by @epriestley).