Add KeyboardDragListener to the blocks

[AgustinVallejo]

Related to phetsims/buoyancy#76 and phetsims/buoyancy#78

[AgustinVallejo]

Subject: [PATCH] Trying to add a KeyboardDraglistener
---
Index: js/common/model/Mass.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/model/Mass.ts b/js/common/model/Mass.ts
--- a/js/common/model/Mass.ts	(revision d3609154944458e566c55fa5cf0805fb2d8b6c4c)
+++ b/js/common/model/Mass.ts	(date 1708106535029)
@@ -633,6 +633,7 @@
    * Updates a current drag with a new 2d (x,y) model position.
    */
   public updateDrag( position: Vector2 ): void {
+    console.log( "Provided position", position )
     this.engine.updatePointerConstraint( this.body, position );
   }
 
Index: js/common/view/MassView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/MassView.ts b/js/common/view/MassView.ts
--- a/js/common/view/MassView.ts	(revision d3609154944458e566c55fa5cf0805fb2d8b6c4c)
+++ b/js/common/view/MassView.ts	(date 1708107638999)
@@ -16,8 +16,9 @@
 import DensityMaterials from './DensityMaterials.js';
 import MassLabelNode from './MassLabelNode.js';
 import MaterialView from './MaterialView.js';
-import { InteractiveHighlighting, Path } from '../../../../scenery/js/imports.js';
+import { InteractiveHighlighting, KeyboardDragListener, Path } from '../../../../scenery/js/imports.js';
 import { Shape } from '../../../../kite/js/imports.js';
+import { Vector2 } from '../../../../dot/js/imports.js';
 
 const TAG_SIZE = 0.03;
 export const TAG_OFFSET = 0.005;
@@ -118,6 +119,27 @@
       tagName: 'div',
       focusable: true
     } );
