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

Discuss Labeling Sliders Controls group with H3 and removing the fieldset #87

Closed terracoda closed 7 years ago

terracoda commented 7 years ago

Sorry for rethinking this structure, but the legend element does NOT (missed in initial post) offer the same kind of navigation and outlining functionality as a heading. It is super useful to have the content, "Slider Controls", in the page outline for the simulation.

Furthermore, although the fieldset element does create a group for sliders, some AT counts every item within the fieldset rather than just the number of actual sliders, making it sound like it has more interactive things in it than it actually has.

I would like to discuss the following change to the HTML PDOM:

<!-- Sliders for Voltage and Resistance -->
    <div id="silder-widget" aria-labelledby="label-silders">
      <h3 id="label-silders">Slider Controls</h3>
      <!-- On-demand help text. -->
        <p>Voltage and resistance sliders allow changes to equation and circuit.</p>
        <ul>
          <li><label for="v-slider">V, voltage <input id="v-slider" name="v-slider" type="range" role="slider" min="0.1" max="9.0" step="0.1" aria-valuemin="0.1" aria-valuemax="0.9" aria-valuenow="4.5" aria-valuetext="4.5 volts"></label></li>
          <li><label for="r-slider">R, resistance <input id="r-slider" name="r-slider" type="range" role="slider" min="10" max="1000" step="5" aria-valuemin="10" aria-valuemax="1000" aria-valuenow="500" aria-valuetext="500 ohms"></label></li>
        </ul>
    </div>

I think this is a simpler and much more friendly structure for users. @jessegreenberg, @zepumph, any thoughts?

terracoda commented 7 years ago

@jessegreenberg and @zepumph, I have updated the html sketches for Ohms Law in the a11y-research repo.

jessegreenberg commented 7 years ago

@terracoda is there a semantic difference by wrapping labels in that way? What is the difference for AT between

<li>
  <label for="v-slider">V, voltage
    <input id="v-slider" name="v-slider" type="range" role="slider" min="0.1" max="9.0" step="0.1" aria-valuemin="0.1" aria-valuemax="0.9" aria-valuenow="4.5" aria-valuetext="4.5 volts">
  </label>
</li>

and

<li>
  <label for="v-slider">V, voltage</label>
  <input id="v-slider" name="v-slider" type="range" role="slider" min="0.1" max="9.0" step="0.1" aria-valuemin="0.1" aria-valuemax="0.9" aria-valuenow="4.5" aria-valuetext="4.5 volts">
</li>
jessegreenberg commented 7 years ago

If it is important to wrap sliders with labels, would it be acceptable to wrap the text content with a <span>? At the moment, scenery doesn't support this kind of wrapping with innerHTML.

EDIT: by wrapping with a span, I mean

<li>
  <label for="v-slider">
    <span>V, voltage</span>
    <input id="v-slider" name="v-slider" type="range" role="slider" min="0.1" max="9.0" step="0.1" aria-valuemin="0.1" aria-valuemax="0.9" aria-valuenow="4.5" aria-valuetext="4.5 volts">
  </label>
</li>
terracoda commented 7 years ago

@jessegreenberg, as long as we use the forattribute to provide an accessible name, there is no need for the wrapping. I think wrapping causes automatic association of the input and the label elements, but I would have to double check that. When we worked on BASE, I thought Scenery preferred the wrapping.

We can go without the wrapping.

terracoda commented 7 years ago

is there a semantic difference by wrapping?

No. Just to be clear, the most important thing is that all controls (sliders in this case) have an accessible name. Using for makes the label's content accessible in the accessibility tree.

jessegreenberg commented 7 years ago

Perfect, thanks @terracoda, that makes sense. Scenery used to have trouble with including <label> elements in general, but at least we support that now.

After today's meeting discussion how would you like to proceed with this issue @terracoda?

terracoda commented 7 years ago

@jessegreenberg, for this sim, I think a heading works best for helping students find what is most important in the sim. The difference in the sim's outline misses an essential piece.

With a fieldset & legend: h1: Ohm's Law h2: Scene Summary h3: State of Sim h2: Play Area h3: The Equation h3: The Circuit h2: Control Panel

With an h3: h1: Ohm's Law h2: Scene Summary h3: State of Sim h2: Play Area h3: The Equation h3: The Circuit h3: Slider Controls h2: Control Panel

Also after reading this article by Léonie Watson (2016), and learning what JAWS does with fieldset/legend, I think we should be careful using them for sims. JAWS repeats the legend text for each form element inside the fieldset. The expected behaviour is that the screen reader should only read out the legend text for the first element.

From a design perspective, the sliders are related in a sense in that they are the only controls in the Play Area and they are both sliders, but they do not depend on each other. Nor does a user need to hear the text, "Slider Controls", to understand what each slider is or what is used for.

In fact, I think using a fieldset in this case is the wrong decision, semantically.

