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 says some statements when turned off in toolbar #314

Closed Nancy-Salpepi closed 1 year ago

Nancy-Salpepi commented 1 year ago

Test device MacBook Air (m1 chip)

Operating System macOS 13.0.1

Browser safari and chrome

Problem description For https://github.com/phetsims/qa/issues/868 and possibly related to https://github.com/phetsims/ratio-and-proportion/issues/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

https://user-images.githubusercontent.com/87318828/209359864-157ef504-ecc9-47f6-be66-4a9296c729e8.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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 (https://github.com/phetsims/utterance-queue/issues/99)

@zepumph over to you to verify/review.

zepumph commented 1 year ago

https://github.com/phetsims/ratio-and-proportion/issues/546 closed and thus so can this!