phetsims / friction

"Friction" is an educational simulation in HTML5, by PhET Interactive Simulations.
http://phet.colorado.edu/en/simulation/friction
GNU General Public License v3.0
4 stars 6 forks source link

Voicing does not indicate whether checkboxes in Preferences Dialog are checked or not #289

Closed stemilymill closed 2 years ago

stemilymill commented 2 years ago

https://phet-dev.colorado.edu/html/friction/1.6.0-dev.24/phet/friction_all_phet.html Test device Dell laptop

Operating System Win10 21H2

Browser Chrome 99.0.4844.84

For https://github.com/phetsims/qa/issues/791. While navigating the preferences menu, voicing does not give any information about whether checkboxes are checked or not, except the different pitch sounds. This is not consistent with published versions of Gravity Force Lab: Basics and John Travoltage, which, when checked, indicate that this option is turned on and when then unchecked, indicate that this option is muted.

Current dev version of Friction:

https://user-images.githubusercontent.com/85511101/161363461-59a8aa62-822f-42c7-8ca5-8fc485ed469f.mp4

Published version of Gravity Force Lab: Basics

https://user-images.githubusercontent.com/85511101/161363466-3120804b-acb8-49d5-b2a3-ea0cd12d0682.mp4

(Please close if this is an intentional change)

Troubleshooting information:

Name: ‪Friction‬ URL: https://phet-dev.colorado.edu/html/friction/1.6.0-dev.24/phet/friction_all_phet.html Version: 1.6.0-dev.24 2022-03-30 15:06:07 UTC Features missing: applicationcache, applicationcache, touch Flags: pixelRatioScaling User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/99.0.4844.84 Safari/537.36 Language: en-US Window: 1493x775 Pixel Ratio: 1.125/1 WebGL: WebGL 1.0 (OpenGL ES 2.0 Chromium) GLSL: WebGL GLSL ES 1.0 (OpenGL ES GLSL ES 1.0 Chromium) Vendor: WebKit (WebKit WebGL) Vertex: attribs: 16 varying: 30 uniform: 4096 Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32) Max viewport: 32767x32767 OES_texture_float: true Dependencies JSON: {}

zepumph commented 2 years ago

Yes, what you experienced is indeed a bug, I can reproduce it on the dev version link you gave, but master seems to have it fixed. Can you confirm on master and feel free to close?

zepumph commented 2 years ago

Just a ping here as we move toward next QA test. Feel free to close.

stemilymill commented 2 years ago

(on master) There is now an indication of checking/unchecking the box, but now there is also repetition. https://user-images.githubusercontent.com/85511101/181342022-c024fca9-7649-4325-884f-a6824fceca7a.mp4

zepumph commented 2 years ago

That seems like the name + object reponse on selection. @terracoda can you please confirm that as the correct design here?

terracoda commented 2 years ago

Sorry, folks, the checkboxes in the Preferences menu are special and do not behave like check boxes in the actual sims.

It would be good if the common code for the Preferences menu (once it is created, if it is not created already) notes this difference.

Basically, because the preferences checkboses have to fully describe their function, their names are really long, so the context responses are all that is needed when toggled with the mouse. For keyboard, the long name is read out on focus, and then the context responses are read out when checked or unchecked.

terracoda commented 2 years ago

@zepumph, for these checkboxes, you want them implemented like they are explained in the Preferences design doc .

zepumph commented 2 years ago

@terracoda described the issue to me today. We don't want name responses spoken with the context responses for these checkboxes. The issue is that these are different than normal common code.

https://github.com/phetsims/joist/blob/e66dd022d515e057731fe4f7d34feb56742c1bbf/js/preferences/VoicingPanelSection.ts#L308-L330

Regression was when adding voicing to checkbox in https://github.com/phetsims/sun/commit/aee043c41128c37d0afd851ac43c6292e5799d96 (oops)

Most likely an option to conditionally supply name responses during context response would be best.

zepumph commented 2 years ago

Alright good deal. Implemented above. Please review. I no longer hear the name response with these checkbox context responses.

terracoda commented 2 years ago

The Extra Sounds checkbox also needs to be fixed. The name is part of the context response, so there should be no repetition of the name response. Ideally, this should be the case for all checkboxes in the Preferences dialog, but I do not want to say all as we will have new ones coming in new sims (e.g., RaP's Camera Input option). I'll have to look at those again.

Just so we understand why this is the case... In the Preferences Menu we are trying to help people set their preferences, we don't necessarily want to force them to try something out and then come back and turn it off. Thus, the labels and/or voice-able help text should make the actual setting or preference clear, and then the context response confirms the actual state of the setting or preference.

Not sure this will work for all checkboxes in the Preferences Menu, but that is why the ones we have now are a little different from typical sim checkboxes.

terracoda commented 2 years ago

@zepumph, please adjust the Extra Sounds checkbox, too.

zepumph commented 2 years ago

Done! Thanks for the review.