Re-implement KeyboardDragListener with KeyboardListener

[jessegreenberg]

A next step from #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).

[jessegreenberg]

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.

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<OneKeyStroke[]> {
  private leftKeyDownProperty: TinyProperty<boolean>;
  private rightKeyDownProperty: TinyProperty<boolean>;
  private upKeyDownProperty: TinyProperty<boolean>;
  private downKeyDownProperty: TinyProperty<boolean>;
  private shiftKeyDownProperty: TinyProperty<boolean>;

  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<boolean> | 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.

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<Keys extends readonly OneKeyStroke[]> = {
@@ -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 ];
   }
 

[jessegreenberg]

Some notes from a discussion with @zepumph and @jonathanolson:

Design for a registry of used keys for KeyboardListener

1. Need to support overlapping keys when shared between two listeners. Sometimes both should fire. Sometimes
   one should prevent the other.

2. Support assertions if you mistakenly overload keys.

• Developer error detection is important.

• We also want to know what hotkeys are used as a design tool.

• We want to be able to 'override' keys sometimes and so we will need to disable the assertion sometimes.

• Concerned about performance for this. Inspecting scene graph every event sounds expensive!

  • Can run behind assert or assertSlow and it could be OK. If we need this map for some user feature, that
    would be a different story.
  • Could also run per-frame checks (each frame, grab all listeners and make sure they don't conflict).
  • When we get a keyboard event, we would need to scan through all listeners for order, specificity, enabled state,
    and that is where we could run into performance problems.
  • For the focused Node, make a check for all listeners to look for hotkey conflicts.

3. Factor keys into a single place so used keys are defined once and used multiple places (in the listener, in the help dialog).

Question: Can we automatically populate contents for the KeyboardHelpDialog?
Maybe we have a new design where there is a popup for specific Nodes. "How do I use this thing?" and it displays
the key commands for it.

Possibly define contents in package.json (or something) entries. This way they could be pulled into sim code for
each listener and into the help dialog.

{
usedKeys: [
{
keys: [ 'w', 'a', 's', 'd' ],
description: 'Move the battery.'
},
{
keys: [ 'enter', 'space' ],
description: 'Press buttons!'
}
]
}

We should think about options for KeyboardListener that allow for flexibility in how "modifier" keys impact
when the callback fires.

We should look at how other OS or platforms handle this problem.
-- Look at the API for keyboard input. MacOS/Windows keyboard shortcuts develop guide.
- https://developer.apple.com/documentation/SwiftUI/KeyboardShortcut
-- Google Docs that sometimes apply shortcuts and sometimes don't.
-- Unity/pixi examples of keyboard input support.

Design
https://sashika.medium.com/j-k-or-how-to-choose-keyboard-shortcuts-for-web-applications-a7c3b7b408ee

Decide on terminology (keyboard shortcut vs hotkey)

Big take away:

Since keyboard events happen infrequently (much less frequently than mouse/touch events), this is not as performance intensive. We can inspect a (pruned) scene graph every event to get information about current keyboard listeners. From this, we should be able to derive the assertions we need as well as prioritizing listeners and letting one handle/abort others.

Edit: Patch with progress on this. THis is actually working quite well (as long as performance is accetpable). I need to switch to an emergent task but will come back to this:

Subject: [PATCH] Indicate that items have been sorted, see https://github.com/phetsims/scenery-phet/issues/815
---
Index: joist/js/preferences/PreferencesTabs.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/joist/js/preferences/PreferencesTabs.ts b/joist/js/preferences/PreferencesTabs.ts
--- a/joist/js/preferences/PreferencesTabs.ts	(revision 95c9675f79c426063c491c41a3c5e31ef50c83ff)
+++ b/joist/js/preferences/PreferencesTabs.ts	(date 1710950464597)
@@ -119,7 +119,7 @@
 
     // pdom - keyboard support to move through tabs with arrow keys
     const keyboardListener = new KeyboardListener( {
-      keys: [ 'arrowRight', 'arrowLeft', 'arrowUp', 'arrowDown' ],
+      keys: [ 'arrowRight', 'arrowLeft' ],
       callback: ( event, keysPressed, listener ) => {
         assert && assert( event, 'event is required for this listener' );
         const sceneryEvent = event!;
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 e23d7d1460d99885c2335c39a5fbe7199e67616c)
+++ b/scenery/js/listeners/KeyboardListener.ts	(date 1710963908363)
@@ -64,6 +64,10 @@
 // Allowed keys are the keys of the EnglishStringToCodeMap.
 type AllowedKeys = keyof typeof EnglishStringToCodeMap;
 
+// Array methods that help us determine if keys overlap between two KeyboardListeners
+const findDuplicates = ( arr: string[] ): string[] => [ ...new Set( arr.filter( ( item, index ) => arr.indexOf( item ) !== index ) ) ];
+const isArraySubset = ( subsetArray: string[], supersetArray: string[] ): boolean => subsetArray.every( value => supersetArray.includes( value ) );
+
 export type OneKeyStroke = `${AllowedKeys}` |
   `${ModifierKey}+${AllowedKeys}` |
   `${ModifierKey}+${ModifierKey}+${AllowedKeys}`;
@@ -78,6 +82,15 @@
 // - 'both': Callbacks fire on both press and release of keys.
 type ListenerFireTrigger = 'up' | 'down' | 'both';
 
+type ListenerOverlapBehavior = 'most_specific' | 'allow' | 'error';
+
+type KeyGroupWithListener = {
+  listener: KeyboardListener<OneKeyStroke[]>;
+  keyGroup: KeyGroup<OneKeyStroke[]>;
+};
+
+const deferredKeyboardListenersMap = new Map<string, KeyboardListener<OneKeyStroke[]>[]>();
+
 type SelfOptions<Keys extends readonly OneKeyStroke[ ]> = {
 
   // The keys that need to be pressed to fire the callback. In a form like `[ 'shift+t', 'alt+shift+r' ]`. See top
@@ -128,6 +141,10 @@
   // Possible input types that decide when callbacks of the listener fire in response to input. See
   // ListenerFireTrigger type documentation.
   listenerFireTrigger?: ListenerFireTrigger;
+
+  allowExtraModifierKeys?: boolean;
+
+  listenerOverlapBehavior?: ListenerOverlapBehavior;
 };
 
 export type KeyboardListenerOptions<Keys extends readonly OneKeyStroke[]> = SelfOptions<Keys> & EnabledComponentOptions;
@@ -140,6 +157,9 @@
   // Contains the triggering key for the listener. One of these keys must be pressed to activate callbacks.
   keys: string[];
 
+  // All of the keys that must be pressed for the callback to fire (modifier keys up to the leading key)
+  allKeyCodes: string[];
+
   // All keys in this KeyGroup using the readable form
   naturalKeys: Keys[number];
 
@@ -183,8 +203,16 @@
   private readonly _fireOnHoldDelay: number;
   private readonly _fireOnHoldInterval: number;
 
+  private readonly _allowExtraModifierKeys: boolean;
+
+  public readonly listenerOverlapBehavior: ListenerOverlapBehavior;
+
+  // (scenery-internal) - Scenery has found that another KeyboardListener will fire for the same keys. It will
+  // defer this listener if the other key has more specific keys.
+  public _deferred = false;
+
   // see options documentation
-  private readonly _global: boolean;
+  public readonly global: boolean;
   private readonly _handle: boolean;
   private readonly _abort: boolean;
 
@@ -203,7 +231,9 @@
       listenerFireTrigger: 'down',
       fireOnHold: false,
       fireOnHoldDelay: 400,
-      fireOnHoldInterval: 100
+      fireOnHoldInterval: 100,
+      allowExtraModifierKeys: false,
+      listenerOverlapBehavior: 'most_specific'
     }, providedOptions );
 
     super( providedOptions );
@@ -217,12 +247,14 @@
     this._fireOnHold = options.fireOnHold;
     this._fireOnHoldDelay = options.fireOnHoldDelay;
     this._fireOnHoldInterval = options.fireOnHoldInterval;
+    this._allowExtraModifierKeys = options.allowExtraModifierKeys;
+    this.listenerOverlapBehavior = options.listenerOverlapBehavior;
 
     this._activeKeyGroups = [];
 
     this.keysDown = false;
 
-    this._global = options.global;
+    this.global = options.global;
     this._handle = options.handle;
     this._abort = options.abort;
 
@@ -244,7 +276,9 @@
    * Mostly required to fire with CallbackTimer since the callback cannot take arguments.
    */
   public fireCallback( event: SceneryEvent<KeyboardEvent> | null, keyGroup: KeyGroup<Keys> ): void {
-    this._callback( event, keyGroup.naturalKeys, this );
+    if ( !this._deferred ) {
+      this._callback( event, keyGroup.naturalKeys, this );
+    }
   }
 
   /**
@@ -369,8 +403,15 @@
       // 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 {
+
+        // TODO: If 'deferred' works, then we can remove this because that behavior for extra modifier keys
+        // would presumably correctly defer the right listener.
+        return globalKeyStateTracker.areKeysDownWithoutExtraModifiers( allKeys );
+      }
     }
     else {
       return false;
@@ -389,8 +430,18 @@
 
     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 {
+
+        // TODO: If 'deffered' works, then we can remove this because that behavior for extra modifier keys
+        // would presumably correctly defer the right listener.
+        return globalKeyStateTracker.areKeysDownWithoutExtraModifiers( downModifierKeys );
+      }
     }
     else {
       return false;
@@ -429,7 +480,7 @@
    * added to the global key events. Target will be the Node, Display, or Pointer this listener was added to.
    */
   public keydown( event: SceneryEvent<KeyboardEvent> ): void {
-    if ( !this._global ) {
+    if ( !this.global ) {
       this.handleKeyDown( event );
     }
   }
@@ -439,7 +490,7 @@
    * added to the global key events. Target will be the Node, Display, or Pointer this listener was added to.
    */
   public keyup( event: SceneryEvent<KeyboardEvent> ): void {
-    if ( !this._global ) {
+    if ( !this.global ) {
       this.handleKeyUp( event );
     }
   }
@@ -449,7 +500,7 @@
    * Event has no target.
    */
   public globalkeydown( event: SceneryEvent<KeyboardEvent> ): void {
-    if ( this._global ) {
+    if ( this.global ) {
       this.handleKeyDown( event );
     }
   }
@@ -459,7 +510,7 @@
    * Event has no target.
    */
   public globalkeyup( event: SceneryEvent<KeyboardEvent> ): void {
-    if ( this._global ) {
+    if ( this.global ) {
       this.handleKeyUp( event );
     }
   }
@@ -578,6 +629,7 @@
         keys: keys,
         modifierKeys: modifierKeys,
         naturalKeys: naturalKeys,
+        allKeyCodes: [ ...modifierKeys.flat(), ...keys ],
         timer: timer
       };
       return keyGroup;
@@ -586,6 +638,98 @@
     return keyGroups;
   }
 
+  public static inspectKeyboardListeners( keyboardListeners: KeyboardListener<OneKeyStroke[]>[], event: KeyboardEvent ): void {
+    const prepareKeyGroupsWithListener: ( () => KeyGroupWithListener[] ) = () => {
+      return keyboardListeners.reduce( ( accumulator, listener ) => {
+        if ( listener.listenerOverlapBehavior !== 'allow' ) {
+          const keyGroups = listener._keyGroups;
+          keyGroups.forEach( keyGroup => {
+            accumulator.push( {
+              listener,
+              keyGroup
+            } );
+          } );
+        }
+        return accumulator;
+      }, [] ) as { listener: KeyboardListener<OneKeyStroke[]>; keyGroup: KeyGroup<OneKeyStroke[]> }[];
+    };
+
+    // Compares each listener with every other, only visiting each pair once
+    const checkListenerOverlaps = ( keyGroupsWithListeners: KeyGroupWithListener[] ) => {
+      for ( let i = 0; i < keyGroupsWithListeners.length; i++ ) {
+        for ( let j = i + 1; j < keyGroupsWithListeners.length; j++ ) {
+          compareAndHandleKeyGroups( keyGroupsWithListeners[ i ], keyGroupsWithListeners[ j ] );
+        }
+      }
+    };
+
+    const compareAndHandleKeyGroups = ( objectA: KeyGroupWithListener, objectB: KeyGroupWithListener ) => {
+      const aSplitKeys = objectA.keyGroup.naturalKeys.split( '+' );
+      const bSplitKeys = objectB.keyGroup.naturalKeys.split( '+' );
+
+      const [ shorterObject, longerObject ] = aSplitKeys.length < bSplitKeys.length ? [ objectA, objectB ] : [ objectB, objectA ];
+      const [ shorterKeys, longerKeys ] = aSplitKeys.length < bSplitKeys.length ? [ aSplitKeys, bSplitKeys ] : [ bSplitKeys, aSplitKeys ];
+
+      const shorterIsSubset = isArraySubset( shorterKeys, longerKeys );
+      if ( shorterIsSubset ) {
+        handleOverlapBasedOnBehavior( shorterObject, longerObject );
+      }
+    };
+
+    const handleOverlapBasedOnBehavior = ( shorterObject: KeyGroupWithListener, longerObject: KeyGroupWithListener ) => {
+      if ( longerObject.listener.areKeysDownForListener( longerObject.keyGroup ) ) {
+        const behavior = shorterObject.listener.listenerOverlapBehavior;
+        const shorterKeys = shorterObject.keyGroup.naturalKeys;
+        const longerKeys = longerObject.keyGroup.naturalKeys;
+
+        switch( behavior ) {
+          case 'error':
+            assert && assert( false, `The keys ${shorterKeys} are a subset of the keys ${longerKeys}` );
+            break;
+          case 'most_specific':
+            console.log( 'deferring', shorterKeys, 'for', longerKeys, 'code', event.code );
+            shorterObject.listener._deferred = true;
+            updateDeferredListenersMap( event.code, shorterObject.listener );
+            break;
+          case 'allow':
+            // Nothing to do.
+            break;
+        }
+      }
+    };
+
+    const updateDeferredListenersMap = ( keyCode: string, listener: KeyboardListener<OneKeyStroke[]> ) => {
+      if ( deferredKeyboardListenersMap.has( keyCode ) ) {
+        deferredKeyboardListenersMap.get( keyCode )!.push( listener );
+      }
+      else {
+        deferredKeyboardListenersMap.set( keyCode, [ listener ] );
+      }
+    };
+
+    const naturalKeysWithListener = prepareKeyGroupsWithListener();
+    checkListenerOverlaps( naturalKeysWithListener );
+
+    const allNaturalKeys = naturalKeysWithListener.map( entry => entry.keyGroup.naturalKeys );
+    const duplicateKeys = findDuplicates( allNaturalKeys );
+    assert && assert( duplicateKeys.length === 0, `At least two KeyboardListeners are going to fire at the same time from the ${duplicateKeys.join( ', ' )} keys(s)` );
+  }
+
+  public static undeferKeyboardListeners( keyCode: string ): void {
+    if ( deferredKeyboardListenersMap.has( keyCode ) ) {
+      deferredKeyboardListenersMap.get( keyCode )!.forEach( listener => { listener._deferred = false; } );
+      deferredKeyboardListenersMap.delete( keyCode );
+
+      console.log( 'undeferred', keyCode );
+    }
+
+    if ( !globalKeyStateTracker.keysAreDown() ) {
+      assert && assert( deferredKeyboardListenersMap.size === 0,
+        'There are deferred listeners but no keys are down! There is a bug or memory leak.'
+      );
+    }
+  }
+
   /**
    * Returns the first EnglishStringToCodeMap that corresponds to the provided event.code. Null if no match is found.
    * Useful when matching an english string used by KeyboardListener to the event code from a
Index: scenery/js/input/Input.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/scenery/js/input/Input.ts b/scenery/js/input/Input.ts
--- a/scenery/js/input/Input.ts	(revision e23d7d1460d99885c2335c39a5fbe7199e67616c)
+++ b/scenery/js/input/Input.ts	(date 1710960845491)
@@ -172,7 +172,7 @@
 import EventType from '../../../tandem/js/EventType.js';
 import NullableIO from '../../../tandem/js/types/NullableIO.js';
 import NumberIO from '../../../tandem/js/types/NumberIO.js';
-import { BatchedDOMEvent, BatchedDOMEventCallback, BatchedDOMEventType, BrowserEvents, Display, EventContext, EventContextIO, Mouse, Node, PDOMInstance, PDOMPointer, PDOMUtils, Pen, Pointer, scenery, SceneryEvent, SceneryListenerFunction, SupportedEventTypes, TInputListener, Touch, Trail, WindowTouch } from '../imports.js';
+import { BatchedDOMEvent, BatchedDOMEventCallback, BatchedDOMEventType, BrowserEvents, Display, EventContext, EventContextIO, KeyboardListener, Mouse, Node, OneKeyStroke, PDOMInstance, PDOMPointer, PDOMUtils, Pen, Pointer, scenery, SceneryEvent, SceneryListenerFunction, SupportedEventTypes, TInputListener, Touch, Trail, WindowTouch } from '../imports.js';
 import PhetioObject, { PhetioObjectOptions } from '../../../tandem/js/PhetioObject.js';
 import IOType from '../../../tandem/js/types/IOType.js';
 import ArrayIO from '../../../tandem/js/types/ArrayIO.js';
@@ -794,9 +794,27 @@
       sceneryLog && sceneryLog.Input && sceneryLog.Input( `keydown(${Input.debugText( null, context.domEvent )});` );
       sceneryLog && sceneryLog.Input && sceneryLog.push();
 
-      this.dispatchGlobalEvent<KeyboardEvent>( 'globalkeydown', context, true );
+      const keyboardListeners: KeyboardListener<OneKeyStroke[]>[] = [];
+      this.recursiveScanForGlobalKeyboardListeners( this.rootNode, keyboardListeners );
 
+      // also add any listeners along the trail
       const trail = this.getPDOMEventTrail( context.domEvent, 'keydown' );
+      if ( trail ) {
+        const nodes = trail.nodes;
+        nodes.forEach( node => {
+
+          // skip the global listeners, they will have been added by the above scan
+          const nodeKeyboardListeners = node.inputListeners.filter( listener => listener instanceof KeyboardListener && !listener.global );
+
+          // @ts-expect-error
+          keyboardListeners.push( ...nodeKeyboardListeners );
+        } );
+      }
+
+      KeyboardListener.inspectKeyboardListeners( keyboardListeners, context.domEvent );
+
+      this.dispatchGlobalEvent<KeyboardEvent>( 'globalkeydown', context, true );
+
       trail && this.dispatchPDOMEvent<KeyboardEvent>( trail, 'keydown', context, true );
 
       this.dispatchGlobalEvent<KeyboardEvent>( 'globalkeydown', context, false );
@@ -823,6 +841,8 @@
 
       this.dispatchGlobalEvent<KeyboardEvent>( 'globalkeyup', context, false );
 
+      KeyboardListener.undeferKeyboardListeners( context.domEvent.code );
+
       sceneryLog && sceneryLog.Input && sceneryLog.pop();
     }, {
       phetioPlayback: true,
@@ -848,6 +868,25 @@
     } );
   }
 
+  public recursiveScanForGlobalKeyboardListeners( node: Node, listeners: KeyboardListener<OneKeyStroke[]>[] ): KeyboardListener<OneKeyStroke[]>[] {
+
+    // The KeyboardListener will be assigned to a Node
+    if ( !node.isDisposed && node.isVisible() && node.isInputEnabled() && node.isPDOMVisible() ) {
+      // Reverse iteration follows the z-order from "visually in front" to "visually in back" like normal dipatch
+      for ( let i = node._children.length - 1; i >= 0; i-- ) {
+        this.recursiveScanForGlobalKeyboardListeners( node._children[ i ], listeners );
+      }
+
+      // if the node has a KeyboardListener that is global, add it to the list
+      const globalKeyboardListeners = node.inputListeners.filter( listener => listener instanceof KeyboardListener && listener.global );
+
+      // @ts-expect-error
+      listeners.push( ...globalKeyboardListeners );
+    }
+
+    return listeners;
+  }
+
   /**
    * Called to batch a raw DOM event (which may be immediately fired, depending on the settings). (scenery-internal)
    *
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 1710788487076)
@@ -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,77 @@
 
     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.
+
+    // NOTE: Consider create a subclass of KeyboardListener that DOES support hotkeys so that old usages
+    // are easy to track down.
+
+    // A concept of precedence between listeners?? A priority between then.
+    //  - An explicit priority.
+    //  - More "specific" key groups take priority.
+    //  - If ANY keys outside of the "specific" keys are pressed, the listener does not fire.
+      // - Except for maybe modifier keys like shift, control, etc.
+    //  - Priority + a handle/abort system so that it is up to the client to decide if a listener should take precedence.
+    // - Just use the scene graph to support this? Add the hotkeylistener first or higher up the scene graph
+    //   so that it always gets the first event and can prevent the drag listener.
+
+    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 ];
   }
 
Index: quadrilateral/js/quadrilateral/view/QuadrilateralNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/quadrilateral/js/quadrilateral/view/QuadrilateralNode.ts b/quadrilateral/js/quadrilateral/view/QuadrilateralNode.ts
--- a/quadrilateral/js/quadrilateral/view/QuadrilateralNode.ts	(revision 4c92500974704504a0df6199b0828de47b2908ff)
+++ b/quadrilateral/js/quadrilateral/view/QuadrilateralNode.ts	(date 1710890557868)
@@ -183,6 +183,7 @@
     this.addInputListener( new KeyboardListener( {
       keys: [ 'shift' ],
       listenerFireTrigger: 'both',
+      listenerOverlapBehavior: 'allow',
       callback: ( event, keysPressed, listener ) => {
         this.model.minorIntervalsFromGlobalKeyProperty.value = listener.keysDown;
       },

Scratch with a new KeyboardDragListener

import { EnglishStringToCodeMap, globalKeyStateTracker, 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';
import Transform3 from '../../../dot/js/Transform3.js';

export default class NewKeyboardDragListener extends KeyboardListener<OneKeyStroke[]> {
  private leftKeyDownProperty: TinyProperty<boolean>;
  private rightKeyDownProperty: TinyProperty<boolean>;
  private upKeyDownProperty: TinyProperty<boolean>;
  private downKeyDownProperty: TinyProperty<boolean>;
  private shiftKeyDownProperty: TinyProperty<boolean>;

  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: 500,
      moveOnHoldInterval: 400,

      keyboardDragDirection: 'both',

      transform: null,

      dragSpeed: 0,
      shiftDragSpeed: 0

    }, providedOptions );

    let keys: OneKeyStroke[];
    if ( options.keyboardDragDirection === 'both' ) {
      keys = [ 'arrowLeft', 'arrowRight', 'arrowUp', 'arrowDown', 'w', 'a', 's', 'd', 'shift' ];
    }
    else if ( options.keyboardDragDirection === 'leftRight' ) {
      keys = [ 'arrowLeft', 'arrowRight', 'a', 'd', 'shift' ];
    }
    else if ( options.keyboardDragDirection === 'upDown' ) {
      keys = [ 'arrowUp', 'arrowDown', 'w', 's', 'shift' ];
    }
    else {
      throw new Error( 'unhandled keyboardDragDirection' );
    }

    // 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: keys,
        listenerFireTrigger: 'both',
        allowExtraModifierKeys: true,
        callback: ( event, keysPressed, listener ) => {
          if ( listener.keysDown ) {
            if ( keysPressed === 'shift' ) {
              this.shiftKeyDownProperty.value = true;
            }
            if ( keysPressed === ( 'arrowLeft' ) || keysPressed === ( 'a' ) ) {
              this.leftKeyDownProperty.value = true;
            }
            if ( keysPressed === ( 'arrowRight' ) || keysPressed === ( 'd' ) ) {
              this.rightKeyDownProperty.value = true;
            }
            if ( keysPressed === ( 'arrowUp' ) || keysPressed === ( 'w' ) ) {
              this.upKeyDownProperty.value = true;
            }
            if ( keysPressed === ( 'arrowDown' ) || keysPressed === ( 's' ) ) {
              this.downKeyDownProperty.value = true;
            }
          }
          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;

        let delta = 0;
        if ( this.useDragSpeed ) {

          // TODO: Is there a better way to get this dt? Its nice that setInterval accounts for 'leftover' time
          // so that errors dont accumulate. But it would be nice to have a way to get the actual dt.
          const dt = interval / 1000; // the interval in seconds
          delta = dt * ( this.shiftKeyDownProperty.value ? options.shiftDragSpeed : options.dragSpeed );
        }
        else {
          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 ) {
          let vectorDelta = new Vector2( deltaX, deltaY );

          // to model coordinates
          if ( options.transform ) {
            const transform = options.transform instanceof Transform3 ? options.transform : options.transform.value;
            vectorDelta = transform.inverseDelta2( vectorDelta );
          }

          options.positionProperty.set( options.positionProperty.get().plus( vectorDelta ) );
        }
      }
    } );

    // When the drag keys are down, start the callback timer. When they are up, stop the callback timer.
    dragKeysDownProperty.link( dragKeysDown => {
      if ( dragKeysDown ) {

        if ( this.useDragSpeed ) {
          this.callbackTimer.start();
        }

        // this is where we call the optional start callback
      }
      else {

        // when keys are no longer pressed, stop the timer
        this.callbackTimer.stop( false );

        // this is where we call the optional end callback
      }
    } );


    // If using discrete steps, the CallbackTimer is restarted every key press
    if ( !this.useDragSpeed ) {

      // If not the shift key, we need to move immediately in that direction. Only important for !useDragSpeed.
      // This is done oustide of the the CallbackTimer listener because we only want to move immediately
      // in the direction of the pressed key.
      const addStartTimerListener = keyProperty => {
        keyProperty.link( keyDown => {
          if ( keyDown ) {

            // restart the callback timer
            this.callbackTimer.stop( false );
            this.callbackTimer.start();

            if ( this.moveOnHoldDelay > 0 ) {

              // fire right away if there is a delay - if there is no delay the timer is going to fire in the next
              // animation frame and so it would appear that the object makes two steps in one frame
              this.callbackTimer.fire();
            }
          }
        } );
      };
      addStartTimerListener( this.leftKeyDownProperty );
      addStartTimerListener( this.rightKeyDownProperty );
      addStartTimerListener( this.upKeyDownProperty );
      addStartTimerListener( this.downKeyDownProperty );
    }
  }

  public override interrupt(): void {
    super.interrupt();

    // Setting these to false doesn't work with the interrupt strategy. They are set to false and the super
    // is interrupted. Then we will get a new keydown event in the super, which will call subclass calbacks,
    // and set these to true again in a later event.
    this.leftKeyDownProperty.value = false;
    this.rightKeyDownProperty.value = false;
    this.upKeyDownProperty.value = false;
    this.downKeyDownProperty.value = false;
    this.shiftKeyDownProperty.value = false;

    this.callbackTimer.stop( false );

  }
}

scenery.register( 'NewKeyboardDragListener', NewKeyboardDragListener );

Important notes from this change set:

1. areKeysDownWithoutExtraModifierKeys can be removed now with this "deferred" strategy. That was our old method of preventing behavior when different combinations of modifier keys were down.
2. Should array utility functions like findDuplicates and isArraySubset live anywhere else?
3. Look at usages of canNodeReceivePDOMInput. Better way to do that with the trail?
4. I am worried about performance. Looking for a way to remove the scan every keydown event. But a new listener might be added in response to the keypress? Or what if in a key press a Node supports PDOM input? Here is a patch with this idea, but I don't think it would work. Note that this is the SECOND full scene graph traversal now, we already traverse every keydown event to dispatch global events.

Subject: [PATCH] Indicate that items have been sorted, see https://github.com/phetsims/scenery-phet/issues/815
---
Index: js/listeners/KeyboardListener.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/listeners/KeyboardListener.ts b/js/listeners/KeyboardListener.ts
--- a/js/listeners/KeyboardListener.ts	(revision fb065d278be67cdfa9445109a021cb53ca954c38)
+++ b/js/listeners/KeyboardListener.ts	(date 1711064472228)
@@ -714,6 +714,13 @@
     );
   }
 
+  /**
+   * Returns true if there are already deferred KeyboardListeners for the provided keyCode.
+   */
+  public static deferringKeyboardListenersForCode( keyCode: string ): boolean {
+    return deferredKeyboardListenersMap.has( keyCode );
+  }
+
   /**
    * Returns the first EnglishStringToCodeMap that corresponds to the provided event.code. Null if no match is found.
    * Useful when matching an english string used by KeyboardListener to the event code from a
Index: js/input/Input.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/input/Input.ts b/js/input/Input.ts
--- a/js/input/Input.ts	(revision fb065d278be67cdfa9445109a021cb53ca954c38)
+++ b/js/input/Input.ts	(date 1711064424829)
@@ -794,17 +794,24 @@
       sceneryLog && sceneryLog.Input && sceneryLog.Input( `keydown(${Input.debugText( null, context.domEvent )});` );
       sceneryLog && sceneryLog.Input && sceneryLog.push();
 
-      // Look for all global KeyboardListeners that are on Nodes that can receive input events. We will inspect
-      // this list for overlapping keys.
-      const keyboardListeners: KeyboardListener<OneKeyStroke[]>[] = [];
-      this.recursiveScanForGlobalKeyboardListeners( this.rootNode, keyboardListeners );
+      const trail = this.getPDOMEventTrail( context.domEvent, 'keydown' );
+
+      // If already deferring KeyboardListeners for this code, don't scan for overlapping keys.
+      // TODO: I am worried about this...What if we add a new listener in response to the key press?
+      //  Looking for another way to improve performance. This scan can potentially happen very often.
+      if ( !KeyboardListener.deferringKeyboardListenersForCode( context.domEvent.code ) ) {
+
+        // Look for all global KeyboardListeners that are on Nodes that can receive input events. We will inspect
+        // this list for overlapping keys.
+        const keyboardListeners: KeyboardListener<OneKeyStroke[]>[] = [];
+        this.recursiveScanForGlobalKeyboardListeners( this.rootNode, keyboardListeners );
 
-      // Also add any local KeyboardListeners along the trail.
-      const trail = this.getPDOMEventTrail( context.domEvent, 'keydown' );
-      trail && this.scanTrailForKeyboardListeners( trail, keyboardListeners );
+        // Also add any local KeyboardListeners along the trail.
+        trail && this.scanTrailForKeyboardListeners( trail, keyboardListeners );
 
-      // Inspect listeners for overlapping keys.
-      KeyboardListener.inspectKeyboardListeners( keyboardListeners, context.domEvent );
+        // Inspect listeners for overlapping keys.
+        KeyboardListener.inspectKeyboardListeners( keyboardListeners, context.domEvent );
+      }
 
       this.dispatchGlobalEvent<KeyboardEvent>( 'globalkeydown', context, true );
 

4.5) What if scenery only dispatched one keydown event on the first press? Deviate from DOM behavior, but that should work fine for our KeyboardListener and KeyStateTracker.

5. Is there a way to prune the scene graph walk further? LIke a way to determine if any descendants have listeners?

Some notes from reviewing with JO and MK:

• "deferred" is confusing, many will think of this as a delay instead of preventing behavior.

• Have a specific set of modifiers, and any extras will fire.

• One thought: Push back on design: The set of standard system modifier keys should be specified for a hotkey. Where if you press more or fewer, they do not fire. This is backed by a lot of user intuition.

• What if we could dynamically define modifier keys. So if any extra modifier keys are pressed, it would not fire the callback.

• Maybe we have an option that could 'ignore custom modifier' keys, and then listeners would still fire if we need to.

• Please don't put number keys as modifier keys.

• We never ignore "Modifier Keys". This way they have a consistent definition and behavior.

• Consider a HotkeyListener that extends KeyboardListener. KeyboardListener is more general and covers whatever you might need for keyboard input. HotkeyListener is more specific and could include the complexity described here.

• For performance, avoid closures in global scans. Use Set instead of array.

• // TODO: Do you need a PDOM instance to receive global keydown events? Is that how it works, what do we want?
  // We are worried about KeyboardListeners not being fired because global dispatch uses visual trails.
  // You could potentially look at the accessible instance tree? This could simplify things a lot.
  // Would require that (global) KeyboardListener are only added to Nodes with a tagName.
  // We would need a way to get the "effective" children with accessible content.
  // We have this in 'getEffectiveChildren'.
  // Consider adding a pdomAudit assertion that makes sure that all nodes with a KeyboardListener also have a tag name.
  // TODO: We want a better understanding of how the system works. For example, when do we use visual vs PDOM trails?
  Some terminology:

• "Modifier Keys" should only be standard browser modifier keys (alt, shift, ctrl, meta)

• "Modifier Keys" change the behavior of the key. If you include a modifier key, it should prevent the firing behavior of a key. For example, "ctrl+t" will NOT fire a listener on "t".

• "Hotkey" - A combination of keys that must be held down with a final key that triggers the behavior. It is possible that a hotkey has only one key.

• "Custom Modifier Key" - You can opt in to having a custom letter behaving like a modifier key. For example add "J" for "j+w". Adding a new modifier key would be done at a global level for all KeyboardListeners.

Latest takeaways from the above discussion:

• We do not want to continue with the "deferred" approach. Instead, we are going to have specific behavior for Modifier Keys. See definitions above.
• As such, global scan is only needed for assertions and performance is less of a concern here.
• A conversation from inspecting the global scan:
  • Worried that global scan is using visual Nodes. We need to use PDOM tree Nodes instead. Use getEffectiveChildren().
  • Make a new issue to use 'getEffectiveChildren' in the global scan and event dispatch.
  • This means that only accessible Nodes can receive global keyboard events. Document this and add assertion to pdomAudit.
  • This means that 'global scans' for event dispatch can be ignored if the Display is not accessible.

[jessegreenberg]

First implementation from notes above in a4c22d0. I found that I did need a way to opt out of this new "Modifier key" behavior for KeyboardDragListener for SOME keys, but still want it for other keys. For example, I needed the KeyboardDragListener to keep firing when 'shift' was pressed, but it should NOT fire when Custom Modifier Key was added.

This is just an initial commit, more thinking and testing to be done.

[jessegreenberg]

I tried to write unit tests to make sure that KeyboardListener overlap problems were caught. But my tests did not work. I ran into a problem where using window.dispatchEvent fires the event synchronously, and creates a new stack for the try/catch. And so it was impossible to verify assertions from KeyboardListener in response to events. I need to bail on this but here is my patch:

Subject: [PATCH] Indicate that items have been sorted, see https://github.com/phetsims/scenery-phet/issues/815
---
Index: js/listeners/KeyboardListenerTests.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/listeners/KeyboardListenerTests.ts b/js/listeners/KeyboardListenerTests.ts
--- a/js/listeners/KeyboardListenerTests.ts	(revision da5f6b035473611ce3aaefc8c62ac9de161d5a6b)
+++ b/js/listeners/KeyboardListenerTests.ts	(date 1711478122026)
@@ -8,7 +8,8 @@
  * @author Agustín Vallejo (PhET Interactive Simulations)
  */
 
-import { Display, globalKeyStateTracker, KeyboardListener, KeyboardUtils, Node, SceneryEvent } from '../imports.js';
+import { Display, EnglishStringToCodeMap, globalKeyStateTracker, KeyboardListener, KeyboardUtils, Node, OneKeyStroke, SceneryEvent } from '../imports.js';
+import IntentionalAny from '../../../phet-core/js/types/IntentionalAny.js';
 
 QUnit.module( 'KeyboardListener', {
   before() {
@@ -33,6 +34,59 @@
     ctrlKey: ctrlKey
   } ) );
 };
+
+/**
+ * Triggers a keydown event on a Node. Optionally releases the key with a keyup event.
+ * @param node - Target for the event - if null, event goes to the body
+ * @param englishCode - the English "readable" string for the key code.
+ * @param providedOptions
+ */
+const keydownOnNode = ( node: Node | null, englishCode: keyof typeof EnglishStringToCodeMap, providedOptions?: IntentionalAny ): void => {
+
+  // Just use the first code if there are more than one for that english string.
+  const keyCode = EnglishStringToCodeMap[ englishCode ][ 0 ];
+
+  const options = _.merge( {
+    releaseKey: true,
+    eventOptions: {}
+  }, providedOptions );
+
+  const targetElement = node ? node.pdomInstances[ 0 ].peer!.primarySibling! : document.body;
+  console.log( 'BEFORE DISP' );
+  targetElement.dispatchEvent( new KeyboardEvent( 'keydown', {
+    code: keyCode,
+    bubbles: true,
+    ...options.eventOptions
+  } ) );
+  console.log( 'AFTER DISP' );
+
+  if ( options.releaseKey ) {
+    targetElement.dispatchEvent( new KeyboardEvent( 'keyup', {
+      code: keyCode,
+      bubbles: true,
+      ...options.eventOptions
+    } ) );
+  }
+};
+
+/**
+ * QUnit assert.throws does not support async, so this is our own. In addition, you
+ * can specify whether you expect an error to be thrown.
+ *
+ * @param assert - QUnit assert
+ * @param asyncCallback - an async function that should throw an error
+ * @param expectError - true if you expect an error to be thrown, false if you don't
+ * @param message - message for the test
+ */
+const assertThrowsAsync = async ( assert: IntentionalAny, asyncCallback: () => Promise<void>, expectError: boolean, message: string ): Promise<void> => {
+  try {
+    await asyncCallback();
+    assert.ok( !expectError, message );
+  }
+  catch( e ) {
+    assert.ok( expectError, message );
+  }
+};
 
 QUnit.test( 'KeyboardListener Tests', assert => {
 
@@ -241,4 +295,126 @@
 
   document.body.removeChild( display.domElement );
   display.dispose();
+} );
+
+QUnit.test( 'listenerOverlapBehaviorTests', assert => {
+
+  //------------------------------------------
+  // Set up the scene graph and test listeners
+  //------------------------------------------
+  const rootNode = new Node( { tagName: 'div' } );
+  const display = new Display( rootNode );
+  display.initializeEvents();
+  document.body.appendChild( display.domElement );
+
+  // Create a test scene graph that looks like this:
+  //    root
+  //       \
+  //        a
+  //         \
+  //          b
+  //        /   \
+  //       c     d
+  //      / \
+  //     e   f
+
+  const focusableNodeOptions = { tagName: 'div', focusable: true };
+  const a = new Node( focusableNodeOptions );
+  const b = new Node( focusableNodeOptions );
+  const c = new Node( focusableNodeOptions );
+  const d = new Node( focusableNodeOptions );
+  const e = new Node( focusableNodeOptions );
+  const f = new Node( focusableNodeOptions );
+
+  rootNode.addChild( a );
+  a.addChild( b );
+  b.addChild( c );
+  b.addChild( d );
+  c.addChild( e );
+  c.addChild( f );
+
+  const nodes = [ a, b, c, d, e, f ];
+
+  const testState = {
+    bFired: false,
+    cFired: false,
+    dFired: false,
+    fFired: false
+  };
+
+  const restartTestState = ( listeners: KeyboardListener<OneKeyStroke[]>[] ) => {
+    testState.bFired = false;
+    testState.cFired = false;
+    testState.dFired = false;
+    testState.fFired = false;
+
+    // remove listeners from nodes and make sure they don't have focus
+    nodes.forEach( node => {
+      node.inputListeners = [];
+      node.blur();
+    } );
+
+    // dispose of the listeners
+    listeners.forEach( listener => listener.dispose() );
+  };
+
+  //------------------------------------------------------------------
+  // A series of listeners with no overlapping keys, all should fire
+  //------------------------------------------------------------------
+  const bListener = new KeyboardListener( { keys: [ 'b' ], callback: () => { testState.bFired = true; } } );
+  const cListener = new KeyboardListener( { keys: [ 'c' ], callback: () => { testState.cFired = true; } } );
+  const dListener = new KeyboardListener( { keys: [ 'd' ], callback: () => { testState.dFired = true; } } );
+  const fListener = new KeyboardListener( { keys: [ 'f' ], callback: () => { testState.fFired = true; }, global: true } );
+
+  b.addInputListener( bListener );
+  c.addInputListener( cListener );
+  d.addInputListener( dListener );
+  f.addInputListener( fListener );
+
+  b.focus();
+  keydownOnNode( b, 'b' );
+
+  c.focus();
+  keydownOnNode( c, 'c' );
+
+  // no focus, should fire globally
+  keydownOnNode( null, 'f' );
+
+  d.focus();
+  keydownOnNode( d, 'd' );
+
+  assert.ok( testState.bFired, 'b should have fired' );
+  assert.ok( testState.cFired, 'c should have fired' );
+  assert.ok( testState.dFired, 'd should have fired' );
+  assert.ok( testState.fFired, 'f should have fired' );
+
+  restartTestState( [ bListener, cListener, dListener, fListener ] );
+
+  // The following tests verify that assertions are thrown, they require ?ea
+  if ( window.assert ) {
+
+    // ------------------------------------------------------------------------------------------------
+    // Overlapping keys along the same trail - expect an assertion unless a listener allows the overlap
+    // ------------------------------------------------------------------------------------------------
+    const bListener2 = new KeyboardListener( { keys: [ 'b' ], callback: () => { testState.bFired = true; } } );
+    const cListener2 = new KeyboardListener( { keys: [ 'b', 'c' ], callback: () => { testState.cFired = true; } } );
+    const dListener2 = new KeyboardListener( { keys: [ 'b' ], callback: () => { testState.dFired = true; } } );
+    const fListener2 = new KeyboardListener( { keys: [ 'c', 'f' ], callback: () => { testState.fFired = true; }, global: true } );
+
+    b.addInputListener( bListener2 );
+    c.addInputListener( cListener2 );
+    d.addInputListener( dListener2 );
+    f.addInputListener( fListener2 );
+
+    keydownOnNode( null, 'c' );
+    assert.ok( testState.fFired, 'f should have fired' );
+
+    keydownOnNode( b, 'b' );
+    assert.ok( testState.bFired, 'b is a parent of c - the event should not reach c and so this is fine' );
+
+    restartTestState( [ bListener2, cListener2, dListener2, fListener2 ] );
+  }
+
+  document.body.removeChild( display.domElement );
+  display.dispose();
 } );
\ No newline at end of file

[jessegreenberg]

We want to try a totally different approach. This is now on hold for #1621.

[jessegreenberg]

A new system for keyboard input was added in #1621 and is working well. KeyboardListener was re-implemented to use Hotkey, and KeyboardDragListener was re-implemented to use KeyboardListener.

KeyboardListener also now has a createGlobal, which is how we will add global hotkeys in PhET sims.

Local testing looks OK and CT has been happy with the changes. Closing.

[phet-dev]

Reopening because there is a TODO marked for this issue.

[jessegreenberg]

Thanks phet-dev, issue link has been removed. Closing.