+
+    const keyboardDragListener = new KeyboardDragListener( {
+      dragDelta: 0.05,
+      shiftDragDelta: 0.025,
+      start: () => {
+        mass.startDrag( mass.matrix.translation );
+        this.screenView.currentMassProperty.value = mass;
+      },
+      drag: ( vectorDelta: Vector2 ) => {
+        mass.updateDrag( mass.matrix.translation.add( vectorDelta ) );
+      },
+      end: () => {
+        mass.endDrag();
+      }
+    } );
+
+    mass.interruptedEmitter.addListener( () => {
+      keyboardDragListener.interrupt();
+    } );
+
+    this.focusablePath.addInputListener( keyboardDragListener );
   }
 
   /**
Index: js/common/view/DensityBuoyancyScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/DensityBuoyancyScreenView.ts b/js/common/view/DensityBuoyancyScreenView.ts
--- a/js/common/view/DensityBuoyancyScreenView.ts	(revision d3609154944458e566c55fa5cf0805fb2d8b6c4c)
+++ b/js/common/view/DensityBuoyancyScreenView.ts	(date 1708107727877)
@@ -639,6 +639,7 @@
       }
 
       if ( massView ) {
+        massView.screenView = this;
         this.sceneNode.stage.threeScene.add( massView );
         this.massViews.push( massView );
 

[zepumph]

Got things working in a much better state here:

Subject: [PATCH] Im' just a patch (actually this time)
---
Index: js/common/view/MassView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/MassView.ts b/js/common/view/MassView.ts
--- a/js/common/view/MassView.ts	(revision 19facb1f24fb72788668e72458af1135ef692eda)
+++ b/js/common/view/MassView.ts	(date 1709319053551)
@@ -13,7 +13,7 @@
 import Material from '../model/Material.js';
 import DensityMaterials from './DensityMaterials.js';
 import MaterialView from './MaterialView.js';
-import { InteractiveHighlighting, Path } from '../../../../scenery/js/imports.js';
+import { InteractiveHighlighting, KeyboardDragListener, Path } from '../../../../scenery/js/imports.js';
 import { Shape } from '../../../../kite/js/imports.js';
 import MassTagView from './MassTagView.js';
 import ConvexHull2 from '../../../../dot/js/ConvexHull2.js';
@@ -90,6 +90,41 @@
 
     this.mass.transformedEmitter.addListener( this.positionListener );
     this.positionListener();
+
+    // TODO: mass && mass.canMove && !mass.userControlledProperty.value as a starting condition? Basically this is a multi touch/pointer problem?
+    // TODO: grab sound // Look into BASE, RAP, FEL for precedent
+    // TODO: release sound
+    // TODO: zoomed in dragging shouldn't get lost
+    // TODO: invert Y so up is up (not down)
+
+    this.focusablePath.addInputListener( {
+      focus: () => {
+        mass.startDrag( mass.matrix.translation );
+
+        // TODO: Provide Property via parameter?
+        this.screenView.currentMassProperty.value = mass;
+      },
+      blur: () => {
+        mass.endDrag();
+      }
+    } );
+
+    // TODO: Drag bounds
+    const keyboardDragListener = new KeyboardDragListener( {
+      dragDelta: 0.05, // TODO: a bit more tweaking probably
+      shiftDragDelta: 0.025,
+      drag: ( vectorDelta: Vector2 ) => {
+        mass.updateDrag( mass.matrix.translation.add( vectorDelta ) );
+      },
+      start: () => { }, // TODO: anything here?
+      end: () => { }
+    } );
+
+    mass.interruptedEmitter.addListener( () => {
+      keyboardDragListener.interrupt();
+    } );
+
+    this.focusablePath.addInputListener( keyboardDragListener );
   }
 
   public get tagHeight(): number | null {

[zepumph]

Working on this now.

[zepumph]

Next steps:

For design:

• We could tighten down the top dimension because for keyboard, you can drag up quite far, but going beyond the visible screen seems important for mouse, so I think this needs a bit of design.
• Tweak drag speeds
• Grab/release sounds on start/end or focus/blur? Let's discuss!

For devs:

• When playing with the blocks for a while, I got into a state where the Left/right arrows apply an upward force too, probably a bug. It'd be good to try to reproduce before closing this issue. UPDATE: I cannot reproduce in 3 minutes of testing.
• How should we guard with mass.userControlledProperty. This is a multitouch question that I don't think is totally straight forward. Likely we should interrupt the other dragging, since it is tied to focus, but we could also blur. That second option seems weird to me but let's talk.
• see PhET-iO Design Review buoyancy#51 (comment) For PhET-iO, do we want the keyboard drag listener instrumented? I don't think so, but I want to double check.
• I am curious if we should blur the object when the mass is interrupted. Probably not, but I want to ask.
• Grab/release sounds
• The dragging should stay on screen when zoomed in.
• Use dragSpeed instead of delta

[zepumph]

Alright. I think we are ready for design thoughts.

[zepumph]

• We could tighten down the top dimension because for keyboard, you can drag up quite far, but going beyond the visible screen seems important for mouse, so I think this needs a bit of design.

We will make it so that you cannot fully drag an object off the screen. Basically keep the bottom bound of the mass visible, with a bit of a margin. We should look at using dragBoundsProperty (custom per mass bounds change) OR just look at mapPosition.

• Tweak drag speeds

@DianaTavares will take a look and let me know if there should be adjustments. The current values are:

density-buoyancy-common/js/common/view/MassView.ts

Lines 137 to 138 in 056f682

	dragSpeed: 3,
	shiftDragSpeed: 0.5,

• Grab/release sounds on start/end or focus/blur? Let's discuss!

@DianaTavares expressed that she liked how often the grab/release sounds played in MSS, because it helped focus in on where the position of the bodies/vectors were. In buoyancy, it didn't sound as good because when you hear the "release" sound, it seems like the mass should be released and gravity should act on the mass again. Let's switch it to focus/blur and see how it feels. @DianaTavares please tell me if there is anything else after that swap.

[DianaTavares]

Tweak drag speeds

I find the velocity ok! I tred some challenges (like putting the block in specific positions) and with the normal and slow motion, it was possible and good!

[zepumph]

For the drag bounds, here is a first attempt at setting the top boundary. I will need to come back.

Subject: [PATCH] fdsafds
---
Index: js/buoyancy/model/BuoyancyExploreModel.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/buoyancy/model/BuoyancyExploreModel.ts b/js/buoyancy/model/BuoyancyExploreModel.ts
--- a/js/buoyancy/model/BuoyancyExploreModel.ts	(revision f6c9b791c7b8747d7f983de3236ef553863a2438)
+++ b/js/buoyancy/model/BuoyancyExploreModel.ts	(date 1709756325686)
@@ -43,7 +43,7 @@
 
     const blocksTandem = tandem.createTandem( 'blocks' );
 
-    this.primaryMass = Cube.createWithMass( this.engine, Material.WOOD, new Vector2( -0.2, 0.2 ), 2, {
+    this.primaryMass = Cube.createWithMass( this.engine, Material.WOOD, new Vector2( -0, 0 ), 2, {
       tag: MassTag.PRIMARY,
       tandem: blocksTandem.createTandem( 'blockA' )
     } );
Index: js/common/view/DensityBuoyancyScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/DensityBuoyancyScreenView.ts b/js/common/view/DensityBuoyancyScreenView.ts
--- a/js/common/view/DensityBuoyancyScreenView.ts	(revision f6c9b791c7b8747d7f983de3236ef553863a2438)
+++ b/js/common/view/DensityBuoyancyScreenView.ts	(date 1709756266559)
@@ -108,7 +108,7 @@
 
   private readonly debugView?: DebugView;
 
-  // Subtypes can provide their own values to control the barrier sizing
+  // Subtypes can provide their own values to control the barrier sizing. TODO: Rename to *PropertyProperty, https://github.com/phetsims/density-buoyancy-common/issues/98
   protected leftBarrierViewPointProperty: Property<TReadOnlyProperty<Vector2>>;
   protected rightBarrierViewPointProperty: Property<TReadOnlyProperty<Vector2>>;
 
@@ -746,15 +746,19 @@
     this.rightBarrierViewPointProperty = new Property<TReadOnlyProperty<Vector2>>( new DerivedProperty( [ this.visibleBoundsProperty ], visibleBounds => visibleBounds.rightCenter ), {
       tandem: Tandem.OPT_OUT
     } );
+    // model.invisibleBarrierBoundsProperty.debug( 'dragBounds' );
 
     const resizeBarrier = () => {
       const stage = this.sceneNode.stage;
       if ( stage.canvasWidth && stage.canvasHeight ) {
         const leftRay = this.sceneNode.getRayFromScreenPoint( this.localToGlobalPoint( this.leftBarrierViewPointProperty.value.value ) );
         const rightRay = this.sceneNode.getRayFromScreenPoint( this.localToGlobalPoint( this.rightBarrierViewPointProperty.value.value ) );
+        const topRay = this.sceneNode.getRayFromScreenPoint( this.localToGlobalPoint( this.visibleBoundsProperty.value.centerTop ) );
         const leftPoint = new Plane3( Vector3.Z_UNIT, 0.09 ).intersectWithRay( leftRay );
         const rightPoint = new Plane3( Vector3.Z_UNIT, 0.09 ).intersectWithRay( rightRay );
-        model.invisibleBarrierBoundsProperty.value = model.invisibleBarrierBoundsProperty.value.withMinX( leftPoint.x + 0.01 ).withMaxX( rightPoint.x - 0.01 );
+        const topPoint = new Plane3( Vector3.Z_UNIT, 0.09 ).intersectWithRay( topRay );
+          console.log( 'topPoint', topPoint );
+        model.invisibleBarrierBoundsProperty.value = model.invisibleBarrierBoundsProperty.value.withMinY( topPoint.y + 0.04 ).withMinX( leftPoint.x + 0.01 ).withMaxX( rightPoint.x - 0.01 );
       }
     };
 
Index: js/common/model/Mass.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/model/Mass.ts b/js/common/model/Mass.ts
--- a/js/common/model/Mass.ts	(revision f6c9b791c7b8747d7f983de3236ef553863a2438)
+++ b/js/common/model/Mass.ts	(date 1709754165499)
@@ -530,6 +530,13 @@
    */
   public abstract getLocalBounds(): Bounds3;
 
+  /**
+   * Get the bounds of this mass in parent coordinates.
+   */
+  public getBounds(): Bounds3 {
+    return this.getLocalBounds().shifted( this.matrix.translation.toVector3() );
+  }
+
   /**
    * Returns the cross-sectional area of this object at a given y level.
    */
Index: js/common/view/MassView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/MassView.ts b/js/common/view/MassView.ts
--- a/js/common/view/MassView.ts	(revision f6c9b791c7b8747d7f983de3236ef553863a2438)
+++ b/js/common/view/MassView.ts	(date 1709758806902)
@@ -21,12 +21,12 @@
 import ModelViewTransform2 from '../../../../phetcommon/js/view/ModelViewTransform2.js';
 import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js';
 import Bounds3 from '../../../../dot/js/Bounds3.js';
-import MappedProperty from '../../../../axon/js/MappedProperty.js';
 import Bounds2 from '../../../../dot/js/Bounds2.js';
 import Tandem from '../../../../tandem/js/Tandem.js';
 import MassTag from '../model/MassTag.js';
 import grabSoundPlayer from '../../../../tambo/js/shared-sound-players/grabSoundPlayer.js';
 import releaseSoundPlayer from '../../../../tambo/js/shared-sound-players/releaseSoundPlayer.js';
+import DerivedProperty from '../../../../axon/js/DerivedProperty.js';
 
 export type ModelPoint3ToViewPoint2 = ( point: Vector3 ) => Vector2;
 
@@ -76,7 +76,7 @@
 
       if ( this.focusablePath && !this.focusablePath.isDisposed ) {
 
-        const shiftedBbox = mass.getLocalBounds().shifted( position.toVector3() );
+        const shiftedBbox = mass.getBounds();
 
         // To support dragging while zoomed in, KeyboardDragListener will keep the position of the focusablePath in view.
         this.focusablePath.center = modelToViewPoint( shiftedBbox.center );
@@ -133,6 +133,18 @@
         }
       } );
 
+      const keyboardDRagBoundsProperty = new DerivedProperty( [
+        dragBoundsProperty,
+        mass.shapeProperty,
+        ...( mass.sizeProperty ? [ mass.sizeProperty ] as const : [] as const )
+      ], () => {
+        const generalDragBounds = Bounds2.create( dragBoundsProperty.value );
+        const massBounds = mass.getBounds();
+        return generalDragBounds.setMinY( massBounds.minY + 0.02 );
+      } );
+
+      console.log( keyboardDRagBoundsProperty.value );
+
       const keyboardDragListener = new KeyboardDragListener( {
         // In model units per second
         dragSpeed: 3,
@@ -140,7 +152,7 @@
 
         // This is needed for keyboard but not for mouse/touch because keyboard input applies deltas, not absolute positions
         transform: INVERT_Y_TRANSFORM,
-        dragBoundsProperty: new MappedProperty( dragBoundsProperty, { map: bounds3 => Bounds2.create( bounds3 ) } ),
+        dragBoundsProperty: keyboardDRagBoundsProperty,
         drag: ( vectorDelta: Vector2 ) => {
           mass.updateDrag( mass.matrix.translation.add( vectorDelta ) );
         },

[zepumph]

Alright. I got the drag bounds working based on the visibleBounds. I couldn't easily change it per mass shape, so I went with a constant based on the top of the mass with an offset. I like it quite a bit. @DianaTavares, let me know how you like the current grab/drag sounds and the dragging towards the top of the screen. Feel free to close.

[DianaTavares]

It's perfect! thanks!!

[phet-dev]

Reopening because there is a TODO marked for this issue.