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

Default Temperature values repeated in preferences menu with screen readers #359

Closed
Tracked by #978
Nancy-Salpepi opened this issue Aug 29, 2023 · 9 comments
Closed
Tracked by #978

Comments

@Nancy-Salpepi
Copy link

For phetsims/qa#973, when I change the default temperature units using the arrow keys, the screen reader (a11y view, VO, NVDA) will state the default temperature twice.

I know that for this release, the sim itself doesn't have interactive description, but according to phetsims/quadrilateral#441 I should be able to navigate the preferences menu with a screen reader when a sim has alt input.

Screenshot 2023-08-29 at 1 49 38 PM
@jessegreenberg
Copy link
Contributor

Hey @arouinfar, yes there was a decision to make Preferences contents screen reader accessible for all sims that support alternative input. I apologize that wasn't communicated well. Feel free to reassess that decision for this sim, I would hate to hold up the release if this is the only issue. It may be quick to fix though.

@arouinfar
Copy link
Contributor

Thanks @Nancy-Salpepi.

@jbphet in my opinion, this is something we can defer until we publish Greenhouse Effect with full description. The Default Temperature Units control is usable with a screen reader, even with the repeated responsive description.

Yes there was a decision to make Preferences contents screen reader accessible for all sims that support alternative input.

Thanks for confirming @jessegreenberg. I understand the motivation, but this seems somewhat arbitrary to me. I question the utility of state description in the Preferences dialog, but no state description elsewhere in the sim.

I apologize that wasn't communicated well. Feel free to reassess that decision for this sim.

Luckily @Nancy-Salpepi has a good memory. 😄

That said, this actually affects quite a few other sims in the pipeline such as Acid-Base Solutions, Center and Variability, and Kepler's Laws. None of these sims currently have any description content for the sim-specific Preferences.

If this is the new norm, there should be a PSA in dev meeting (and maybe on the dev-public Slack channel) and process documentation should be updated accordingly.

I would hate to hold up the release if this is the only issue. It may be quick to fix though.

Greenhouse Effect will need to go through a spot-check, so I'll defer to @jbphet to decide whether or not he wants to address it for this release or defer until we publish with full description.

@arouinfar arouinfar assigned jbphet and jessegreenberg and unassigned arouinfar Aug 29, 2023
@jessegreenberg
Copy link
Contributor

Sounds good! I just sent out a PSA about this to a the dev-public and design-sims slack channels.

@jessegreenberg jessegreenberg removed their assignment Aug 29, 2023
jbphet added a commit that referenced this issue Aug 31, 2023
@jbphet
Copy link
Contributor

jbphet commented Aug 31, 2023

I think this is a bug, but it has been a little misinterpreted. This is not a double announcement of the change to the default temperature units. Note that the message isn't about the temperature units, it is about actual temperature values. What is really going on here is that the change to the default temperature units is triggering a change to the chosen units in the first two screens, and their "alerters" announce the temperature using the new units. The only reason you don't see such an announcement for all three screens is that the third screen hasn't had much description added yet.

Another way this bug manifests is that if you change the default units when viewing one of the first two screens you'll also see two alerts.

I've addressed this by adding a feature to the gas concentration alerter that prevents it from making alerts if the Scenery node associated with it is not visible. This seems to work pretty well.

@jbphet
Copy link
Contributor

jbphet commented Aug 31, 2023

@jessegreenberg - The fix I've described in the previous comments and that can be seen in the commits for this issue may be something that should be generalized to the Alerter class. Since you're the primary author on Alerter, I think this should be your call. Please take a look and see if you think a way to enable/disable alerts in the base class is appropriate. If so, we should probably log a separate issue and I'd be happy to work with you to move this code into the base class.

@jbphet
Copy link
Contributor

jbphet commented Sep 1, 2023

I created a separate issue for the question of whether this should be generalized, see phetsims/scenery-phet#819. I'll unassign @jessegreenberg and will have QA check this issue on the next RC.

@jbphet
Copy link
Contributor

jbphet commented Sep 1, 2023

QA - feel free to close once you've verified that there are no more messages from the home screen and only a single message when the units are changed when viewing each of the first two screens.

@Nancy-Salpepi
Copy link
Author

In rc.2 with VO, I do not hear any temperature values when on the home screen and I only hear values once when switching between default units.
I will leave open for @KatieWoe to take a look with NVDA/JAWS.

@KatieWoe
Copy link
Contributor

KatieWoe commented Sep 5, 2023

NVDA looks ok to me.

@KatieWoe KatieWoe closed this as completed Sep 5, 2023
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

5 participants