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

ABSwitch: support for accessibleName and helpText #880

Closed
pixelzoom opened this issue Oct 20, 2024 · 13 comments
Closed

ABSwitch: support for accessibleName and helpText #880

pixelzoom opened this issue Oct 20, 2024 · 13 comments

Comments

@pixelzoom
Copy link
Contributor

Related to phetsims/models-of-the-hydrogen-atom#67 and phetsims/sun#901 ...

ABSwitch does not currently support accessibleName and helpText. Values that are provided are silently ignored.

@terracoda noted that there has been work done on OnOffSwitch, but not on ABSwitch. Since OnOffSwitch extends ABSwitch, whoever does this work should review OnOffSwitch for things that may belong in the base class ABSwitch.

Binder documentation should be provided in ABSwitch.md. The documentation should be clear about the low-level internals of the component and the high-level API intended to be used in sim-specific code. Provide programming examples of how to use in sim-specific code.

@pixelzoom
Copy link
Contributor Author

Note that this issue blocks phetsims/models-of-the-hydrogen-atom#67.

@jessegreenberg
Copy link
Contributor

Support for accessibleName and helpText were added in phetsims/sun@1ca9716.

In phetsims/scenery#1666, I also tried out a way to omit ParallelDOMOptions and tested it in ABSwitch (phetsims/sun@4a5761c).

Binder documentation should be provided in ABSwitch.md.

I am not sure what the plan is for binder, and I think we should wait on these until that is decided.

@pixelzoom can you please review?

pixelzoom added a commit to phetsims/models-of-the-hydrogen-atom that referenced this issue Oct 28, 2024
@pixelzoom
Copy link
Contributor Author

Implementation looks good. And it seems to be working in MOTHA, but that probably needs a designer to sign off.

I'll comment about TrimmedParallelDOMOptions in phetsims/scenery#1666. But it's usage in ABSwitch should probably be:

- export type ABSwitchOptions = TrimmedParallelDOMOptions<SelfOptions & HBoxOptions>;
+ export type ABSwitchOptions = SelfOptions & TrimmedParallelDOMOptions<HBoxOptions>;

... because HBoxOptions is where ParallelDOMOptions is coming from.

Back to @jessegreenberg.

@jessegreenberg
Copy link
Contributor

You are right, thanks! I fixed that. OK, Ill review this with design before closing.

@jessegreenberg
Copy link
Contributor

jessegreenberg commented Nov 5, 2024

I reviewed this with @terracoda. The behavior of accessibleName and helpText is correct.

However, the accessible name + aria-pressed state alone are not sufficient for ABSwitch when the switch values are more than just "true"/"false". Using aria-describedby might be able to describe the state of the switch better than having both values in the accessible name. We are going to continue to work on it.

For example, in this ABSwitch:

image

The user will hear "Experiment or model, not checked" - that does not convey the state.

@pixelzoom
Copy link
Contributor Author

I suspect that this is blocking for MOTHA, so I've created a GitHub issue in that repo: phetsims/models-of-the-hydrogen-atom#74

@terracoda
Copy link

terracoda commented Nov 5, 2024

VoiceOver recognizes the switch role, and says:
"Experiment or Model, on, switch" when Model is selected and
Screenshot 2024-11-05 at 17 49 56
"Experiment or Model, off, switch" when Experiment is selected
Screenshot 2024-11-05 at 17 51 09

Unfortunately, HTML and ARIA do not yet natively support an AB Switch, though there are countless numbers of them around.

I think we need a different approach... see next comment.

@terracoda
Copy link

terracoda commented Nov 5, 2024

This is an idea.

  • use a simple push button, like the Add/Remove Wall in BASE.
  • make the name dynamic, like in BASE.
  • make the helptext dynamic and automatic in order to have the non-active state of the visual AB Switch - NOT like BASE.

Focus moves to AB Switch with B selected, we hear:

  • Model Mode, button, + auto helptext: Explore atomic models or switch to Experiment Mode to collect data.

Focus moves to AB Switch with A selected, we hear:

  • Eperiment Mode, button, + auto helptext: Collect data or switch to Model Mode to explore atomic models.

Space and Enter - toggle the position of the switch
I am suggesting a push button with a dynamic name, and dynamic help text automatically spoken via aria-describedby.

A context response could announce additional changes or used to scaffold/confirm the change in the mode.
This might be simple enough to try out.

@terracoda
Copy link

terracoda commented Nov 6, 2024

Second idea - H3 + dynamic button name + static help text + (optional confirmatory context response).

This design might be somewhat like a radio group / radio button experience, but without the associated structures of a radio button group that make the screen reader experience very chatty. Screen readers say things like "selected, Model Mode, 1 of 2, Exploration Mode, group".

H3: Exploration Mode
P: Explore atomic models or collect experimental data.
Push Button with dynamic accessible name:

  • Button focus event A (Model Mode): Model, Switch to Experiment Mode, button
    • Confirmatory Context Response: Experiment Mode, start collecting data.
  • Button focus event B (Experiment Mode): Experiment, Switch to Model Mode, button
    • Confirmatory Context Response: Model Mode, Explore atomic models.

My understanding is that a confirmatory Context Responses is simple to implement, BUT, in this case, the response repeats what is already available in the help text. We could leave the spoken confirmation off and rely solely on the UI-sound to confirm a successful switch in the mode.

@jessegreenberg
Copy link
Contributor

Discussed with @terracoda. We are going to use the design in #880 (comment). So it will need options for the name for valueA and the name for valueB.

The actual accessible name will be set based on the state of the switch, with a string pattern:

"{{valueAAccessibleName}}", Switch to {{valueBAccessibleName}}"

The aria attributes set by ToggleSwitch should be removed for ABSwitch. So we will either need to make them optional or move them somewhere else.

The context responses are not part of "Accessible Names and Help Text", but they would be added with options like this:

valueAContextResponse - The context response when the value becomes A
valueBContextResponse - The context response when the value becomes B

@jessegreenberg
Copy link
Contributor

This was implemented in phetsims/sun@f9f6b98.

I removed the accessibleName from the ABSwitch used in MOTHA. Providing valueAAccessibleName and valueBAccessibleName should not be needed in MOTHA because of PDOMUtils.findStringProperty in ABSwitch.

This design for ABSwitch removes the aria attributes that are usually added to ToggleSwitch, so I made them optional in phetsims/sun@bf64f68.

@pixelzoom would you mind reviewing these changes?

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Nov 8, 2024

Code changes and API look good to me.

Since @terracoda identified several design options, I think we should have her review, to verify that the implementation meets her design expectations. MOTHA may be the only sim using this feature, so feel free to review in the context of that sim.

@terracoda If the current behavior looks correct, feel free to close this issue. If there are still problems, please discuss with @jessegreenberg.

@terracoda
Copy link

I tested MacOS 14.5 Sonoma with VoiceOver, and I think the AB Switch works great.

Sounds good in Chrome, too. Tested with Version 129.0.6668.90 (Official Build) (arm64).

I would love a dev version to try it out with a screen reader user. I'll ask on slack.

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

No branches or pull requests

4 participants