Edit: FYI, VO also repeats legend text with each radio, as does JAWS.

jessegreenberg commented 7 years ago

Sounds great @terracoda, thanks. We will proceed with the markup listed in the original issue comment (without wrapping input with label).

terracoda commented 7 years ago

And for a list of radios, I think using the radiogroup role might be the most semantically correct thing to, but I also think we should test how the group's label works across our supported platforms. This is something we can work in https://github.com/phetsims/a11y-research/issues/57.

terracoda commented 7 years ago

Thanks @jessegreenberg! Updating the html for clarity:


<!-- Sliders for Voltage and Resistance -->
<div id="silder-widget" aria-labelledby="label-silders">
  <h3 id="label-silders">Slider Controls</h3>
<!-- On-demand help text. -->
  <p>Voltage and resistance sliders allow changes to equation and circuit.</p>
  <ul>
    <li>
      <label for="v-slider">V, voltage </label>
      <input id="v-slider" name="v-slider" type="range" role="slider" min="0.1" max="9.0" step="0.1" aria-valuemin="0.1" aria-valuemax="0.9" aria-valuenow="4.5" aria-valuetext="4.5 volts">
    </li>
    <li>
      <label for="r-slider">R, resistance</label>
      <input id="r-slider" name="r-slider" type="range" role="slider" min="10" max="1000" step="5" aria-valuemin="10" aria-valuemax="1000" aria-valuenow="500" aria-valuetext="500 ohms">
    </li>
  </ul>
</div>
jessegreenberg commented 7 years ago

The Accessibility mixin in Scenery doesn't allow setting aria-labelledby and aria-describedby associations for parent container elements. We should add this functionality in the mixin.

terracoda commented 7 years ago

@jessegreenberg, I agree that is a good thing to have in the mixin.

terracoda commented 7 years ago

And, I think that it is not a show-stopper for Ohm's Law. We can see how it works without aria-lablledby and add it later.

jessegreenberg commented 7 years ago

Thanks for the updated HTML @terracoda! Changes applied above, here is the HTML in the sim:

<div tabindex="-1" id="container-157-218-749-733-732" aria-labelledby="label-157-218-749-733-732">
  <h3 tabindex="-1" id="label-157-218-749-733-732">Slider Controls</h3>
  <p tabindex="-1" id="description-157-218-749-733-732">Voltage and resistance sliders allow changes to equation and circuit.</p>
  <ul tabindex="-1" id="157-218-749-733-732">
    <li tabindex="-1" id="container-157-218-749-733-732-696-713-697">
      <label tabindex="-1" id="label-157-218-749-733-732-696-713-697" for="157-218-749-733-732-696-713-697">V, Voltage</label>
      <input id="157-218-749-733-732-696-713-697" tabindex="0" role="slider" aria-valuetext="4.5 Volts" type="range" style="width: 1px;">
    </li>
    <li tabindex="-1" id="container-157-218-749-733-732-714-731-715">
      <label tabindex="-1" id="label-157-218-749-733-732-714-731-715" for="157-218-749-733-732-714-731-715">R, Resistance</label>
      <input id="157-218-749-733-732-714-731-715" tabindex="0" role="slider" aria-valuetext="500 Ohms" type="range" style="width: 1px;">
    </li>
  </ul>
</div>

Assigning back to @terracoda to close if all looks well.

terracoda commented 7 years ago

@jessegreenberg, just curious why tabindex="-1" is on everything?

terracoda commented 7 years ago

I would think you would only need to

Use tabindex=-1 to give an element programmatic focus, but exclude it from the tab order of the content

See article Using tabindex attribute

I might not fully understand when programmatic focus control is needed.

terracoda commented 7 years ago

@jessegreenberg, maybe you need to explicitly use tabindex="0" and tabindex="-1" because you are building a "Parallel DOM" and not a real DOM? Is that the reason?

terracoda commented 7 years ago

@jessegreenberg, everything else looks good to me.

jessegreenberg commented 7 years ago

Yes, good question @terracoda, in https://github.com/phetsims/john-travoltage/issues/206 we discovered that IE11 gives everything tabindex="0" by default. Recommendation suggests that default value should be -1 for things that are not focusable, but so it goes.

This was problematic because we have functions in scenery that rely on tabindex to calculate what the next focusable element in the document should be. So giving everything tabindex="-1" explicitly is a way to normalize behavior for IE11.

terracoda commented 7 years ago

@jessegreenberg thanks for answering. I was wondering if it was some kind of work around. So, it is not needed explicitly for the PDOM, but actually is a work around for IE11.

jessegreenberg commented 7 years ago

@terracoda, exactly, just a workaround for IE11. Closing issue, a new version with this structure is deployed here: http://www.colorado.edu/physics/phet/dev/html/ohms-law/1.4.0-dev.11/ohms-law-a11y-view.html

terracoda commented 7 years ago

@jessegreenberg thanks!