Associate the Units ul with the Units h3 with aria-labelledby

[terracoda]

@zepumph from https://github.com/phetsims/ohms-law/issues/153#issuecomment-583528576, I asked you to label the Units ul ... so making a separate issue for this item.

Also, @zepumph, could you associate the Units ul with the Units h3 with an aria-labelledby? I realize that our group-naming conventions are not so consistent, I don't think we used aria-labelledby on the "Force Values" heading in GFL regular, but maybe we should. I'll make an issue for that.

Let me know if you have any questions or concerns.

[zepumph]

Today @terracoda and I discussed how to proceed with conventions around when to add aria-labelledby or not. Right now we feel like if there is a visual heading, then adding aria-labelledby to the UL makes a lot of sense.

I updated the sun demo to include an example like this.

[zepumph]

I'm not really sure how to add this as an option to VerticalAquaRadioButtonGroup though. It isn't really its job to manage things outside of the radio button group. Perhaps we just not this pattern and expect each usage with a heading to add their own labelledby association. @jessegreenberg can you think of a way to add this pattern more strictly. By "this pattern" I mean conditionally creating an aria-labelledby association between a heading and the ul only if(f) there is a heading.

[zepumph]

@jessegreenberg and I discussed, and we can't think of a way to automatically add or assert for this case. I think it is just up to the designers of these radio buttons to make sure that aria-labelledby is added when there is a visual heading. Assigning to @terracoda to make sure this ends up in the style guide for RadioButtons.

[jessegreenberg]

Right now we feel like if there is a visual heading, then adding aria-labelledby to the UL makes a lot of sense.

I am working on https://github.com/phetsims/molecules-and-light/issues/267 and the request there is to have aria-labelledby for the radio buttons which do not have a visual heading. Would there be an issue adding aria-labelledby to all AquaRadioButtonGroups?

[terracoda]

@jessegreenberg, in phetsims/molecules-and-light#267, I broke the "rule of thumb" because I thought the extra help was needed. That's why I think an option is nice, if it is possible.

[jessegreenberg]

OK - just looking for a place to save effort or standardize but if it makes most sense to add association on a case by case basis we can definitely do that.

[zepumph]

OK - just looking for a place to save effort or standardize but if it makes most sense to add association on a case by case basis we can definitely do that.

That is ideal, but I can't think of a reasonable way to make it part of the API. I guess we could have it be an option for an associationObject, but I don't know how we would assert that the client was handling it correctly. Thus I feel like probably just ad hoc additions next to each radio button group is as good as can be.

[jessegreenberg]

To clarify, I was wondering about using aria-labelledby on every AquaRadioButtonGroup, like we do for RadioButtonGroup so the markup and attributes for all PhET radio buttons is the same. But it sounds like there are important semantic differences between the two and so they should be different.

[zepumph]

To clarify, I was wondering about using aria-labelledby on every AquaRadioButtonGroup,

If we decide to do this, let's think about how it will be implemented.

Here I have the following structure (similarly found in ohms-law and GFL)

<h3>Force Values</h3>

<ul>
  <li>Shown</li>
  <li>Hidden</li>
</ul>

With the following scenery code:

var heading = new Node({tagName:'h3', innerContent:'Force Values'});
var radioButtonGroup = new VerticalAquaRadioButtonGroup( .... );

How would you recommend automatically embedding a general aria-labelledby relationship between an "outside" Node like this heading and the ul of the radiobutton group.

Right now we would add a third line like:

var heading = new Node({tagName:'h3', innerContent:'Force Values'});
var radioButtonGroup = new VerticalAquaRadioButtonGroup( .... );
radioButtonGroup.addArialLabelledbyAssociation({
  thisElementName: AccessiblePeer.PRIMARY_SIBLING,
  otherElementName: AccessiblePeer.PRIMARY_SIBLING,
  otherNode: heading

In the "general" pattern though, there is no way to know otherElementName short of passing it in.

Because of this constraint, I don't currently think that a factored out association would benefit us greatly.

[zepumph]

Anyways, for this specific issue the task was just one aria-labelledby association, done in this commit. Anything else here @terracoda?

[zepumph]

@jessegreenberg do you think we should create a new issue to discuss a general way to add an aria-labelledby association to AquaRadioButton? The alternative being "remember to do it at each usage.

[jessegreenberg]

Yes, Ill move to another issue just in case.

[terracoda]

The radio group with aria-labelledby is working great in Ohm's Law.

[zepumph]

Sounds great. Closing