zepumph
commented
2 months ago This is getting us in the right direction, but still has some work to do and review, like the TODOs in it.
```diff
Subject: [PATCH] opt out of overwriting range in MaterialControlNode when there is a density slider that already has a range, https://github.com/phetsims/density-buoyancy-common/issues/268
---
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 24f7b3b7029f560b811fff0e156742ee41b20980)
+++ b/js/common/view/MassView.ts (date 1722029607182)
@@ -23,6 +23,7 @@
import MassThreeMesh from './MassThreeMesh.js';
import { THREEModelViewTransform } from './DensityBuoyancyScreenView.js';
import sharedSoundPlayers from '../../../../tambo/js/sharedSoundPlayers.js';
+import GrabDragInteraction from '../../../../scenery-phet/js/accessibility/GrabDragInteraction.js';
const INVERT_Y_TRANSFORM = ModelViewTransform2.createSinglePointScaleInvertedYMapping( Vector2.ZERO, Vector2.ZERO, 1 );
@@ -110,30 +111,14 @@
focusable: true
} );
const endKeyboardInteraction = () => {
- keyboardDragListener.interrupt();
- mass.interruptedEmitter.removeListener( endKeyboardInteraction );
- this.focusablePath!.removeInputListener( blurListener );
+ grabDragInteraction.interrupt();
+
+ // TODO: is it best to be graceful here? OR try to figure out why we are removing the listener twice? https://github.com/phetsims/density-buoyancy-common/issues/209
+ mass.interruptedEmitter.hasListener( endKeyboardInteraction ) && mass.interruptedEmitter.removeListener( endKeyboardInteraction );
releaseSoundPlayer.play();
mass.endDrag();
};
- const blurListener = {
- blur: endKeyboardInteraction
- };
-
- this.focusablePath.addInputListener( {
- focus: () => {
-
- // We want the newer interaction to take precedent, so tabbing to the item should interrupt the previous mouse drag (if applicable).
- mass.userControlledProperty.value && mass.interruptedEmitter.emit();
-
- mass.interruptedEmitter.addListener( endKeyboardInteraction );
- this.focusablePath!.addInputListener( blurListener );
- grabSoundPlayer.play();
- mass.startDrag( mass.matrix.translation );
- }
- } );
-
const keyboardDragListener = new KeyboardDragListener( {
// In model units per second
dragSpeed: 3,
@@ -148,9 +133,27 @@
tandem: Tandem.OPT_OUT
} );
- this.focusablePath.addInputListener( keyboardDragListener );
+ const grabDragInteraction = new GrabDragInteraction( this.focusablePath, keyboardDragListener, {
+ grabCueOptions: { // TODO: Position the cue? https://github.com/phetsims/density-buoyancy-common/issues/209
+ centerX: 0,
+ bottom: 0
+ },
+ onGrab() {
+ // We want the newer interaction to take precedent, so tabbing to the item should interrupt the previous mouse drag (if applicable).
+ mass.userControlledProperty.value && mass.interruptedEmitter.emit();
+
+ mass.interruptedEmitter.addListener( endKeyboardInteraction );
+ grabSoundPlayer.play();
+ mass.startDrag( mass.matrix.translation );
+ },
+ onRelease() {
+ endKeyboardInteraction();
+ },
+ tandem: Tandem.OPT_OUT
+ } );
this.disposeEmitter.addListener( () => {
+ grabDragInteraction.dispose();
keyboardDragListener.dispose();
this.focusablePath && this.focusablePath.dispose();
} );
samreid
From https://github.com/phetsims/density-buoyancy-common/issues/190, because tabbing to the blocks makes the gravity no longer effect the block, a "tab then grab" interaction seems best. We also, in design meeting, discussed how this case would be similar to ESP where a skater is moving in a half pipe. Tabbing to the skater shouldn't pause the skater. This is a overload of the focus() event, and so GrabDragInteraction is likely a good solution here. Let's take a look.