phetsims / ratio-and-proportion

"Ratio and Proportion" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
1 stars 4 forks source link

Hand control is not paused when a dialog is open #509

Closed KatieWoe closed 2 years ago

KatieWoe commented 2 years ago

Test device Dell Operating System Win 11 Browser Chrome Problem description For https://github.com/phetsims/qa/issues/831. When a dialog, such as the preferences menu, is open, typically the controls to the sim behind it pause. With the hand controls, this is not the case. This can lead to some odd seeming behavior. In particular, I had voicing for the sim occur as I was trying to change the voice type, since my hands were briefly on screen. Steps to reproduce

  1. Go to the sim with hand controls enabled, and enter either screen.
  2. Open the preferences dialog
  3. Move hands around

Visuals stillcontrol

Troubleshooting information:

!!!!! DO NOT EDIT !!!!! Name: ‪Ratio and Proportion‬ URL: https://phet-dev.colorado.edu/html/ratio-and-proportion/1.2.0-dev.31/phet/ratio-and-proportion_all_phet.html?cameraInput=hands&showVideo Version: 1.2.0-dev.31 2022-08-24 16:50:40 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/104.0.0.0 Safari/537.36 Language: en-US Window: 1280x649 Pixel Ratio: 1.5/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

It would be nice to brainstorm some suggestions about how to hand this generally. Perhaps with @jessegreenberg.

jessegreenberg commented 2 years ago

I think it is OK that hands continue to control the sim while the dialog is open - sims generally continue to animate while dialogs are open. If we decide to change this I think that will be a larger decision for PhET.

I had voicing for the sim occur as I was trying to change the voice type, since my hands were briefly on screen.

I expected sim voicing to stop while the dialog is opened, maybe there is a loose end in joist about this. If this part is fixed that might be enough for this issue.

zepumph commented 2 years ago

I thought that this code would ensure that dialogs silence screen view voicing, but perhaps we need to check on that.

https://github.com/phetsims/ratio-and-proportion/blob/8bec57a73ad22e225638671c019eeae1550f2d32/js/common/view/RAPScreenView.ts#L293-L295

I'll investigate.

zepumph commented 2 years ago

This patch works, but I believe we should discuss before I commit. @jessegreenberg, we were using voicingVisible for toggling "sim voicing", but not with dialogs.

Seems potentially like we forgot to get here in https://github.com/phetsims/scenery/issues/1300 even though that was part of that work (not really sure, I can't remember). Anyways! What do you think about this? It should apply to all modal entities, even though we just have dialogs at this point.

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 0f2668684a76091bc771b646ff70faf66643f965)
+++ b/js/Sim.ts (date 1664991281019)
@@ -820,6 +820,7 @@
     if ( isModal ) {
       this.rootNode.interruptSubtreeInput();
       this.modalNodeStack.push( popup );
+      this.setNonModelVoicingVisible( false );
     }
     if ( popup.layout ) {
       popup.layout( this.screenBoundsProperty.value! );
@@ -836,6 +837,7 @@
     assert && assert( popup && this.modalNodeStack.includes( popup ) );
     assert && assert( this.topLayer.hasChild( popup ), 'popup was not shown' );
     if ( isModal ) {
+      this.setNonModelVoicingVisible( true );
       this.modalNodeStack.remove( popup );
     }
     this.topLayer.removeChild( popup );
@@ -1041,13 +1043,21 @@
    * only Toolbar content is announced.
    */
   public setSimVoicingVisible( visible: boolean ): void {
+    this.setNonModelVoicingVisible( visible );
+    this.topLayer && this.topLayer.setVoicingVisible( visible );
+  }
+
+  /**
+   * Sets voicingVisible on all elements "behind" the modal node stack. In this case, voicing should not work for those
+   * components when set to false.
+   * @param visible
+   */
+  public setNonModelVoicingVisible( visible: boolean ): void {
     for ( let i = 0; i < this.screens.length; i++ ) {
-      this.screens[ i ].view.voicingVisible = visible;
+      this.screens[ i ].view.voicingVisible = visible; // home screen is the first item, if created
     }

     this.navigationBar.voicingVisible = visible;
-    this.topLayer && this.topLayer.setVoicingVisible( visible );
-    this.homeScreen && this.homeScreen.view.setVoicingVisible( visible );
   }
 }

Also, if this works, do we need to do this also for the phet menu or combo box list being open?

zepumph commented 2 years ago

@KatieWoe, we have fixed this now. Please confirm on master and let me know how it goes!

KatieWoe commented 2 years ago

Voicing playing when moving hands while the dialog is open no longer occurs on master. Movement itself does still happen. If this is the desired behavior I believe this is fixed.