phetsims / ohms-law

"Ohm's Law" is an educational simulation in HTML5, by PhET Interactive Simulations.
http://phet.colorado.edu/en/simulation/ohms-law
GNU General Public License v3.0
5 stars 6 forks source link

Slider control heading might be out of order in PDOM #94

Closed terracoda closed 7 years ago

terracoda commented 7 years ago

Sorry missed this issue in my notes from Sept 28 session with screen reader user.

Using Safari 11 Mac os x 10.12.6 (sierra) and Ohm's Law described version 1.4 Dev 12

User noted while looking for sliders that the slider control heading seems to be below controls?

@zepumph or @jessegreenberg, can you check that the Slider Controls heading comes before the controls?

This might already be fixed.

zepumph commented 7 years ago

Currently on master I see image

in the a11y view, and here in the actual html image

Does this seem right to you @terracoda?

terracoda commented 7 years ago

Thanks for checking @zepumph. Yes that looks right to me. I'll check this again with the screen reader consultant. Hoping to meet tomorrow.

zepumph commented 7 years ago

Alright thanks

terracoda commented 7 years ago

@zepumph, I do not know why, but in my session yesterday with a VO, the Slider Controls heading seems to come after the group of sliders.

I will have to explore more myself, and get back to you on this.

jessegreenberg commented 7 years ago

In 10/13/17 a11y meeting, we discovered that this could be related to the placement of aria-labelledby. @terracoda will investigate this further.

terracoda commented 7 years ago

@zepumph, I experienced this today. I heard the heading and the on-demand help text after the un-ordered list containing the sliders.

zepumph commented 7 years ago

In the above picture of the PDOM HTML, you see that the sliders is the only div that uses aria-labeledby. I will try removing it.

zepumph commented 7 years ago

@terracoda can you see if it is fixed on phettest.colorado.edu/ohms-law/ohms-law-a11y-view.html?brand=phet&accessibility

terracoda commented 7 years ago

@zepumph, the latest build sounds good to me without the aria-labelledby attribute at all. I would like to try the attribute on the ul that wraps the two slider controls. That is the group that the Slider Controls heading is actually labeling. I think aria-labelledby makes them interactive as a group. The screen reader consultant said this "group interactivity" was useful.

Do you mind making this small update, so I can test it. Then we can see which is better, 1. no aria-labelleby or 2. a better placed aria-labelledby.

<ul aria-labelledby="label-210-271-799-783-782">
...
</ul>
zepumph commented 7 years ago

@jessegreenberg how do I do this? Is It currently possible as part of the mixin? If not perhaps we should add it as part of https://github.com/phetsims/scenery/issues/686. I'm thinking something like:

new Node({
  tagName: 'ul',
  accessibleLabel: 'Slider Controls',
  labelTagName: 'h3',
  ariaLabeledBy: true,
});

html:

<div tabindex="-1" id="container-210-271-799-783-782">
<h3 tabindex="-1" id="label-210-271-799-783-782">Slider Controls</h3>
<ul tabindex="-1" id="210-271-799-783-782" aria-labeledby="label-210-271-799-783-782">
    <li tabindex="-1" id="container-210-271-799-783-782-744-762-745"><label tabindex="-1" id="label-210-271-799-783-782-744-762-745" for="210-271-799-783-782-744-762-745">V, Voltage</label><input id="210-271-799-783-782-744-762-745" tabindex="-1" step="0.1" min="0.1" max="9" role="slider" aria-valuetext="4.5 Volts" type="range"></li>
    <li tabindex="-1" id="container-210-271-799-783-782-763-781-764"><label tabindex="-1" id="label-210-271-799-783-782-763-781-764" for="210-271-799-783-782-763-781-764">R, Resistance</label><input id="210-271-799-783-782-763-781-764" tabindex="-1" step="0.1" min="10" max="1000" role="slider" aria-valuetext="500 Ohms" type="range"></li>
</ul>
</div>
jessegreenberg commented 7 years ago

@zepumph I think this is possible, outside of options there are functions setAriaLabelledByNode setAriaLabelContent. The first takes a node, the second takes a string which is one of the static constants in AccessiblePeer.

The reason for complexity is that the value for aria-labelledby can be an id for any HTMLElement in the DOM, not just the label for an adjacent element. And for the functions to work, accessible content for both Nodes must be defined. Something like this might work (haven't verified)

var myNode = new Node({
  tagName: 'ul',
  accessibleLabel: 'Slider Controls',
  labelTagName: 'h3'
});

// this node is aria-labelledby its own accessible label element
myNode.ariaLabelledByNode = myNode;
myNode.ariaLabelContent = AccessiblePeer.LABEL;

There is documentation for these functions in Acecssibility.js, let me know if this helps! The API is probably far from ideal but it was the only way I could think of to associate two arbitrary Nodes in the scene graph.

terracoda commented 7 years ago

@zepumph, aria-labelledby has three L's not two :-) The spelling is confusing.

zepumph commented 7 years ago

@terracoda try out master now, the ul now has aria-labelledby on it. Let me know what you like.

terracoda commented 7 years ago

@zepumph, it might be extraneous information, but it is working fine. The un-ordered list containing the sliders is named as "list Slider Controls, two items". VO announces this on enter the list with the arrow keys and as exiting the list.

Sounds good to me. Closing this issue.