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

Voicing says some statements when turned off in toolbar #314

Closed
Nancy-Salpepi opened this issue Dec 23, 2022 · 4 comments
Closed

Voicing says some statements when turned off in toolbar #314

Nancy-Salpepi opened this issue Dec 23, 2022 · 4 comments

Comments

@Nancy-Salpepi
Copy link

Test device
MacBook Air (m1 chip)

Operating System
macOS 13.0.1

Browser
safari and chrome

Problem description
For phetsims/qa#868 and possibly related to phetsims/ratio-and-proportion#541 after turning off Voicing in the Toolbar I still hear:

  1. "hide toolbar"
  2. "Toolbar hidden"
  3. "Toolbar shown"
  4. "Close" when exiting a dialog

This doesn't happen with published GFLB.

Also--in Ratio and Proportion in Master, I will also hear the screen name and sim title when selecting a screen from the Home Screen. Ex: "Discover Screen. Ratio and Proportion"

Steps to reproduce

  1. In the Preferences Menu, turn on Voicing and check all Sim Voicing checkboxes
  2. Turn off Voicing in the toolbar
  3. Tab to the Hide Toolbar button
  4. Press the Hide Toolbar button with mouse or keyboard
  5. Tab to or click on the Keyboard Shortcuts Dialog
  6. Click on the Close button or press Return when the Close button has keyboard focus

Visuals

frictionVoicing.mov
Troubleshooting information: !!!!! DO NOT EDIT !!!!! Name: ‪Friction‬ URL: https://phet-dev.colorado.edu/html/friction/1.6.0-dev.28/phet/friction_all_phet.html?maskf Version: 1.6.0-dev.28 2022-12-20 18:30:08 UTC Features missing: applicationcache, applicationcache, touch Flags: pixelRatioScaling User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/108.0.0.0 Safari/537.36 Language: en-US Window: 1473x780 Pixel Ratio: 1.7999999523162842/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: 31 uniform: 1024 Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32) Max viewport: 16384x16384 OES_texture_float: true Dependencies JSON: {}
@jessegreenberg
Copy link
Contributor

I thought this would work, but I am confused. First, setting voicingVisible: false on the topLayer after the dialog closes does not prevent the "CloseButton" context response from being spoken and I don't know why. Also, I realized this fix would be bad because it would interrupt that context response always. But voicingVisible interrupt isn't working like I expected.

Index: js/Sim.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/Sim.ts b/js/Sim.ts
--- a/js/Sim.ts	(revision 1c4aa3e3661068b54d1e623a1c33cf134aa1188d)
+++ b/js/Sim.ts	(date 1672852199050)
@@ -820,8 +820,10 @@
       // pdom - modal dialogs should be the only readable content in the sim
       this.setPDOMViewsVisible( false );
 
-      // voicing - responses from Nodes hidden by the modal dialog should not voice.
+      // voicing - Responses from Nodes hidden by the modal dialog should not voice. Contents of the popup
+      // should voice if "Sim Voicing" is enabled.
       this.setNonModalVoicingVisible( false );
+      this.topLayer.voicingVisibleProperty.value = voicingManager.voicingFullyEnabledProperty.value;
     }
     if ( popup.layout ) {
       popup.layout( this.screenBoundsProperty.value! );
@@ -842,8 +844,9 @@
       if ( this.modalNodeStack.length === 0 ) {
 
         // After hiding all popups, Voicing becomes enabled for components in the simulation window only if
-        // "Sim Voicing" switch is on.
+        // "Sim Voicing" switch is on. C
         this.setNonModalVoicingVisible( voicingManager.voicingFullyEnabledProperty.value );
+        this.topLayer.voicingVisibleProperty.value = false;
 
         // pdom - when the dialog is hidden, make all ScreenView content visible to assistive technology
         this.setPDOMViewsVisible( true );

@jessegreenberg jessegreenberg self-assigned this Jan 4, 2023
@jessegreenberg
Copy link
Contributor

jessegreenberg commented Jan 4, 2023

OK, I found why voicingVisbile wasn't working as I expected and it was an explicit choice. Serves me right for not reading documentation...

    // We need an "unattached" utterance so that when the close button fires, hiding the close button, we still hear the context response.
    const contextResponseUtterance = new Utterance( {
      priority: Utterance.MEDIUM_PRIORITY
    } );

But this implementation will also make it impossible to prevent speaking "Close" with voicingVisible.

@jessegreenberg
Copy link
Contributor

OK, here is my status on this issue:

  1. The responses related to the toolbar are intentional when "Sim Voicing" is off. "Sim Voicing" currently turns off all Voicing for everything except the Toolbar, so we should hear voicing for the "Sim Voicing" toggle, the "Quick Info" buttons, and the toolbar push button.
  2. The "Close" button case is a bug. I think it is fixed by the above commit, but it required a change to the typing in utterance-queue (Can canAnnounceProperties be TReadOnlyProperty? utterance-queue#99)

@zepumph over to you to verify/review.

@zepumph
Copy link
Member

zepumph commented Jan 5, 2023

phetsims/ratio-and-proportion#546 closed and thus so can this!

@zepumph zepumph closed this as completed Jan 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

3 participants