search

phetsims / a11y-research

a repository to track PhETs research into accessibility, or "a11y" for short
MIT License
3 stars 0 forks source link

Should ResetAllButton instances be labeled in the PDOM? #55

Closed zepumph closed 6 years ago

zepumph commented 7 years ago

From https://github.com/phetsims/molarity/issues/37,

I noticed that none of our accessibility outfitted sims have a name/label for the reset all button, see the a11y view below for BASE (witnessed in OL as well). Is this desired, or should the reset all button have more work done to it.

image

In the code I see that there is an accessibleLabel assigned in the default options. Maybe I am not understanding fully, @jessegreenberg please explain if I am wrong.

jessegreenberg commented 7 years ago

The ResetAllButton is labelled, but the label is added as an inline attribute with ARIA. The HTML looks like this:

<input id="157-218-750-751-746" aria-label="‪Reset All‬" type="button">

So no label is rendered in the DOM copy. I don't believe there is any reason to have the HTML look like this. @zepumph would you please change the HTML to look like this?

<button>Reset All</button>
zepumph commented 7 years ago

Could you be a little bit more specific here @jessegreenberg. Am I updating options for the Accessibility mixin in ResetAllButton.js, or something more direct to the a11y view, because I thought that that specific view was created dynamically based on scenery.

jessegreenberg commented 7 years ago

Sure, I was envisioning updating the a11y options used in ResetAllButton so that instead of having tagName of input with an aria-label it could have a tagName of button with the label as textContent.

This will fix the problem for ResetAll (and also improve the markup), but you are correct that other elements that use aria-label will have this problem in the a11y view.

jessegreenberg commented 7 years ago

A way to improve the a11y view to fix this problem could be to detect when aria-label attribute is used on a PDOM element and add some visible label in the PDOM copy. That might be non-trivial, but haven't looked into it in depth.

zepumph commented 7 years ago

I'm always stoked for a robust solution. I'll take a look at the a11y view first.

zepumph commented 7 years ago

@jessegreenberg if there is an 'aria-label' attribute on a tag, is there any benefit in having content in the inner text as well?

jessegreenberg commented 7 years ago

No, we would typically want one or the other.

zepumph commented 7 years ago

@jessegreenberg would you take a look at this implementation, does anything strike you as bad form?

jessegreenberg commented 6 years ago

@zepumph this is working great!

I noticed that the aria-label is being replaced by value, but aria-label can be added to any HTML element, not just inputs. So to be general, I think we need a way to show the aria-label on elements that are not input.

Otherwise, looks great.

jessegreenberg commented 6 years ago

Just brainstorming, maybe we can use value if an input, innerText if the element supports inner text, and maybe something else otherwise? (if there even is another case).

zepumph commented 6 years ago

@jessegreenberg what do you think about that? I'm not sure if it is full proof yet, and I didn't have a way to really test it as I couldn't find an example that made sense to me.

terracoda commented 6 years ago

@zepumph, great question. It will be great to have the text of the aria-label attributes in the visual PDOM side for clarity.

jessegreenberg commented 6 years ago

That seems good to me @zepumph, thanks!