Changing Scenes shows the keyboard grab cue again

[Nancy-Salpepi]

Test device
MacBook Air M1 chip

Operating System
14.6.1

Browser
Safari 17.6

Problem description
For phetsims/qa#1136, the ‘space to grab or release’ message resets every time I switch scenes.

From Slack:

Michael Kauzmann
Thanks. This is a bug based on how often we are re-creating grabDragInteractions

Steps to reproduce
Here is an example:

1. On the Compare Screen, tab to, grab, and move both blocks
2. Tab to the Blocks Panel and select Same Volume or Same Density
3. Tab to either block -- the ‘space to grab or release’ message is there again.

Visuals

messageReappears.mp4

[samreid]

I'll take a look

[samreid]

Fixed in this patch. @zepumph can you please review? You can commit the patch if all is well:

Subject: [PATCH] Add tolerance to support inaccurate stepMasses, see https://github.com/phetsims/density-buoyancy-common/issues/351
---
Index: density-buoyancy-common/js/common/view/MassView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/density-buoyancy-common/js/common/view/MassView.ts b/density-buoyancy-common/js/common/view/MassView.ts
--- a/density-buoyancy-common/js/common/view/MassView.ts	(revision bc64def88a94349258c83284b9f4d88cab6020b2)
+++ b/density-buoyancy-common/js/common/view/MassView.ts	(date 1724366892121)
@@ -144,6 +144,9 @@
         tandem: Tandem.OPT_OUT
       } );
 
+      // eslint-disable-next-line @typescript-eslint/no-this-alias,consistent-this
+      const selfMassView = this;
+
       this.grabDragInteraction = new GrabDragInteraction( this.focusablePath, keyboardDragListener, {
         onGrab() {
 
@@ -153,13 +156,17 @@
           mass.interruptedEmitter.addListener( endKeyboardInteraction );
           grabSoundPlayer.play();
           mass.startDrag( mass.matrix.translation );
+
+          mass.numberOfKeyboardGrabs = selfMassView.grabDragInteraction!.numberOfKeyboardGrabs;
+          mass.numberOfGrabs = selfMassView.grabDragInteraction!.numberOfGrabs;
         },
         onRelease() {
           endKeyboardInteraction();
         },
-        tandem: Tandem.OPT_OUT
+        tandem: Tandem.OPT_OUT,
+        numberOfKeyboardGrabs: mass.numberOfKeyboardGrabs,
+        numberOfGrabs: mass.numberOfGrabs
       } );
-
 
       const myListener = () => {
 
Index: density-buoyancy-common/js/common/model/Mass.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/density-buoyancy-common/js/common/model/Mass.ts b/density-buoyancy-common/js/common/model/Mass.ts
--- a/density-buoyancy-common/js/common/model/Mass.ts	(revision bc64def88a94349258c83284b9f4d88cab6020b2)
+++ b/density-buoyancy-common/js/common/model/Mass.ts	(date 1724366467380)
@@ -175,6 +175,13 @@
 
   public readonly resetEmitter = new Emitter();
 
+  // MassViews and their GrabDragInteractions are recreated, the mass stores relevant information from one instantiation to
+  // another. These quantities do not need to be PhET-iO stateful, because they are related to usability and
+  // accessibility, and when studio launches a Standard PhET-iO Wrapper, these values should be zeroed out, not preserved
+  // in the state
+  public numberOfKeyboardGrabs = 0;
+  public numberOfGrabs = 0;
+
   protected constructor( engine: PhysicsEngine, providedOptions: MassOptions ) {
 
     const options = optionize<MassOptions, SelfOptions, PhetioObjectOptions>()( {
@@ -582,6 +589,9 @@
 
     this.resetPosition();
 
+    this.numberOfGrabs = 0;
+    this.numberOfKeyboardGrabs = 0;
+
     this.resetEmitter.emit();
   }
 
Index: scenery-phet/js/accessibility/GrabDragInteraction.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/scenery-phet/js/accessibility/GrabDragInteraction.ts b/scenery-phet/js/accessibility/GrabDragInteraction.ts
--- a/scenery-phet/js/accessibility/GrabDragInteraction.ts	(revision 0b0527fc6e01e949fa67392c1cea3c77aca67b39)
+++ b/scenery-phet/js/accessibility/GrabDragInteraction.ts	(date 1724367495647)
@@ -145,6 +145,11 @@
 
   // Like keyboardHelpText but when supporting gesture interactive description.
   gestureHelpText?: PDOMValueType;
+
+  // When a view is dynamically or lazily created for a persistent model, we may need to indicate that it has previously
+  // been interacted with. Hence you can pass non-zero values to indicate that the view has been interacted with.
+  numberOfGrabs?: number;
+  numberOfKeyboardGrabs?: number;
 };
 
 type ParentOptions = EnabledComponentOptions;
@@ -187,11 +192,11 @@
   // The number of times the component has been picked up for dragging, regardless
   // of pickup method for things like determining content for "hints" describing the interaction
   // to the user
-  private numberOfGrabs = 0;
+  public numberOfGrabs: number;
 
   // The number of times this component has been picked up with a keyboard specifically to provide hints specific
   // to alternative input.
-  private numberOfKeyboardGrabs = 0;
+  public numberOfKeyboardGrabs: number;
 
   // The aria-describedby association object that will associate "grabbable" with its
   // help text so that it is read automatically when the user finds it. This reference is saved so that
@@ -264,6 +269,9 @@
         phetioFeatured: false
       },
 
+      numberOfGrabs: 0,
+      numberOfKeyboardGrabs: 0,
+
       // {Tandem} - For instrumenting
       tandem: Tandem.REQUIRED
     }, providedOptions );
