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

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

Closed
terracoda opened this issue Oct 11, 2017 · 14 comments
Closed

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

terracoda opened this issue Oct 11, 2017 · 14 comments
Assignees

Comments

@terracoda
Copy link

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
Copy link
Member

zepumph commented Oct 11, 2017

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
Copy link
Author

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
Copy link
Member

zepumph commented Oct 11, 2017

Alright thanks

@terracoda
Copy link
Author

@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
Copy link
Contributor

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
Copy link
Author

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

@zepumph
Copy link
Member

zepumph commented Oct 18, 2017

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 zepumph self-assigned this Oct 18, 2017
@zepumph
Copy link
Member

zepumph commented Oct 18, 2017

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

@zepumph zepumph removed their assignment Oct 18, 2017
@terracoda
Copy link
Author

terracoda commented Oct 23, 2017

@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
Copy link
Member

zepumph commented Oct 23, 2017

@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 phetsims/scenery#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
Copy link
Contributor

@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.

@jessegreenberg jessegreenberg removed their assignment Oct 23, 2017
@terracoda
Copy link
Author

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

@zepumph
Copy link
Member

zepumph commented Oct 24, 2017

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

@zepumph zepumph removed their assignment Oct 24, 2017
zepumph added a commit that referenced this issue Oct 24, 2017
zepumph added a commit to phetsims/scenery that referenced this issue Oct 24, 2017
@terracoda
Copy link
Author

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants