Add default grab/release sounds to MeasuringTapeNode.

[pixelzoom]

Shouldn't grab/release sounds be included for MeasuringTapeNode in the new PDL sim? See https://github.com/phetsims/projectile-data-lab/issues/270.

Adding support involves a quick conversion to RichDragListener and RichKeyboardDragListener. Patch included below.

Adding options to be propagated to the 4 listeners would be a bit more work, especially given the odd definition of MeasuringTapeNodeOptions.keyboardDragListenerOptions.

MeasuringTapeNode patch

```diff Subject: [PATCH] TODO https://github.com/phetsims/gas-properties/issues/213 --- Index: js/MeasuringTapeNode.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/MeasuringTapeNode.ts b/js/MeasuringTapeNode.ts --- a/js/MeasuringTapeNode.ts (revision f0e0b58e6ca99f49e6f7ec8afc85af96091697c0) +++ b/js/MeasuringTapeNode.ts (date 1711058647463) @@ -35,6 +35,8 @@ import TProperty from '../../axon/js/TProperty.js'; import DerivedStringProperty from '../../axon/js/DerivedStringProperty.js'; import Tandem from '../../tandem/js/Tandem.js'; +import RichKeyboardDragListener from '../../sun/js/RichKeyboardDragListener.js'; +import RichDragListener from '../../sun/js/RichDragListener.js'; export type MeasuringTapeUnits = { name: string; @@ -364,7 +366,7 @@ }; // Drag listener for base - this.baseDragListener = new DragListener( { + this.baseDragListener = new RichDragListener( { tandem: options.tandem?.createTandem( 'baseDragListener' ), start: event => { baseStart(); @@ -389,7 +391,7 @@ this.baseImage.addInputListener( this.baseDragListener ); // Drag listener for base - const baseKeyboardDragListener = new KeyboardDragListener( { + const baseKeyboardDragListener = new RichKeyboardDragListener( { tandem: options.tandem?.createTandem( 'baseKeyboardDragListener' ), positionProperty: this.basePositionProperty, transform: this.modelViewTransformProperty, @@ -410,7 +412,7 @@ let tipStartOffset: Vector2; // Drag listener for tip - const tipDragListener = new DragListener( { + const tipDragListener = new RichDragListener( { tandem: options.tandem?.createTandem( 'tipDragListener' ), start: event => { @@ -437,7 +439,7 @@ } ); tip.addInputListener( tipDragListener ); - const tipKeyboardDragListener = new KeyboardDragListener( { + const tipKeyboardDragListener = new RichKeyboardDragListener( { tandem: options.tandem?.createTandem( 'tipKeyboardDragListener' ), positionProperty: this.tipPositionProperty, dragBoundsProperty: options.isTipDragBounded ? this.dragBoundsProperty : null, ```

[pixelzoom]

Adding options to be propagated to the 4 listeners would be a bit more work, especially given the odd definition of MeasuringTapeNodeOptions.keyboardDragListenerOptions.

Suggested improvement to options below. There is 1 usage of keyboardDragListenerOptions for MeasuringTapeNode, in keplers-laws SolarSystemCommonMeasuringTapeNode.

-  keyboardDragListenerOptions?: {
-    baseDragSpeed?: number;
-    baseShiftDragSpeed?: number;
-    tipDragSpeed?: number;
-    tipShiftDragSpeed?: number;
-  };
+  baseDragListenerOptions?: RichDragListenerOptions;
+  tipDragListenerOptions?: RichDragListenerOptions;
+  baseKeyboardDragListenerOptions?: RichKeyboardDragListenerOptions; 
+  tipKeyboardDragListenerOptions?: RichKeyboardDragListenerOptions; 

[pixelzoom]

Work completed in the above commits. @samreid would you mind taking a look? Reminder that this impacts https://github.com/phetsims/projectile-data-lab/issues/270.

[samreid]

This worked well in my testing. In the code review I observed these options:

      baseKeyboardDragListenerOptions: {
        dragSpeed: KEYBOARD_DRAG_SPEED,
        shiftDragSpeed: KEYBOARD_DRAG_SPEED / 2
      },
      tipKeyboardDragListenerOptions: {
        dragSpeed: KEYBOARD_DRAG_SPEED,
        shiftDragSpeed: 150
      }

Should both shiftDragSpeed be KEYBOARD_DRAG_SPEED / 2? I also observed https://github.com/phetsims/scenery/issues/1622 but that is being tracked separately.

[pixelzoom]

Should both shiftDragSpeed be KEYBOARD_DRAG_SPEED / 2?

These are the values that were in the code, I did not change them. I don't know the motivation for having a different default shiftDragSpeed for the base vs tip. You'll need to ask @AgustinVallejo, who set those values in https://github.com/phetsims/scenery-phet/commit/f72cf3e6a94e8d968a4458c3652f66b3ecac2dac. Unfortunately that commit does not reference a GitHub issue. I suspect that the values are specific to Kepler's Laws, which uses MeasuringTapeNode, and which @AgustinVallejo was working on around that time (7/18/23).

[samreid]

I'll self-unassign until we hear from @AgustinVallejo

[AgustinVallejo]

If I recall correctly, we lowered the default tip drag speed from KEYBOARD_DRAG_SPEED / 2 (300) to 150 for the user to have finer control over the tip. I'd say that for consistency it could be better for both tip and base to be KEYBOARD_DRAG_SPEED / 4. Is that reasonable? Let me know and I could make the change, or feel free as well. @samreid @pixelzoom

I could see reasons not to increase the tip's shiftDragSpeed, but decreasing the base's shiftDragSpeed seems a correct approach.

[pixelzoom]

I don't have a sim that is using MeasuringTapeNode, and I took this issue on as a courtesy when reviewing PDL, which has a MeasuringTapeNode. My task here was to add the grab/release sounds, not decide on whether/how to change the drag speeds, and I don't have time to get involved in that aspect. I think it would be more appropriate to sort this out for PDL, so assigning back to @samreid.

[AgustinVallejo]

Spoke with @samreid on Slack. He said:

1/4 seems better to me. I agree they should be consistent.

Above commit addresses this.

[pixelzoom]

I'll also note that this issue is about sounds, which are independent of drag speeds. So changing shiftDragSpeed feels like it should be in a new issue.

[samreid]

It looks like everything here is completed. Nice work @pixelzoom and @AgustinVallejo. Closing.