@@ -362,8 +370,8 @@
     this.onDraggable = secondPassOptions.onDraggable;
     this.addAriaDescribedbyPredicate = secondPassOptions.addAriaDescribedbyPredicate;
     this.supportsGestureDescription = secondPassOptions.supportsGestureDescription;
-    this.numberOfGrabs = 0;
-    this.numberOfKeyboardGrabs = 0;
+    this.numberOfGrabs = secondPassOptions.numberOfGrabs;
+    this.numberOfKeyboardGrabs = secondPassOptions.numberOfKeyboardGrabs;
 
     // set the help text, if provided - it will be associated with aria-describedby when in the "grabbable" state
     this.node.descriptionContent = this.supportsGestureDescription ? secondPassOptions.gestureHelpText : secondPassOptions.keyboardHelpText;

[samreid]

@zepumph and I fixed this in the commits. @Nancy-Salpepi can you please test on phettest?

[Nancy-Salpepi]

I can still reproduce on main using the original steps in #354 (comment)

2 other examples:

1. On the Applications Screen: tab to, grab, and move the bottle
2. Release the bottle
3. Tab to the applicationModeRadioButtonGroup select boat and then switch back to bottle
4. shift + tab back to the bottle --message is there again

__

1. On the Shapes Screen: tab to, grab, and move Object A
2. Release Object A
3. Tab to the shapeComboBox and select a different shape
4. shift + tab back to Object A --message is there again

[samreid]

Some observations:

1. The instructions in the top comment show a message on 2A after grabbing item 1A because they are different items. We are following the precedent in balloons and static electricity where each item has its own independent "press to grab" message. We weren't sure that is best, but followed the precedent.
2. Regarding the applications screen, I can reproduce the problem following the instructions in the preceding comment. However, if clicking the applicationModeRadioButtonGroup with the mouse, the problem does not occur. So something different is happening with mouse vs keyboard for that interaction. It will require more investigation. UPDATE: In several experiments it did preserve the hidden message through just keyboard interaction. So maybe there is a randomness or race condition element?

[samreid]

The problem for the Applications screen is that protected override getMassViewFromMass( mass: Mass ): MassView { is called from the super constructor. Hence, at runtime this.bottleView is assigned at this point. However, then the sub-constructor is called which had this code:

  private bottleView: BottleView | null = null;
  private boatView: BoatView | null = null;

which nulls it back out. Hence the BottleView and BoatView are created twice.

[zepumph]

Discussed during standup today, and we think this is a good general design question to ask about grab drag interaction. The general question is, "how much does the user need to see the grab cue?"

• Does the user need to see this cue fresh for each screen?
• Does the user need to see this cue fresh for each new mass type (scale/boat/block)
• Does the user need to see this cue fresh for each new mass (left block/right block)

Let's make a common code issue and ask some experts

[zepumph]

We discussed at a designers meeting with JG SR MK in attendance as well. We noted that it isn't necessarily automatic that cueing grab for a block would translate to cueing grab for the scale. Furthermore, more cueing is better than less cueing, so long as it isn't too distracting or obscuring.

We decided to try out the following:

1. We need to add drag cues for 2d motion (4 cardinal direction arrows)
2. Show the cues (for grab and for drag) per object. This means the scale has its own logic, and so does each "individual block".
3. "Individual Block" means something different than what is in the code, we mean each conceptual block, for example on the compare screen, 1a/2a/3a blocks are all the same conceptually, and so only need to have the cues shown once for all of them. This mimics (kinda) BASE where changing the charge view doesn't show "new" balloons with the same cue.

[zepumph]

This bug will be designed and sorted out over in #368.

[zepumph]

Design has been solidified over in #368. Closing