jessegreenberg
commented
7 months ago I started a new class to brainstorm using KeyboardListener with KeyboardDragListener. I don't think this class will be used directly but its helping think thorugh the changes needed in KeyboardDragListener.
This approach uses KeyboardListener + CallbackTimer. There is some complexity around how to move when new keys are pressed while using dragDelta. Im saving this patch here because I want to try out a different method for that part.
```js
import { KeyboardListener, OneKeyStroke, scenery } from '../imports.js';
import Vector2 from '../../../dot/js/Vector2.js';
import assertMutuallyExclusiveOptions from '../../../phet-core/js/assertMutuallyExclusiveOptions.js';
import CallbackTimer from '../../../axon/js/CallbackTimer.js';
import TinyProperty from '../../../axon/js/TinyProperty.js';
import DerivedProperty from '../../../axon/js/DerivedProperty.js';
import Property from '../../../axon/js/Property.js';
export default class NewKeyboardDragListener extends KeyboardListener {
private leftKeyDownProperty: TinyProperty;
private rightKeyDownProperty: TinyProperty;
private upKeyDownProperty: TinyProperty;
private downKeyDownProperty: TinyProperty;
private shiftKeyDownProperty: TinyProperty;
private callbackTimer: CallbackTimer;
private useDragSpeed: boolean;
private positionProperty: Property | null;
private dragDelta: number;
private shiftDragDelta: number;
private moveOnHoldDelay: number;
public constructor( providedOptions ) {
assert && assertMutuallyExclusiveOptions( providedOptions, [ 'dragSpeed', 'shiftDragSpeed' ], [ 'dragDelta', 'shiftDragDelta' ] );
const options = _.merge( {
positionProperty: null,
dragDelta: 10,
shiftDragDelta: 5,
moveOnHoldDelay: 0,
moveOnHoldInterval: 400,
dragSpeed: 0,
shiftDragSpeed: 0
}, providedOptions );
// The callback timer should start
// when the first drag key is pressed and stop when the last drag key is released.
// For hotkeys, the timer will start when a hotkey combination is pressed (will be reported by the KeyboardListener).
// We need our own interval for smooth dragging across multiple keys.
// Use KeyboardListener for adding event listeners.
// Use stepTimer for updating the PositionProperty.
// use globalKeyStateTracker to watch the keystate.
super(
{
keys: [
'arrowLeft', 'arrowRight', 'arrowUp', 'arrowDown',
'w', 'a', 's', 'd',
'shift'
],
listenerFireTrigger: 'both',
allowExtraModifierKeys: true,
callback: ( event, keysPressed, listener ) => {
if ( listener.keysDown ) {
let immediateMoveDirection = null;
if ( keysPressed === 'shift' ) {
this.shiftKeyDownProperty.value = true;
}
if ( keysPressed === ( 'arrowLeft' ) || keysPressed === ( 'a' ) ) {
this.leftKeyDownProperty.value = true;
immediateMoveDirection = 'left';
}
if ( keysPressed === ( 'arrowRight' ) || keysPressed === ( 'd' ) ) {
this.rightKeyDownProperty.value = true;
immediateMoveDirection = 'right';
}
if ( keysPressed === ( 'arrowUp' ) || keysPressed === ( 'w' ) ) {
this.upKeyDownProperty.value = true;
immediateMoveDirection = 'up';
}
if ( keysPressed === ( 'arrowDown' ) || keysPressed === ( 's' ) ) {
this.downKeyDownProperty.value = true;
immediateMoveDirection = 'down';
}
}
else {
if ( keysPressed === ( 'arrowLeft' ) || keysPressed === ( 'a' ) ) {
this.leftKeyDownProperty.value = false;
}
if ( keysPressed === ( 'arrowRight' ) || keysPressed === ( 'd' ) ) {
this.rightKeyDownProperty.value = false;
}
if ( keysPressed === ( 'arrowUp' ) || keysPressed === ( 'w' ) ) {
this.upKeyDownProperty.value = false;
}
if ( keysPressed === ( 'arrowDown' ) || keysPressed === ( 's' ) ) {
this.downKeyDownProperty.value = false;
}
if ( keysPressed === ( 'shift' ) ) {
this.shiftKeyDownProperty.value = false;
}
}
}
}
);
// Since dragSpeed and dragDelta are mutually-exclusive drag implementations, a value for either one of these
// options indicates we should use a speed implementation for dragging.
this.useDragSpeed = options.dragSpeed > 0 || options.shiftDragSpeed > 0;
this.leftKeyDownProperty = new TinyProperty( false );
this.rightKeyDownProperty = new TinyProperty( false );
this.upKeyDownProperty = new TinyProperty( false );
this.downKeyDownProperty = new TinyProperty( false );
this.shiftKeyDownProperty = new TinyProperty( false );
this.positionProperty = options.positionProperty;
this.dragDelta = options.dragDelta;
this.shiftDragDelta = options.shiftDragDelta;
this.moveOnHoldDelay = options.moveOnHoldDelay;
const dragKeysDownProperty = new DerivedProperty( [ this.leftKeyDownProperty, this.rightKeyDownProperty, this.upKeyDownProperty, this.downKeyDownProperty ], ( left, right, up, down ) => {
return left || right || up || down;
} );
const interval = this.useDragSpeed ? 1000 / 60 : options.moveOnHoldInterval;
const delay = this.useDragSpeed ? 0 : options.moveOnHoldDelay;
this.callbackTimer = new CallbackTimer( {
delay: delay,
interval: interval,
callback: () => {
let deltaX = 0;
let deltaY = 0;
if ( this.useDragSpeed ) {
const dt = interval / 1000; // the interval in seconds
const delta = dt * ( this.shiftKeyDownProperty.value ? options.shiftDragSpeed : options.dragSpeed );
if ( this.leftKeyDownProperty.value ) {
deltaX -= delta;
}
if ( this.rightKeyDownProperty.value ) {
deltaX += delta;
}
if ( this.upKeyDownProperty.value ) {
deltaY += delta;
}
if ( this.downKeyDownProperty.value ) {
deltaY -= delta;
}
}
else {
const delta = this.shiftKeyDownProperty.value ? options.shiftDragDelta : options.dragDelta;
if ( this.leftKeyDownProperty.value ) {
deltaX -= delta;
}
if ( this.rightKeyDownProperty.value ) {
deltaX += delta;
}
if ( this.upKeyDownProperty.value ) {
deltaY += delta;
}
if ( this.downKeyDownProperty.value ) {
deltaY -= delta;
}
}
if ( options.positionProperty ) {
options.positionProperty.set( options.positionProperty.get().plus( new Vector2( deltaX, deltaY ) ) );
}
}
} );
// Keep a reference to the movement direction Property that initiated the movement to decide if we should move
// immediately in that direction. This is only important for !useDragSpeed.
let initiatingMovementProperty: Property | null = null;
const movementProperties = [ this.leftKeyDownProperty, this.rightKeyDownProperty, this.upKeyDownProperty, this.downKeyDownProperty ];
// When the drag keys are down, start the callback timer. When they are up, stop the callback timer.
dragKeysDownProperty.link( dragKeysDown => {
if ( dragKeysDown ) {
this.callbackTimer.start();
// If the delay is zero, we should NOT move immediately in the direction of the pressed key because
// the timer will start and fire right away. However, for all presses that are NOT the initiating movement
// key, we should move immediately in that direction.
if ( this.moveOnHoldDelay === 0 ) {
initiatingMovementProperty = movementProperties.find( property => property.value );
}
}
else {
this.callbackTimer.stop( false );
initiatingMovementProperty = null;
}
} );
// If not the shift key, we need to move immediately in that direction. Only important for !useDragSpeed.
// This is done here instead of firing the CallbackTimer listener because we only want to move immediately
// in the direction of the pressed key.
// TODO: If the delay is zero, we don't want to move immediately because the timer will start and fire
// right away. Is this behavior right???
// But then it fails to move right away for new key pressed
// Options: Require that the delay be non zero
// Only move right away if it is a new key press AND the press comes from a different key than the initial press
// What if we start over on the timer on every new press?
const addImmediateMoveListener = ( keyProperty, direction ) => {
keyProperty.link( keyDown => {
if ( keyDown && initiatingMovementProperty !== keyProperty ) {
console.log( this.callbackTimer.isRunning() );
this.moveImmediatelyInDirection( direction );
}
} );
};
addImmediateMoveListener( this.leftKeyDownProperty, 'left' );
addImmediateMoveListener( this.rightKeyDownProperty, 'right' );
addImmediateMoveListener( this.upKeyDownProperty, 'up' );
addImmediateMoveListener( this.downKeyDownProperty, 'down' );
}
private moveImmediatelyInDirection( direction: 'left' | 'right' | 'up' | 'down' ): void {
if ( this.positionProperty && !this.useDragSpeed ) {
const delta = this.shiftKeyDownProperty.value ? this.shiftDragDelta : this.dragDelta;
const position = this.positionProperty.get();
switch( direction ) {
case 'left':
this.positionProperty.set( position.plus( new Vector2( -delta, 0 ) ) );
break;
case 'right':
this.positionProperty.set( position.plus( new Vector2( delta, 0 ) ) );
break;
case 'up':
this.positionProperty.set( position.plus( new Vector2( 0, delta ) ) );
break;
case 'down':
this.positionProperty.set( position.plus( new Vector2( 0, -delta ) ) );
break;
}
}
}
}
scenery.register( 'NewKeyboardDragListener', NewKeyboardDragListener );
```
A new patch with some testing notes. This now supports most of the options of KeyboardDragListener. But I am trying out a new strategy for hotkeys. I think it would be best for KeyboardDragListener to no longer support hotkeys. If hotkeys are needed, just add a new KeyboardListener. The tricky part of this is to prevent dragging functionality if the hotkey overlaps with one of the keys used for dragging. ('j+w' does this in BASE). This patch gets around this by having the client declare a 'preventDefaultKeys' list that prevents dragging if any of those keys are down. Notes in this patch describe some other things I considered but that did not work.
```patch
Subject: [PATCH] Indicate that items have been sorted, see https://github.com/phetsims/scenery-phet/issues/815
---
Index: scenery/js/listeners/KeyboardListener.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/scenery/js/listeners/KeyboardListener.ts b/scenery/js/listeners/KeyboardListener.ts
--- a/scenery/js/listeners/KeyboardListener.ts (revision 3561b0fff76aa7cce63c26aed66c78b36044c15a)
+++ b/scenery/js/listeners/KeyboardListener.ts (date 1710544709942)
@@ -127,6 +127,8 @@
// Possible input types that decide when callbacks of the listener fire in response to input. See
// ListenerFireTrigger type documentation.
listenerFireTrigger?: ListenerFireTrigger;
+
+ allowExtraModifierKeys?: boolean;
};
type KeyGroup = {
@@ -180,6 +182,8 @@
private readonly _fireOnHoldDelay: number;
private readonly _fireOnHoldInterval: number;
+ private readonly _allowExtraModifierKeys: boolean;
+
// see options documentation
private readonly _global: boolean;
private readonly _handle: boolean;
@@ -200,7 +204,8 @@
listenerFireTrigger: 'down',
fireOnHold: false,
fireOnHoldDelay: 400,
- fireOnHoldInterval: 100
+ fireOnHoldInterval: 100,
+ allowExtraModifierKeys: false
}, providedOptions );
this._callback = options.callback;
@@ -212,6 +217,7 @@
this._fireOnHold = options.fireOnHold;
this._fireOnHoldDelay = options.fireOnHoldDelay;
this._fireOnHoldInterval = options.fireOnHoldInterval;
+ this._allowExtraModifierKeys = options.allowExtraModifierKeys;
this._activeKeyGroups = [];
@@ -354,8 +360,14 @@
// All keys are down.
const allKeys = [ ...downModifierKeys, finalDownKey ];
- // If there are any extra modifier keys down, the listener will not fire
- return globalKeyStateTracker.areKeysDownWithoutExtraModifiers( allKeys );
+ if ( this._allowExtraModifierKeys ) {
+ return globalKeyStateTracker.areKeysDown( allKeys );
+ }
+ else {
+
+ // If there are any extra modifier keys down, the listener will not fire
+ return globalKeyStateTracker.areKeysDownWithoutExtraModifiers( allKeys );
+ }
}
else {
return false;
@@ -374,8 +386,17 @@
if ( modifierKeysDown ) {
- // If there are any extra modifier keys down, the listener will not fire
- return globalKeyStateTracker.areKeysDownWithoutExtraModifiers( downModifierKeys );
+ if ( this._allowExtraModifierKeys ) {
+
+ // As long as modifier keys from the KeyGroup are down, the listener will fire - for this listener,
+ // its OK if OTHER modifier keys are down as well.
+ return true;
+ }
+ else {
+
+ // If there are any extra modifier keys down, the listener will not fire
+ return globalKeyStateTracker.areKeysDownWithoutExtraModifiers( downModifierKeys );
+ }
}
else {
return false;
Index: quadrilateral/js/quadrilateral/view/QuadrilateralScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/quadrilateral/js/quadrilateral/view/QuadrilateralScreenView.ts b/quadrilateral/js/quadrilateral/view/QuadrilateralScreenView.ts
--- a/quadrilateral/js/quadrilateral/view/QuadrilateralScreenView.ts (revision 4c92500974704504a0df6199b0828de47b2908ff)
+++ b/quadrilateral/js/quadrilateral/view/QuadrilateralScreenView.ts (date 1710544380303)
@@ -18,7 +18,7 @@
import QuadrilateralConstants from '../../QuadrilateralConstants.js';
import quadrilateral from '../../quadrilateral.js';
import QuadrilateralModel from '../model/QuadrilateralModel.js';
-import { VBox } from '../../../../scenery/js/imports.js';
+import { KeyboardListener, VBox } from '../../../../scenery/js/imports.js';
import QuadrilateralQueryParameters from '../QuadrilateralQueryParameters.js';
import QuadrilateralNode from './QuadrilateralNode.js';
import QuadrilateralSoundView from './sound/QuadrilateralSoundView.js';
@@ -42,6 +42,7 @@
import QuadrilateralModelViewTransform from './QuadrilateralModelViewTransform.js';
import QuadrilateralTangibleController from './prototype/QuadrilateralTangibleController.js';
import { SpeakableResolvedResponse } from '../../../../utterance-queue/js/ResponsePacket.js';
+import NewKeyboardDragListener from '../../../../scenery/js/listeners/NewKeyboardDragListener.js';
export default class QuadrilateralScreenView extends ScreenView {
private readonly model: QuadrilateralModel;
@@ -218,10 +219,65 @@
debugValuesPanel.leftTop = gridNode.leftTop.plusXY( 5, 5 );
+ // debugging
+ const testRectangle = new phet.scenery.Rectangle( 0, 0, 25, 25, { fill: 'red', tagName: 'button' } );
+ this.addChild( testRectangle );
+
+ const testPositionProperty = new phet.axon.Property( new Vector2( 0, 0 ) );
+
+ testPositionProperty.link( position => {
+
+ testRectangle.center = this.modelViewTransform.modelToViewPosition( position );
+ // testRectangle.center = position;
+ } );
+
+ const listener = new NewKeyboardDragListener( {
+ positionProperty: testPositionProperty,
+ transform: this.modelViewTransform,
+
+ // in view coordinates
+ // dragSpeed: 100,
+ // shiftDragSpeed: 50,
+
+ dragDelta: 25,
+ shiftDragDelta: 10,
+
+ preventDefaultKeys: [ 'j' ]
+
+ // keyboardDragDirection: 'upDown'
+ } );
+ testRectangle.addInputListener( listener );
+
+ // FOR NEXT TIME: how do we support this? I want to try to not have hotkeys in KeyboardListener - if you
+ // need special keys, make your own KeyboardListner. But when there is overlap in keys (like j+w), we want
+ // the KeyboardListener to prevent the drag from happening. How can that work?
+ // abort only works if the hotkeyListener is added BEFORE the drag listener (listener order dependent)
+ // I suppose we would interrupt the keyboardDragListener
+ // Running into problems with this because events start up again when keys are pressed down. Also tricky
+ // for dragDelta form (update happens right on down).
+
+ // SOME OPTIONS:
+ // - Maybe we need to change KeyboardDragListener so that it doesn't start dragging if extra (non-shift) modifier
+ // keys are pressed. That would mean if ANY key is pressed other than declared keys, there is no dragging.
+ // - Maybe KeyboardDragListener takes a list of "consumed" keys so that it doesn't start dragging if any of those
+ // keys are pressed.
+ // The Problem is that we need "extra" modifier keys to fire the callbacks (like shift) but we do NOT want
+ // extra keys like "j" to fire the callbacks.
+
+ const hotkeyListener = new KeyboardListener( {
+ keys: [ 'q', 'j+w' ],
+ callback: event => {
+ console.log( 'key pressed' );
+ }
+ } );
+
+ testRectangle.addInputListener( hotkeyListener );
+
+
//---------------------------------------------------------------------------------------------------------------
// Traversal order
//---------------------------------------------------------------------------------------------------------------
- this.pdomPlayAreaNode.pdomOrder = [ this.quadrilateralNode, shapeNameDisplay, resetShapeButton, shapeSoundsCheckbox ];
+ this.pdomPlayAreaNode.pdomOrder = [ testRectangle, this.quadrilateralNode, shapeNameDisplay, resetShapeButton, shapeSoundsCheckbox ];
this.pdomControlAreaNode.pdomOrder = [ visibilityControls, smallStepsLockToggleButton, resetAllButton, deviceConnectionParentNode ];
}
```
phet-dev
A next step from https://github.com/phetsims/scenery/issues/1520. Would be great to re-implement KeyboardDragListener with KeyboardListener. Biggest benefit will be that KeyboardDragListener uses the same API as KeyboardListener for hotkeys (they are different at the moment).