Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create a registry for global keyboard events #1621

Closed
jessegreenberg opened this issue Mar 26, 2024 · 7 comments
Closed

Create a registry for global keyboard events #1621

jessegreenberg opened this issue Mar 26, 2024 · 7 comments
Assignees

Comments

@jessegreenberg
Copy link
Contributor

jessegreenberg commented Mar 26, 2024

We need a global keyboard registry for keyboard input. This is a global object which collects all of the used 'keys' that should have behavior without a target/focus Trail. This is going to replace globalkeydown/globalkeyup.

  • This will have much better performance. We are very nervous about performance of Display.dispatchGlobalEvent.
  • This will support checking for used key overlaps at runtime.
  • This may support having a single location where we define used keys (for listeners, keyboard help dialogs, i18n, and so on) but we are less certain about that.

We are thinking of a Registry and Hotkey class like this:

Registry.register( new Hotkey ( {
  keystroke: 'alt+a',
  fire: ( event: SceneryEvent<KeyboardEvent> ) => {
    // runs the behavior
  },
  isEnabled: ( event: SceneryEvent<KeyboardEvent> ) => {
    // ...and walk up the trail...
    // IF we only want one Display to fire, could make sure the Display has focus in this function.
    // myNode.visible && myNode.pdomEnabled && myNode.inputEnabled;

    // For KeyboardListener, it would make sure that the Node is connected to a Display and that
    // every ancestor can receive input events.
  },

  // Possibly in the future, it could also have an object like this for KeyboardHelpDIalog content
  keyboardHelpDialogContent: {
    title: 'Title',
    description: 'Description',
    pdomDescription: 'screen reader specific description',
    displayedICons: [ '->'],
    isEnabled: () => {
      // ...
      // custom logic that decides if this content should be shown
    }
  }
} ) )

For keyboard events that still fire along the trail, TInputListenerListener will have a hotkeys array. During event dispatch, we can then grab used keys from a listener, and compare them to the global registry. Then we can look for used key overlaps, or only fire certain listeners. With hotkeys on TInputListener, one listener can still be added to multiple Nodes.

Raw scratch notes from meeting in case I missed anything important in the summary.


Concerned about the global dispatch performance.
  - Need a 'pickable' equivalent for Nodes that support accessibility events.

Create a "registry" for global events as an alternative to traversal.

The most efficient way is a scan through parents to find a connected display. That trail must be a visible Trail
and an a11y visible Trail.

const listener = new KeyboardListener( {
  keys: [ 'shift', 'a' ],

  // This is what makes it global, and how we
  targetNode: mySceneryNode
} );

Listener as another abstraction??

// Create a class called Hotkey (instead of this object).
Registry.register( new Hotkey ( {
  keys: ['alt+a', 'j'],
  fire: ( event: SceneryEvent<KeyboardEvent> ) => {
    // runs the behavior
  },
  isEnabled: ( event: SceneryEvent<KeyboardEvent> ) => {
    // ...and walk up the trail...
    // IF we only want one Display to fire, could make sure the Display has focus in this function.
    // myNode.visible && myNode.pdomEnabled && myNode.inputEnabled;

    // For KeyboardListener, it would make sure that the Node is connected to a Display and that
    // every ancestor can receive input events.
  },

  // Possibly in the future, it could also have an object like this for KeyboardHelpDIalog content
  keyboardHelpDialogContent: {
    title: 'Title',
    description: 'Description',
    pdomDescription: 'screen reader specific description',
    displayedICons: [ ''->'],
    isEnabled: () => {
      // ...
      // custom logic that decides if this content should be shown
    }
  }
} ) )

For keyboard events that still fire along the trail, TInputListenerListener would have the hotKeys array.
 - Grab hotkeys from the input listener.
 - This would support one listener added to multiple Nodes.
 - This way we can get the hotkeys array during input dispatch along the trail.

Don't handle or abort by default.

KeyboardListener exposes a public field called hotKeys. It will be in the input listener's list.
It will be its trail/local hotkeys that depend on focus.

When you have a hotkey array on an input listener, whenever a keypress happens and a trail includes
that Node, those hotkeys can be "activated". They get combined with the "global"/registry hotkeys.
This way the "local" input listeners can respect the global registry - conflict handling, assertions,
custom modifier keys.
@jonathanolson
Copy link
Contributor

Hotkey would presumably take just 1 key combination (if you need multiple, it is multiple hotkeys).

Add an attach: false equivalent

jonathanolson added a commit to phetsims/balloons-and-static-electricity that referenced this issue Mar 28, 2024
jonathanolson added a commit to phetsims/inverse-square-law-common that referenced this issue Mar 28, 2024
jonathanolson added a commit to phetsims/phet-info that referenced this issue Mar 28, 2024
jonathanolson added a commit that referenced this issue Mar 28, 2024
jonathanolson added a commit to phetsims/faradays-law that referenced this issue Mar 28, 2024
@jonathanolson
Copy link
Contributor

I got it to a stability place where I felt comfortable with putting it in main (as it hit 7 different repos). I'll be checking to see if I need to revert things out.

I added an option to support both the "browser" fire-on-hold (users may have customized or be used to a timing) or "custom" fire-on-hold (we control how often it fires).

It has a good number of TODOs marked with this issue that will be good to discuss.

jonathanolson added a commit that referenced this issue Mar 28, 2024
jonathanolson added a commit that referenced this issue Mar 28, 2024
…teTracker, and corresponding fixes/updates in hotkeyManager, see #1621
jonathanolson added a commit to phetsims/wilder that referenced this issue Mar 28, 2024
jonathanolson added a commit that referenced this issue Mar 28, 2024
@jessegreenberg
Copy link
Contributor Author

The registry is working well but using enabledProperty for global keyboard events makes it difficult to use with an actual scenery Node. How do we derive a single Property for whether a Node can receive input events?

a) Whenever the instances of a Node change, add a listeners to every Node along all trails attached to a Display that can impact whether the Node can receive input events. (visibleProperty, enabledProperty, disposedProperty, pdomVisibleProperty). That is potentially a LOT of listeners, and I am worried about using changedInstanceEmitter.
b) Instead of a Property, Hotkey has an isGloballyEnabled which can be checked before firing. Works OK but not as well with our Hotkey overlap strategy.

Here is a patch with b. But after discussing with @jonathanolson we are going to try a. It sounds like DisplayedProperty should be OK to use and modify for our needs and that using changedInstanceEmitter is relatively safe for this.

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 df9ab1245ac2fd0a5f2e1832bf54e1f5c750ba5e)
+++ b/scenery/js/listeners/KeyboardListener.ts	(date 1712004988226)
@@ -16,7 +16,7 @@
  *
  * this.addInputListener( new KeyboardListener( {
  *   keys: [ 'a+b', 'a+c', 'shift+arrowLeft', 'alt+g+t', 'ctrl+3', 'alt+ctrl+t' ],
- *   callback: ( event, keysPressed, listener ) => {
+ *   fire: ( event, keysPressed, listener ) => {
  *     if ( keysPressed === 'a+b' ) {
  *       console.log( 'you just pressed a+b!' );
  *     }
@@ -35,11 +35,11 @@
  *   }
  * } ) );
  *
- * By default the callback will fire when the last key is pressed down. See additional options for firing on key
+ * By default the fire callback will fire when the last key is pressed down. See additional options for firing on key
  * up or other press and hold behavior.
  *
  * **Potential Pitfall!**
- * The callback is only called if exactly the keys in a group are pressed. If you need to listen to a modifier key,
+ * The fire callback is only called if exactly the keys in a group are pressed. If you need to listen to a modifier key,
  * you must include it in the keys array. For example if you add a listener for 'tab', you must ALSO include
  * 'shift+tab' in the array to observe 'shift+tab' presses. If you provide 'tab' alone, the callback will not fire
  * if 'shift' is also pressed.
@@ -49,8 +49,7 @@
 
 import CallbackTimer from '../../../axon/js/CallbackTimer.js';
 import optionize from '../../../phet-core/js/optionize.js';
-import { EnglishStringToCodeMap, FocusManager, globalKeyStateTracker, scenery, SceneryEvent, TInputListener } from '../imports.js';
-import KeyboardUtils from '../accessibility/KeyboardUtils.js';
+import { EnglishKey, EnglishStringToCodeMap, globalHotkeyRegistry, Hotkey, Node, scenery, TInputListener } from '../imports.js';
 
 // NOTE: The typing for ModifierKey and OneKeyStroke is limited TypeScript, there is a limitation to the number of
 //       entries in a union type. If that limitation is not acceptable remove this typing. OR maybe TypeScript will
@@ -85,29 +84,11 @@
 
   // If true, the listener will fire for keys regardless of where focus is in the document. Use this when you want
   // to add some key press behavior that will always fire no matter what the event target is. If this listener
-  // is added to a Node, it will only fire if the Node (and all of its ancestors) are visible with inputEnabled: true.
-  // More specifically, this uses `globalKeyUp` and `globalKeyDown`. See definitions in Input.ts for more information.
+  // is added to a Node, it will only fire if the Node (and all of its ancestors) can receive input events.
   global?: boolean;
 
-  // If true, this listener is fired during the 'capture' phase. Only relevant for `global` key events.
-  // When a listener uses capture, the callbacks will be fired BEFORE the dispatch through the scene graph
-  // (very similar to DOM's addEventListener with `useCapture` set to true - see
-  // https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener).
-  capture?: boolean;
-
-  // If true, all SceneryEvents that trigger this listener (keydown and keyup) will be `handled` (no more
-  // event bubbling). See `manageEvent` for more information.
-  handle?: boolean;
-
-  // If true, all SceneryEvents that trigger this listener (keydown and keyup) will be `aborted` (no more
-  // event bubbling, no more listeners fire). See `manageEvent` for more information.
-  abort?: boolean;
-
   // Called when the listener detects that the set of keys are pressed.
-  callback?: ( event: SceneryEvent<KeyboardEvent> | null, keysPressed: Keys[number], listener: KeyboardListener<Keys> ) => void;
-
-  // Called when the listener is cancelled/interrupted.
-  cancel?: ( listener: KeyboardListener<Keys> ) => void;
+  fire?: ( event: KeyboardEvent | null, keysPressed: Keys[number], listener: KeyboardListener<Keys> ) => void;
 
   // Called when the listener target receives focus.
   focus?: ( listener: KeyboardListener<Keys> ) => void;
@@ -127,6 +108,10 @@
   // Possible input types that decide when callbacks of the listener fire in response to input. See
   // ListenerFireTrigger type documentation.
   listenerFireTrigger?: ListenerFireTrigger;
+
+  // The Node that will be the target for global key events. Listener will only fire if this Node can
+  // receive input events. If not provided, the listener must be added to a specific Node with addInputListener().
+  globalTargetNode?: Node | null;
 };
 
 type KeyGroup<Keys extends readonly OneKeyStroke[]> = {
@@ -149,10 +134,7 @@
   // The function called when a KeyGroup is pressed (or just released). Provides the SceneryEvent that fired the input
   // listeners and this the keys that were pressed from the active KeyGroup. The event may be null when using
   // fireOnHold or in cases of cancel or interrupt. A reference to the listener is provided for other state.
-  private readonly _callback: ( event: SceneryEvent<KeyboardEvent> | null, keysPressed: Keys[number], listener: KeyboardListener<Keys> ) => void;
-
-  // The optional function called when this listener is cancelled.
-  private readonly _cancel: ( listener: KeyboardListener<Keys> ) => void;
+  private readonly _fire: ( event: KeyboardEvent | null, keysPressed: Keys[number], listener: KeyboardListener<Keys> ) => void;
 
   // The optional function called when this listener's target receives focus.
   private readonly _focus: ( listener: KeyboardListener<Keys> ) => void;
@@ -166,9 +148,6 @@
   // Does the listener fire the callback continuously when keys are held down?
   private readonly _fireOnHold: boolean;
 
-  // (scenery-internal) All the KeyGroups of this listener from the keys provided in natural language.
-  public readonly _keyGroups: KeyGroup<Keys>[];
-
   // All the KeyGroups that are currently firing
   private readonly _activeKeyGroups: KeyGroup<Keys>[];
 
@@ -182,29 +161,21 @@
 
   // see options documentation
   private readonly _global: boolean;
-  private readonly _handle: boolean;
-  private readonly _abort: boolean;
-
-  private readonly _windowFocusListener: ( windowHasFocus: boolean ) => void;
 
   public constructor( providedOptions: KeyboardListenerOptions<Keys> ) {
     const options = optionize<KeyboardListenerOptions<Keys>>()( {
-      callback: _.noop,
-      cancel: _.noop,
+      fire: _.noop,
       focus: _.noop,
       blur: _.noop,
       global: false,
-      capture: false,
-      handle: false,
-      abort: false,
       listenerFireTrigger: 'down',
       fireOnHold: false,
       fireOnHoldDelay: 400,
-      fireOnHoldInterval: 100
+      fireOnHoldInterval: 100,
+      globalTargetNode: null
     }, providedOptions );
 
-    this._callback = options.callback;
-    this._cancel = options.cancel;
+    this._fire = options.fire;
     this._focus = options.focus;
     this._blur = options.blur;
 
@@ -218,293 +189,26 @@
     this.keysDown = false;
 
     this._global = options.global;
-    this._handle = options.handle;
-    this._abort = options.abort;
 
-    // convert the provided keys to data that we can respond to with scenery's Input system
-    this._keyGroups = this.convertKeysToKeyGroups( options.keys );
-
-    // Assign listener and capture to this, implementing TInputListener
+    // Assign listener to this, implementing TInputListener
     ( this as unknown as TInputListener ).listener = this;
-    ( this as unknown as TInputListener ).capture = options.capture;
 
-    this._windowFocusListener = this.handleWindowFocusChange.bind( this );
-    FocusManager.windowHasFocusProperty.link( this._windowFocusListener );
+    // convert the provided keys to data that we can respond to with scenery's Input system
+    ( this as unknown as TInputListener ).hotkeys = this.createHotkeys( options.keys, options.globalTargetNode );
   }
 
   /**
    * 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 );
-  }
-
-  /**
-   * Responding to a keydown event, update active KeyGroups and potentially fire callbacks and start CallbackTimers.
-   */
-  private handleKeyDown( event: SceneryEvent<KeyboardEvent> ): void {
-    if ( this._listenerFireTrigger === 'down' || this._listenerFireTrigger === 'both' ) {
-
-      // modifier keys can be pressed in any order but the last key in the group must be pressed last
-      this._keyGroups.forEach( keyGroup => {
-
-        if ( !this._activeKeyGroups.includes( keyGroup ) ) {
-          if ( this.areKeysDownForListener( keyGroup ) &&
-               keyGroup.keys.includes( globalKeyStateTracker.getLastKeyDown()! ) ) {
-
-            this._activeKeyGroups.push( keyGroup );
-
-            this.keysDown = true;
-
-            // reserve the event for this listener, disabling more 'global' input listeners such as
-            // those for pan and zoom (this is similar to DOM event.preventDefault).
-            event.pointer.reserveForKeyboardDrag();
-
-            if ( keyGroup.timer ) {
-              keyGroup.timer.start();
-            }
-
-            this.fireCallback( event, keyGroup );
-          }
-        }
-      } );
-    }
-
-    this.manageEvent( event );
-  }
-
-  /**
-   * If there are any active KeyGroup firing stop and remove if KeyGroup keys are no longer down. Also, potentially
-   * fires a KeyGroup callback if the key that was released has all other modifier keys down.
-   */
-  private handleKeyUp( event: SceneryEvent<KeyboardEvent> ): void {
-
-    if ( this._activeKeyGroups.length > 0 ) {
-      this._activeKeyGroups.forEach( ( activeKeyGroup, index ) => {
-        if ( !this.areKeysDownForListener( activeKeyGroup ) ) {
-          if ( activeKeyGroup.timer ) {
-            activeKeyGroup.timer.stop( false );
-          }
-          this._activeKeyGroups.splice( index, 1 );
-        }
-      } );
-    }
-
-    if ( this._listenerFireTrigger === 'up' || this._listenerFireTrigger === 'both' ) {
-      const eventCode = KeyboardUtils.getEventCode( event.domEvent )!;
-
-      // Screen readers may send key events with no code for unknown reasons, we need to be graceful when that
-      // happens, see https://github.com/phetsims/scenery/issues/1534.
-      if ( eventCode ) {
-        this._keyGroups.forEach( keyGroup => {
-          if ( this.areModifierKeysDownForListener( keyGroup ) &&
-               keyGroup.keys.includes( eventCode ) ) {
-            this.keysDown = false;
-            this.fireCallback( event, keyGroup );
-          }
-        } );
-      }
-    }
-
-    this.manageEvent( event );
-  }
-
-  /**
-   * Returns an array of KeyboardEvent.codes from the provided key group that are currently pressed down.
-   */
-  private getDownModifierKeys( keyGroup: KeyGroup<Keys> ): string[] {
-
-    // Remember, this is a 2D array. The inner array is the list of 'equivalent' keys to be pressed for the required
-    // modifier key. For example [ 'shiftLeft', 'shiftRight' ]. If any of the keys in that inner array are pressed,
-    // that set of modifier keys is considered pressed.
-    const modifierKeysCollection = keyGroup.modifierKeys;
-
-    // The list of modifier keys that are actually pressed
-    const downModifierKeys: string[] = [];
-    modifierKeysCollection.forEach( modifierKeys => {
-      for ( const modifierKey of modifierKeys ) {
-        if ( globalKeyStateTracker.isKeyDown( modifierKey ) ) {
-          downModifierKeys.push( modifierKey );
-
-          // One modifier key from this inner set is down, stop looking
-          break;
-        }
-      }
-    } );
-
-    return downModifierKeys;
-  }
-
-  /**
-   * Returns true if keys are pressed such that the listener should fire. In order to fire, all modifier keys
-   * should be down and the final key of the group should be down. If any extra modifier keys are down that are
-   * not specified in the keyGroup, the listener will not fire.
-   */
-  private areKeysDownForListener( keyGroup: KeyGroup<Keys> ): boolean {
-    const downModifierKeys = this.getDownModifierKeys( keyGroup );
-
-    // modifier keys are down if one key from each inner array is down
-    const areModifierKeysDown = downModifierKeys.length === keyGroup.modifierKeys.length;
-
-    // The final key of the group is down if any of them are pressed
-    const finalDownKey = keyGroup.keys.find( key => globalKeyStateTracker.isKeyDown( key ) );
-
-    if ( areModifierKeysDown && !!finalDownKey ) {
-
-      // 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 );
-    }
-    else {
-      return false;
-    }
-  }
-
-  /**
-   * Returns true if the modifier keys of the provided key group are currently down. If any extra modifier keys are
-   * down that are not specified in the keyGroup, the listener will not fire.
-   */
-  private areModifierKeysDownForListener( keyGroup: KeyGroup<Keys> ): boolean {
-    const downModifierKeys = this.getDownModifierKeys( keyGroup );
-
-    // modifier keys are down if one key from each inner array is down
-    const modifierKeysDown = downModifierKeys.length === keyGroup.modifierKeys.length;
-
-    if ( modifierKeysDown ) {
-
-      // If there are any extra modifier keys down, the listener will not fire
-      return globalKeyStateTracker.areKeysDownWithoutExtraModifiers( downModifierKeys );
-    }
-    else {
-      return false;
-    }
-  }
-
-  /**
-   * In response to every SceneryEvent, handle and/or abort depending on listener options. This cannot be done in
-   * the callbacks because press-and-hold behavior triggers many keydown events. We need to handle/abort each, not
-   * just the event that triggered the callback. Also, callbacks can be called without a SceneryEvent from the
-   * CallbackTimer.
-   */
-  private manageEvent( event: SceneryEvent<KeyboardEvent> ): void {
-    this._handle && event.handle();
-    this._abort && event.abort();
-  }
-
-  /**
-   * This is part of the scenery Input API (implementing TInputListener). Handle the keydown event when not
-   * 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 ) {
-      this.handleKeyDown( event );
-    }
-  }
-
-  /**
-   * This is part of the scenery Input API (implementing TInputListener). Handle the keyup event when not
-   * 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 ) {
-      this.handleKeyUp( event );
-    }
-  }
-
-  /**
-   * This is part of the scenery Input API (implementing TInputListener). Handle the global keydown event.
-   * Event has no target.
-   */
-  public globalkeydown( event: SceneryEvent<KeyboardEvent> ): void {
-    if ( this._global ) {
-      this.handleKeyDown( event );
-    }
-  }
-
-  /**
-   * This is part of the scenery Input API (implementing TInputListener). Handle the global keyup event.
-   * Event has no target.
-   */
-  public globalkeyup( event: SceneryEvent<KeyboardEvent> ): void {
-    if ( this._global ) {
-      this.handleKeyUp( event );
-    }
-  }
-
-  /**
-   * Work to be done on both cancel and interrupt.
-   */
-  private handleCancel(): void {
-    this.clearActiveKeyGroups();
-    this._cancel( this );
-  }
-
-  /**
-   * When the window loses focus, cancel.
-   */
-  private handleWindowFocusChange( windowHasFocus: boolean ): void {
-    if ( !windowHasFocus ) {
-      this.handleCancel();
-    }
-  }
-
-  /**
-   * Part of the scenery listener API. On cancel, clear active KeyGroups and stop their behavior.
-   */
-  public cancel(): void {
-    this.handleCancel();
-  }
-
-  /**
-   * Part of the scenery listener API. Clear active KeyGroups and stop their callbacks.
-   */
-  public interrupt(): void {
-    this.handleCancel();
-  }
-
-  /**
-   * Interrupts and resets the listener on blur so that state is reset and active keyGroups are cleared.
-   * Public because this is called with the scenery listener API. Do not call this directly.
-   */
-  public focusout( event: SceneryEvent ): void {
-    this.interrupt();
-
-    // Optional work to do on blur.
-    this._blur( this );
-  }
-
-  /**
-   * Public because this is called through the scenery listener API. Do not call this directly.
-   */
-  public focusin( event: SceneryEvent ): void {
-
-    // Optional work to do on focus.
-    this._focus( this );
+  public fireCallback( event: KeyboardEvent | null, naturalKeys: Keys[number] ): void {
+    this._fire( event, naturalKeys, this );
   }
 
   /**
    * Dispose of this listener by disposing of any Callback timers. Then clear all KeyGroups.
    */
   public dispose(): void {
-    this._keyGroups.forEach( activeKeyGroup => {
-      activeKeyGroup.timer && activeKeyGroup.timer.dispose();
-    } );
-    this._keyGroups.length = 0;
-
-    FocusManager.windowHasFocusProperty.unlink( this._windowFocusListener );
-  }
-
-  /**
-   * Clear the active KeyGroups on this listener. Stopping any active groups if they use a CallbackTimer.
-   */
-  private clearActiveKeyGroups(): void {
-    this._activeKeyGroups.forEach( activeKeyGroup => {
-      activeKeyGroup.timer && activeKeyGroup.timer.stop( false );
-    } );
-
-    this._activeKeyGroups.length = 0;
+    ( this as unknown as TInputListener ).hotkeys!.forEach( hotkey => hotkey.dispose() );
   }
 
   /**
@@ -512,45 +216,93 @@
    * will take a string that defines the keys for this listener like 'a+c|1+2+3+4|shift+leftArrow' and return an array
    * with three KeyGroups, one describing 'a+c', one describing '1+2+3+4' and one describing 'shift+leftArrow'.
    */
-  private convertKeysToKeyGroups( keys: Keys ): KeyGroup<Keys>[] {
+  private createHotkeys( keys: Keys, globalTargetNode: Node | null ): Hotkey[] {
+
+    return keys.map( naturalKeys => {
 
-    const keyGroups = keys.map( naturalKeys => {
+      // Split the keys into the main key and the modifier keys
+      const keys = naturalKeys.split( '+' );
+      const modifierKeys = keys.slice( 0, keys.length - 1 );
+      const naturalKey = keys[ keys.length - 1 ];
 
-      // all of the keys in this group in an array
-      const groupKeys = naturalKeys.split( '+' );
-      assert && assert( groupKeys.length > 0, 'no keys provided?' );
+      // If the globalTargetNode exists, assemble an enabledProperty for the hotkey that describes
+      // whether the Node can receive global events.
 
-      const naturalKey = groupKeys.slice( -1 )[ 0 ] as AllowedKeys;
-      const keys = EnglishStringToCodeMap[ naturalKey ]!;
-      assert && assert( keys, `Codes were not found, do you need to add it to EnglishStringToCodeMap? ${naturalKey}` );
+      // This describes when we can receive input events:
+      //   if ( !node.isDisposed && node.isVisible() && node.isInputEnabled() && node.isPDOMVisible() )
+      // For every trail from the globalTargetNode that is attached to the display,
+      // Add a listener to each of those attributes to update the enabledProperty.
+      // AND would re-compute whenever trails change??
+      let isGloballyEnabled: () => boolean = () => true;
+      if ( globalTargetNode ) {
 
-      let modifierKeys: string[][] = [];
-      if ( groupKeys.length > 1 ) {
-        const naturalModifierKeys = groupKeys.slice( 0, groupKeys.length - 1 ) as ModifierKey[];
-        modifierKeys = naturalModifierKeys.map( naturalModifierKey => {
-          const modifierKeys = EnglishStringToCodeMap[ naturalModifierKey ]!;
-          assert && assert( modifierKeys, `Key not found, do you need to add it to EnglishStringToCodeMap? ${naturalModifierKey}` );
-          return modifierKeys;
-        } );
-      }
+        // globalTargetNode.changedInstanceEmitter.addListener( () => {
+        //
+        //
+        //   // Add listeners to 4 Properties on all Nodes in all trails.
+        //   // Yikes! changedInstanceEmitter should not be used.
+        // } );
+
+        // The hotkey is globally enabled if there is one trail that is attached to a display where all Nodes
+        // can receive input events.
+        isGloballyEnabled = () => {
+          let enabled = false;
+
+          const trails = globalTargetNode.getTrails();
+          for ( let i = 0; i < trails.length; i++ ) {
+
+            // prune trails that are not attached to a display
+            if ( trails[ i ].rootNode().getRootedDisplays().length < 1 ) {
+              continue;
+            }
 
-      // Set up the timer for triggering callbacks if this listener supports press and hold behavior
-      const timer = this._fireOnHold ? new CallbackTimer( {
-        callback: () => this.fireCallback( null, keyGroup ),
-        delay: this._fireOnHoldDelay,
-        interval: this._fireOnHoldInterval
-      } ) : null;
+            let trailCanReceiveInput = true;
+            const nodes = trails[ i ].nodes; // Capture the nodes array once to avoid repeated property access
+            for ( let j = 0; j < nodes.length; j++ ) {
+              const node = nodes[ j ];
 
-      const keyGroup: KeyGroup<Keys> = {
-        keys: keys,
-        modifierKeys: modifierKeys,
-        naturalKeys: naturalKeys,
-        timer: timer
-      };
-      return keyGroup;
-    } );
+              // If any node condition fails, the trail cannot receive input
+              if ( node.isDisposed || !node.isVisible() || !node.isInputEnabled() || !node.isPDOMVisible() ) {
+                trailCanReceiveInput = false;
+                break;
+              }
+            }
+
+            // If any trail can receive input, the hotkey is globally enabled
+            if ( trailCanReceiveInput ) {
+              enabled = true;
+              break;
+            }
+          }
+
+          return enabled;
+        };
+      }
+
+      const hotkey = new Hotkey( {
+        key: naturalKey as EnglishKey,
+        modifierKeys: modifierKeys as EnglishKey[],
+        fire: event => {
+          this.fireCallback( event, naturalKeys );
+        },
+        isGloballyEnabled: isGloballyEnabled
+      } );
 
-    return keyGroups;
+      globalHotkeyRegistry.add( hotkey );
+
+      return hotkey;
+    } );
+  }
+
+  public static createGlobalKeyboardListener( providedOptions: KeyboardListenerOptions<any>, target: Node ): void {
+
+    // Need to set the enabled Property on the HOtkey.
+    // Need to update that enabled Property when the Node state for how it can receive inptu changes.
+    const listenerOptions = _.merge( {
+      globalTargetNode: target
+    }, providedOptions );
+
+    const listener = new KeyboardListener( listenerOptions );
   }
 }
 
Index: scenery/js/input/Hotkey.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/scenery/js/input/Hotkey.ts b/scenery/js/input/Hotkey.ts
--- a/scenery/js/input/Hotkey.ts	(revision df9ab1245ac2fd0a5f2e1832bf54e1f5c750ba5e)
+++ b/scenery/js/input/Hotkey.ts	(date 1712003324171)
@@ -67,6 +67,8 @@
   // For each main `key`, the hotkey system will only allow one hotkey with allowOverlap:false to be active at any time.
   // This is provided to allow multiple hotkeys with the same keys to fire. Default is false.
   allowOverlap?: boolean;
+
+  isGloballyEnabled?: () => boolean;
 };
 
 export type HotkeyOptions = SelfOptions & EnabledComponentOptions;
@@ -82,6 +84,7 @@
   public readonly fireOnHold: boolean;
   public readonly fireOnHoldTiming: HotkeyFireOnHoldTiming;
   public readonly allowOverlap: boolean;
+  public readonly isGloballyEnabled: () => boolean;
 
   // All keys that are part of this hotkey (key + modifierKeys)
   public readonly keys: EnglishKey[];
@@ -118,7 +121,8 @@
       fireOnHoldTiming: 'browser',
       fireOnHoldCustomDelay: 400,
       fireOnHoldCustomInterval: 100,
-      allowOverlap: false
+      allowOverlap: false,
+      isGloballyEnabled: () => true
     }, providedOptions );
 
     super( options );
@@ -132,6 +136,7 @@
     this.fireOnHold = options.fireOnHold;
     this.fireOnHoldTiming = options.fireOnHoldTiming;
     this.allowOverlap = options.allowOverlap;
+    this.isGloballyEnabled = options.isGloballyEnabled;
 
     this.keys = _.uniq( [ this.key, ...this.modifierKeys ] );
 
Index: scenery/js/input/hotkeyManager.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/scenery/js/input/hotkeyManager.ts b/scenery/js/input/hotkeyManager.ts
--- a/scenery/js/input/hotkeyManager.ts	(revision df9ab1245ac2fd0a5f2e1832bf54e1f5c750ba5e)
+++ b/scenery/js/input/hotkeyManager.ts	(date 1712005717781)
@@ -178,11 +178,17 @@
         // Handle re-entrancy (if something changes the state of activeHotkeys)
         for ( const hotkey of [ ...this.activeHotkeys ] ) {
           if ( hotkey.fireOnHold && hotkey.fireOnHoldTiming === 'browser' ) {
-            hotkey.fire( keyboardEvent );
-          }
-        }
-      }
-    } );
+            this.fireHotkey( hotkey, keyboardEvent );
+          }
+        }
+      }
+    } );
+  }
+
+  private fireHotkey( hotkey: Hotkey, keyboardEvent: KeyboardEvent | null ): void {
+    if ( hotkey.isGloballyEnabled() ) {
+      hotkey.fire( keyboardEvent );
+    }
   }
 
   /**
@@ -257,7 +263,7 @@
     hotkey.interrupted = false;
 
     if ( triggeredFromPress && hotkey.fireOnDown ) {
-      hotkey.fire( keyboardEvent );
+      this.fireHotkey( hotkey, keyboardEvent );
     }
   }
 
@@ -268,7 +274,7 @@
     hotkey.interrupted = !triggeredFromRelease;
 
     if ( triggeredFromRelease && !hotkey.fireOnDown ) {
-      hotkey.fire( keyboardEvent );
+      this.fireHotkey( hotkey, keyboardEvent );
     }
 
     hotkey.isPressedProperty.value = false;
Index: wilder/js/wilder/view/WilderScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/wilder/js/wilder/view/WilderScreenView.ts b/wilder/js/wilder/view/WilderScreenView.ts
--- a/wilder/js/wilder/view/WilderScreenView.ts	(revision 4812e57d2a01b566c4dbe16105fcd070b505eb89)
+++ b/wilder/js/wilder/view/WilderScreenView.ts	(date 1712005467007)
@@ -11,7 +11,7 @@
 import WilderModel from '../model/WilderModel.js';
 import { PhetioObjectOptions } from '../../../../tandem/js/PhetioObject.js';
 import PickRequired from '../../../../phet-core/js/types/PickRequired.js';
-import { globalHotkeyRegistry, Hotkey, Text } from '../../../../scenery/js/imports.js';
+import { globalHotkeyRegistry, Hotkey, KeyboardListener, Text } from '../../../../scenery/js/imports.js';
 import BooleanProperty from '../../../../axon/js/BooleanProperty.js';
 import ABSwitch from '../../../../sun/js/ABSwitch.js';
 import PhetFont from '../../../../scenery-phet/js/PhetFont.js';
@@ -119,37 +119,51 @@
       } )
     } ) );
 
-    resetAllButton.addInputListener( {
-      hotkeys: [
-        new Hotkey( {
-          key: 'x',
-          fire: () => console.log( 'fire: x' ),
-          enabledProperty: extraEnabledProperty
-        } ),
-        new Hotkey( {
-          key: 'x',
-          modifierKeys: [ 'b' ],
-          fire: () => console.log( 'fire: b+x' ),
-          enabledProperty: extraEnabledProperty
-        } ),
-        new Hotkey( {
-          key: 'w',
-          fire: () => console.log( 'fire: w' )
-        } ),
-        new Hotkey( {
-          key: 'a',
-          fire: () => console.log( 'fire: a' )
-        } ),
-        new Hotkey( {
-          key: 's',
-          fire: () => console.log( 'fire: s' )
-        } ),
-        new Hotkey( {
-          key: 'd',
-          fire: () => console.log( 'fire: d' )
-        } )
-      ]
-    } );
+    resetAllButton.addInputListener( new KeyboardListener( {
+      keys: [ 'x' ],
+      fire: () => console.log( 'fire: x from KeyboardListener' )
+    } ) );
+
+    KeyboardListener.createGlobalKeyboardListener( {
+      keys: [ 'x' ],
+      fire: () => console.log( 'fire: h from global KeyboardListener' )
+    }, resetAllButton );
+
+    window.setInterval( () => {
+      resetAllButton.enabled = !resetAllButton.enabled;
+    }, 2000 );
+
+    // resetAllButton.addInputListener( {
+    //   hotkeys: [
+    //     new Hotkey( {
+    //       key: 'x',
+    //       fire: () => console.log( 'fire: x' ),
+    //       enabledProperty: extraEnabledProperty
+    //     } ),
+    //     new Hotkey( {
+    //       key: 'x',
+    //       modifierKeys: [ 'b' ],
+    //       fire: () => console.log( 'fire: b+x' ),
+    //       enabledProperty: extraEnabledProperty
+    //     } ),
+    //     new Hotkey( {
+    //       key: 'w',
+    //       fire: () => console.log( 'fire: w' )
+    //     } ),
+    //     new Hotkey( {
+    //       key: 'a',
+    //       fire: () => console.log( 'fire: a' )
+    //     } ),
+    //     new Hotkey( {
+    //       key: 's',
+    //       fire: () => console.log( 'fire: s' )
+    //     } ),
+    //     new Hotkey( {
+    //       key: 'd',
+    //       fire: () => console.log( 'fire: d' )
+    //     } )
+    //   ]
+    // } );
   }
 }
 

@jessegreenberg
Copy link
Contributor Author

Here is a patch, but I am having a hard time with it because I need to observe changes to inputEnabled which is not available on Instance. This patch tries to emit, but the dirty flags aren't working to update the subtree correctly. I have a low level of confidence that we can use these changes so I am going to bail on this for now.

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 df9ab1245ac2fd0a5f2e1832bf54e1f5c750ba5e)
+++ b/scenery/js/listeners/KeyboardListener.ts	(date 1712006855963)
@@ -16,7 +16,7 @@
  *
  * this.addInputListener( new KeyboardListener( {
  *   keys: [ 'a+b', 'a+c', 'shift+arrowLeft', 'alt+g+t', 'ctrl+3', 'alt+ctrl+t' ],
- *   callback: ( event, keysPressed, listener ) => {
+ *   fire: ( event, keysPressed, listener ) => {
  *     if ( keysPressed === 'a+b' ) {
  *       console.log( 'you just pressed a+b!' );
  *     }
@@ -35,11 +35,11 @@
  *   }
  * } ) );
  *
- * By default the callback will fire when the last key is pressed down. See additional options for firing on key
+ * By default the fire callback will fire when the last key is pressed down. See additional options for firing on key
  * up or other press and hold behavior.
  *
  * **Potential Pitfall!**
- * The callback is only called if exactly the keys in a group are pressed. If you need to listen to a modifier key,
+ * The fire callback is only called if exactly the keys in a group are pressed. If you need to listen to a modifier key,
  * you must include it in the keys array. For example if you add a listener for 'tab', you must ALSO include
  * 'shift+tab' in the array to observe 'shift+tab' presses. If you provide 'tab' alone, the callback will not fire
  * if 'shift' is also pressed.
@@ -49,8 +49,7 @@
 
 import CallbackTimer from '../../../axon/js/CallbackTimer.js';
 import optionize from '../../../phet-core/js/optionize.js';
-import { EnglishStringToCodeMap, FocusManager, globalKeyStateTracker, scenery, SceneryEvent, TInputListener } from '../imports.js';
-import KeyboardUtils from '../accessibility/KeyboardUtils.js';
+import { DisplayedProperty, EnglishKey, EnglishStringToCodeMap, globalHotkeyRegistry, Hotkey, HotkeyOptions, Node, scenery, TInputListener } from '../imports.js';
 
 // NOTE: The typing for ModifierKey and OneKeyStroke is limited TypeScript, there is a limitation to the number of
 //       entries in a union type. If that limitation is not acceptable remove this typing. OR maybe TypeScript will
@@ -85,29 +84,11 @@
 
   // If true, the listener will fire for keys regardless of where focus is in the document. Use this when you want
   // to add some key press behavior that will always fire no matter what the event target is. If this listener
-  // is added to a Node, it will only fire if the Node (and all of its ancestors) are visible with inputEnabled: true.
-  // More specifically, this uses `globalKeyUp` and `globalKeyDown`. See definitions in Input.ts for more information.
+  // is added to a Node, it will only fire if the Node (and all of its ancestors) can receive input events.
   global?: boolean;
 
-  // If true, this listener is fired during the 'capture' phase. Only relevant for `global` key events.
-  // When a listener uses capture, the callbacks will be fired BEFORE the dispatch through the scene graph
-  // (very similar to DOM's addEventListener with `useCapture` set to true - see
-  // https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener).
-  capture?: boolean;
-
-  // If true, all SceneryEvents that trigger this listener (keydown and keyup) will be `handled` (no more
-  // event bubbling). See `manageEvent` for more information.
-  handle?: boolean;
-
-  // If true, all SceneryEvents that trigger this listener (keydown and keyup) will be `aborted` (no more
-  // event bubbling, no more listeners fire). See `manageEvent` for more information.
-  abort?: boolean;
-
   // Called when the listener detects that the set of keys are pressed.
-  callback?: ( event: SceneryEvent<KeyboardEvent> | null, keysPressed: Keys[number], listener: KeyboardListener<Keys> ) => void;
-
-  // Called when the listener is cancelled/interrupted.
-  cancel?: ( listener: KeyboardListener<Keys> ) => void;
+  fire?: ( event: KeyboardEvent | null, keysPressed: Keys[number], listener: KeyboardListener<Keys> ) => void;
 
   // Called when the listener target receives focus.
   focus?: ( listener: KeyboardListener<Keys> ) => void;
@@ -127,6 +108,10 @@
   // Possible input types that decide when callbacks of the listener fire in response to input. See
   // ListenerFireTrigger type documentation.
   listenerFireTrigger?: ListenerFireTrigger;
+
+  // The Node that will be the target for global key events. Listener will only fire if this Node can
+  // receive input events. If not provided, the listener must be added to a specific Node with addInputListener().
+  globalTargetNode?: Node | null;
 };
 
 type KeyGroup<Keys extends readonly OneKeyStroke[]> = {
@@ -149,10 +134,7 @@
   // The function called when a KeyGroup is pressed (or just released). Provides the SceneryEvent that fired the input
   // listeners and this the keys that were pressed from the active KeyGroup. The event may be null when using
   // fireOnHold or in cases of cancel or interrupt. A reference to the listener is provided for other state.
-  private readonly _callback: ( event: SceneryEvent<KeyboardEvent> | null, keysPressed: Keys[number], listener: KeyboardListener<Keys> ) => void;
-
-  // The optional function called when this listener is cancelled.
-  private readonly _cancel: ( listener: KeyboardListener<Keys> ) => void;
+  private readonly _fire: ( event: KeyboardEvent | null, keysPressed: Keys[number], listener: KeyboardListener<Keys> ) => void;
 
   // The optional function called when this listener's target receives focus.
   private readonly _focus: ( listener: KeyboardListener<Keys> ) => void;
@@ -166,9 +148,6 @@
   // Does the listener fire the callback continuously when keys are held down?
   private readonly _fireOnHold: boolean;
 
-  // (scenery-internal) All the KeyGroups of this listener from the keys provided in natural language.
-  public readonly _keyGroups: KeyGroup<Keys>[];
-
   // All the KeyGroups that are currently firing
   private readonly _activeKeyGroups: KeyGroup<Keys>[];
 
@@ -182,29 +161,21 @@
 
   // see options documentation
   private readonly _global: boolean;
-  private readonly _handle: boolean;
-  private readonly _abort: boolean;
-
-  private readonly _windowFocusListener: ( windowHasFocus: boolean ) => void;
 
   public constructor( providedOptions: KeyboardListenerOptions<Keys> ) {
     const options = optionize<KeyboardListenerOptions<Keys>>()( {
-      callback: _.noop,
-      cancel: _.noop,
+      fire: _.noop,
       focus: _.noop,
       blur: _.noop,
       global: false,
-      capture: false,
-      handle: false,
-      abort: false,
       listenerFireTrigger: 'down',
       fireOnHold: false,
       fireOnHoldDelay: 400,
-      fireOnHoldInterval: 100
+      fireOnHoldInterval: 100,
+      globalTargetNode: null
     }, providedOptions );
 
-    this._callback = options.callback;
-    this._cancel = options.cancel;
+    this._fire = options.fire;
     this._focus = options.focus;
     this._blur = options.blur;
 
@@ -218,293 +189,26 @@
     this.keysDown = false;
 
     this._global = options.global;
-    this._handle = options.handle;
-    this._abort = options.abort;
 
-    // convert the provided keys to data that we can respond to with scenery's Input system
-    this._keyGroups = this.convertKeysToKeyGroups( options.keys );
-
-    // Assign listener and capture to this, implementing TInputListener
+    // Assign listener to this, implementing TInputListener
     ( this as unknown as TInputListener ).listener = this;
-    ( this as unknown as TInputListener ).capture = options.capture;
 
-    this._windowFocusListener = this.handleWindowFocusChange.bind( this );
-    FocusManager.windowHasFocusProperty.link( this._windowFocusListener );
+    // convert the provided keys to data that we can respond to with scenery's Input system
+    ( this as unknown as TInputListener ).hotkeys = this.createHotkeys( options.keys, options.globalTargetNode );
   }
 
   /**
    * 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 );
-  }
-
-  /**
-   * Responding to a keydown event, update active KeyGroups and potentially fire callbacks and start CallbackTimers.
-   */
-  private handleKeyDown( event: SceneryEvent<KeyboardEvent> ): void {
-    if ( this._listenerFireTrigger === 'down' || this._listenerFireTrigger === 'both' ) {
-
-      // modifier keys can be pressed in any order but the last key in the group must be pressed last
-      this._keyGroups.forEach( keyGroup => {
-
-        if ( !this._activeKeyGroups.includes( keyGroup ) ) {
-          if ( this.areKeysDownForListener( keyGroup ) &&
-               keyGroup.keys.includes( globalKeyStateTracker.getLastKeyDown()! ) ) {
-
-            this._activeKeyGroups.push( keyGroup );
-
-            this.keysDown = true;
-
-            // reserve the event for this listener, disabling more 'global' input listeners such as
-            // those for pan and zoom (this is similar to DOM event.preventDefault).
-            event.pointer.reserveForKeyboardDrag();
-
-            if ( keyGroup.timer ) {
-              keyGroup.timer.start();
-            }
-
-            this.fireCallback( event, keyGroup );
-          }
-        }
-      } );
-    }
-
-    this.manageEvent( event );
-  }
-
-  /**
-   * If there are any active KeyGroup firing stop and remove if KeyGroup keys are no longer down. Also, potentially
-   * fires a KeyGroup callback if the key that was released has all other modifier keys down.
-   */
-  private handleKeyUp( event: SceneryEvent<KeyboardEvent> ): void {
-
-    if ( this._activeKeyGroups.length > 0 ) {
-      this._activeKeyGroups.forEach( ( activeKeyGroup, index ) => {
-        if ( !this.areKeysDownForListener( activeKeyGroup ) ) {
-          if ( activeKeyGroup.timer ) {
-            activeKeyGroup.timer.stop( false );
-          }
-          this._activeKeyGroups.splice( index, 1 );
-        }
-      } );
-    }
-
-    if ( this._listenerFireTrigger === 'up' || this._listenerFireTrigger === 'both' ) {
-      const eventCode = KeyboardUtils.getEventCode( event.domEvent )!;
-
-      // Screen readers may send key events with no code for unknown reasons, we need to be graceful when that
-      // happens, see https://github.com/phetsims/scenery/issues/1534.
-      if ( eventCode ) {
-        this._keyGroups.forEach( keyGroup => {
-          if ( this.areModifierKeysDownForListener( keyGroup ) &&
-               keyGroup.keys.includes( eventCode ) ) {
-            this.keysDown = false;
-            this.fireCallback( event, keyGroup );
-          }
-        } );
-      }
-    }
-
-    this.manageEvent( event );
-  }
-
-  /**
-   * Returns an array of KeyboardEvent.codes from the provided key group that are currently pressed down.
-   */
-  private getDownModifierKeys( keyGroup: KeyGroup<Keys> ): string[] {
-
-    // Remember, this is a 2D array. The inner array is the list of 'equivalent' keys to be pressed for the required
-    // modifier key. For example [ 'shiftLeft', 'shiftRight' ]. If any of the keys in that inner array are pressed,
-    // that set of modifier keys is considered pressed.
-    const modifierKeysCollection = keyGroup.modifierKeys;
-
-    // The list of modifier keys that are actually pressed
-    const downModifierKeys: string[] = [];
-    modifierKeysCollection.forEach( modifierKeys => {
-      for ( const modifierKey of modifierKeys ) {
-        if ( globalKeyStateTracker.isKeyDown( modifierKey ) ) {
-          downModifierKeys.push( modifierKey );
-
-          // One modifier key from this inner set is down, stop looking
-          break;
-        }
-      }
-    } );
-
-    return downModifierKeys;
-  }
-
-  /**
-   * Returns true if keys are pressed such that the listener should fire. In order to fire, all modifier keys
-   * should be down and the final key of the group should be down. If any extra modifier keys are down that are
-   * not specified in the keyGroup, the listener will not fire.
-   */
-  private areKeysDownForListener( keyGroup: KeyGroup<Keys> ): boolean {
-    const downModifierKeys = this.getDownModifierKeys( keyGroup );
-
-    // modifier keys are down if one key from each inner array is down
-    const areModifierKeysDown = downModifierKeys.length === keyGroup.modifierKeys.length;
-
-    // The final key of the group is down if any of them are pressed
-    const finalDownKey = keyGroup.keys.find( key => globalKeyStateTracker.isKeyDown( key ) );
-
-    if ( areModifierKeysDown && !!finalDownKey ) {
-
-      // 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 );
-    }
-    else {
-      return false;
-    }
-  }
-
-  /**
-   * Returns true if the modifier keys of the provided key group are currently down. If any extra modifier keys are
-   * down that are not specified in the keyGroup, the listener will not fire.
-   */
-  private areModifierKeysDownForListener( keyGroup: KeyGroup<Keys> ): boolean {
-    const downModifierKeys = this.getDownModifierKeys( keyGroup );
-
-    // modifier keys are down if one key from each inner array is down
-    const modifierKeysDown = downModifierKeys.length === keyGroup.modifierKeys.length;
-
-    if ( modifierKeysDown ) {
-
-      // If there are any extra modifier keys down, the listener will not fire
-      return globalKeyStateTracker.areKeysDownWithoutExtraModifiers( downModifierKeys );
-    }
-    else {
-      return false;
-    }
-  }
-
-  /**
-   * In response to every SceneryEvent, handle and/or abort depending on listener options. This cannot be done in
-   * the callbacks because press-and-hold behavior triggers many keydown events. We need to handle/abort each, not
-   * just the event that triggered the callback. Also, callbacks can be called without a SceneryEvent from the
-   * CallbackTimer.
-   */
-  private manageEvent( event: SceneryEvent<KeyboardEvent> ): void {
-    this._handle && event.handle();
-    this._abort && event.abort();
-  }
-
-  /**
-   * This is part of the scenery Input API (implementing TInputListener). Handle the keydown event when not
-   * 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 ) {
-      this.handleKeyDown( event );
-    }
-  }
-
-  /**
-   * This is part of the scenery Input API (implementing TInputListener). Handle the keyup event when not
-   * 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 ) {
-      this.handleKeyUp( event );
-    }
-  }
-
-  /**
-   * This is part of the scenery Input API (implementing TInputListener). Handle the global keydown event.
-   * Event has no target.
-   */
-  public globalkeydown( event: SceneryEvent<KeyboardEvent> ): void {
-    if ( this._global ) {
-      this.handleKeyDown( event );
-    }
-  }
-
-  /**
-   * This is part of the scenery Input API (implementing TInputListener). Handle the global keyup event.
-   * Event has no target.
-   */
-  public globalkeyup( event: SceneryEvent<KeyboardEvent> ): void {
-    if ( this._global ) {
-      this.handleKeyUp( event );
-    }
-  }
-
-  /**
-   * Work to be done on both cancel and interrupt.
-   */
-  private handleCancel(): void {
-    this.clearActiveKeyGroups();
-    this._cancel( this );
-  }
-
-  /**
-   * When the window loses focus, cancel.
-   */
-  private handleWindowFocusChange( windowHasFocus: boolean ): void {
-    if ( !windowHasFocus ) {
-      this.handleCancel();
-    }
-  }
-
-  /**
-   * Part of the scenery listener API. On cancel, clear active KeyGroups and stop their behavior.
-   */
-  public cancel(): void {
-    this.handleCancel();
-  }
-
-  /**
-   * Part of the scenery listener API. Clear active KeyGroups and stop their callbacks.
-   */
-  public interrupt(): void {
-    this.handleCancel();
-  }
-
-  /**
-   * Interrupts and resets the listener on blur so that state is reset and active keyGroups are cleared.
-   * Public because this is called with the scenery listener API. Do not call this directly.
-   */
-  public focusout( event: SceneryEvent ): void {
-    this.interrupt();
-
-    // Optional work to do on blur.
-    this._blur( this );
-  }
-
-  /**
-   * Public because this is called through the scenery listener API. Do not call this directly.
-   */
-  public focusin( event: SceneryEvent ): void {
-
-    // Optional work to do on focus.
-    this._focus( this );
+  public fireCallback( event: KeyboardEvent | null, naturalKeys: Keys[number] ): void {
+    this._fire( event, naturalKeys, this );
   }
 
   /**
    * Dispose of this listener by disposing of any Callback timers. Then clear all KeyGroups.
    */
   public dispose(): void {
-    this._keyGroups.forEach( activeKeyGroup => {
-      activeKeyGroup.timer && activeKeyGroup.timer.dispose();
-    } );
-    this._keyGroups.length = 0;
-
-    FocusManager.windowHasFocusProperty.unlink( this._windowFocusListener );
-  }
-
-  /**
-   * Clear the active KeyGroups on this listener. Stopping any active groups if they use a CallbackTimer.
-   */
-  private clearActiveKeyGroups(): void {
-    this._activeKeyGroups.forEach( activeKeyGroup => {
-      activeKeyGroup.timer && activeKeyGroup.timer.stop( false );
-    } );
-
-    this._activeKeyGroups.length = 0;
+    ( this as unknown as TInputListener ).hotkeys!.forEach( hotkey => hotkey.dispose() );
   }
 
   /**
@@ -512,45 +216,49 @@
    * will take a string that defines the keys for this listener like 'a+c|1+2+3+4|shift+leftArrow' and return an array
    * with three KeyGroups, one describing 'a+c', one describing '1+2+3+4' and one describing 'shift+leftArrow'.
    */
-  private convertKeysToKeyGroups( keys: Keys ): KeyGroup<Keys>[] {
+  private createHotkeys( keys: Keys, globalTargetNode: Node | null ): Hotkey[] {
+
+    return keys.map( naturalKeys => {
 
-    const keyGroups = keys.map( naturalKeys => {
+      // Split the keys into the main key and the modifier keys
+      const keys = naturalKeys.split( '+' );
+      const modifierKeys = keys.slice( 0, keys.length - 1 );
+      const naturalKey = keys[ keys.length - 1 ];
 
-      // all of the keys in this group in an array
-      const groupKeys = naturalKeys.split( '+' );
-      assert && assert( groupKeys.length > 0, 'no keys provided?' );
+      // If the globalTargetNode exists, assemble an enabledProperty for the hotkey that describes
+      // whether the Node can receive global events.
 
-      const naturalKey = groupKeys.slice( -1 )[ 0 ] as AllowedKeys;
-      const keys = EnglishStringToCodeMap[ naturalKey ]!;
-      assert && assert( keys, `Codes were not found, do you need to add it to EnglishStringToCodeMap? ${naturalKey}` );
+      // This describes when we can receive input events:
+      //   if ( !node.isDisposed && node.isVisible() && node.isInputEnabled() && node.isPDOMVisible() )
+      // For every trail from the globalTargetNode that is attached to the display,
+      // Add a listener to each of those attributes to update the enabledProperty.
+      // AND would re-compute whenever trails change??
+      const hotkeyOptions: HotkeyOptions = {
+        key: naturalKey as EnglishKey,
+        modifierKeys: modifierKeys as EnglishKey[],
+        fire: ( event: KeyboardEvent | null ) => {
+          this.fireCallback( event, naturalKeys );
+        }
+      };
+      if ( globalTargetNode ) {
+        hotkeyOptions.enabledProperty = new DisplayedProperty( globalTargetNode );
+      }
 
-      let modifierKeys: string[][] = [];
-      if ( groupKeys.length > 1 ) {
-        const naturalModifierKeys = groupKeys.slice( 0, groupKeys.length - 1 ) as ModifierKey[];
-        modifierKeys = naturalModifierKeys.map( naturalModifierKey => {
-          const modifierKeys = EnglishStringToCodeMap[ naturalModifierKey ]!;
-          assert && assert( modifierKeys, `Key not found, do you need to add it to EnglishStringToCodeMap? ${naturalModifierKey}` );
-          return modifierKeys;
-        } );
-      }
+      const hotkey = new Hotkey( hotkeyOptions );
+      globalHotkeyRegistry.add( hotkey );
+      return hotkey;
+    } );
+  }
 
-      // Set up the timer for triggering callbacks if this listener supports press and hold behavior
-      const timer = this._fireOnHold ? new CallbackTimer( {
-        callback: () => this.fireCallback( null, keyGroup ),
-        delay: this._fireOnHoldDelay,
-        interval: this._fireOnHoldInterval
-      } ) : null;
+  public static createGlobalKeyboardListener( providedOptions: KeyboardListenerOptions<any>, target: Node ): void {
 
-      const keyGroup: KeyGroup<Keys> = {
-        keys: keys,
-        modifierKeys: modifierKeys,
-        naturalKeys: naturalKeys,
-        timer: timer
-      };
-      return keyGroup;
-    } );
+    // Need to set the enabled Property on the HOtkey.
+    // Need to update that enabled Property when the Node state for how it can receive inptu changes.
+    const listenerOptions = _.merge( {
+      globalTargetNode: target
+    }, providedOptions );
 
-    return keyGroups;
+    const listener = new KeyboardListener( listenerOptions );
   }
 }
 
Index: scenery/js/input/Hotkey.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/scenery/js/input/Hotkey.ts b/scenery/js/input/Hotkey.ts
--- a/scenery/js/input/Hotkey.ts	(revision df9ab1245ac2fd0a5f2e1832bf54e1f5c750ba5e)
+++ b/scenery/js/input/Hotkey.ts	(date 1712003324171)
@@ -67,6 +67,8 @@
   // For each main `key`, the hotkey system will only allow one hotkey with allowOverlap:false to be active at any time.
   // This is provided to allow multiple hotkeys with the same keys to fire. Default is false.
   allowOverlap?: boolean;
+
+  isGloballyEnabled?: () => boolean;
 };
 
 export type HotkeyOptions = SelfOptions & EnabledComponentOptions;
@@ -82,6 +84,7 @@
   public readonly fireOnHold: boolean;
   public readonly fireOnHoldTiming: HotkeyFireOnHoldTiming;
   public readonly allowOverlap: boolean;
+  public readonly isGloballyEnabled: () => boolean;
 
   // All keys that are part of this hotkey (key + modifierKeys)
   public readonly keys: EnglishKey[];
@@ -118,7 +121,8 @@
       fireOnHoldTiming: 'browser',
       fireOnHoldCustomDelay: 400,
       fireOnHoldCustomInterval: 100,
-      allowOverlap: false
+      allowOverlap: false,
+      isGloballyEnabled: () => true
     }, providedOptions );
 
     super( options );
@@ -132,6 +136,7 @@
     this.fireOnHold = options.fireOnHold;
     this.fireOnHoldTiming = options.fireOnHoldTiming;
     this.allowOverlap = options.allowOverlap;
+    this.isGloballyEnabled = options.isGloballyEnabled;
 
     this.keys = _.uniq( [ this.key, ...this.modifierKeys ] );
 
Index: scenery/js/display/Display.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/scenery/js/display/Display.ts b/scenery/js/display/Display.ts
--- a/scenery/js/display/Display.ts	(revision df9ab1245ac2fd0a5f2e1832bf54e1f5c750ba5e)
+++ b/scenery/js/display/Display.ts	(date 1712008671440)
@@ -614,7 +614,7 @@
     // pre-repaint phase: update relative transform information for listeners (notification) and precomputation where desired
     this.updateDirtyTransformRoots();
     // pre-repaint phase update visibility information on instances
-    this._baseInstance!.updateVisibility( true, true, true, false );
+    this._baseInstance!.updateVisibility( true, true, true, true, false );
     if ( assertSlow ) { this._baseInstance!.auditVisibility( true ); }
 
     if ( assertSlow ) { this._baseInstance!.audit( this._frameId, true ); }
Index: scenery/js/display/Instance.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/scenery/js/display/Instance.js b/scenery/js/display/Instance.js
--- a/scenery/js/display/Instance.js	(revision df9ab1245ac2fd0a5f2e1832bf54e1f5c750ba5e)
+++ b/scenery/js/display/Instance.js	(date 1712008568683)
@@ -106,6 +106,7 @@
     this.visibilityDirty = true; // entire subtree of visibility will need to be updated
     this.childVisibilityDirty = true; // an ancestor needs its visibility updated
     this.voicingVisible = true; // whether this instance is "visible" for Voicing and allows speech with that feature
+    this.inputEnabled = true; // whether this instance is "visible" for input (e.g. pointer events)
 
     // @private {Object.<instanceId:number,number>} - Maps another instance's `instance.id` {number} => branch index
     // {number} (first index where the two trails are different). This effectively operates as a cache (since it's more
@@ -153,6 +154,7 @@
     this.relativeVisibleEmitter = new TinyEmitter();
     this.selfVisibleEmitter = new TinyEmitter();
     this.canVoiceEmitter = new TinyEmitter();
+    this.inputEnabledEmitter = new TinyEmitter();
 
     this.cleanInstance( display, trail );
 
@@ -1532,11 +1534,12 @@
    * @public
    *
    * @param {boolean} parentGloballyVisible - Whether our parent (if any) is globally visible
+   * @param {boolean} parentGloballyInputEnabled - Whether our parent (inf any) is globally inputEnabled
    * @param {boolean} parentGloballyVoicingVisible - Whether our parent (if any) is globally voicingVisible.
    * @param {boolean} parentRelativelyVisible - Whether our parent (if any) is relatively visible
    * @param {boolean} updateFullSubtree - If true, we will visit the entire subtree to ensure visibility is correct.
    */
-  updateVisibility( parentGloballyVisible, parentGloballyVoicingVisible, parentRelativelyVisible, updateFullSubtree ) {
+  updateVisibility( parentGloballyVisible, parentGloballyInputEnabled, parentGloballyVoicingVisible, parentRelativelyVisible, updateFullSubtree ) {
     // If our visibility flag for ourself is dirty, we need to update our entire subtree
     if ( this.visibilityDirty ) {
       updateFullSubtree = true;
@@ -1549,11 +1552,14 @@
     const wasSelfVisible = this.selfVisible;
     const nodeVoicingVisible = this.node.voicingVisibleProperty.value;
     const wasVoicingVisible = this.voicingVisible;
+    const nodeInputEnabled = this.node.inputEnabled;
+    const wasInputEnabled = this.inputEnabled;
     const couldVoice = wasVisible && wasVoicingVisible;
     this.visible = parentGloballyVisible && nodeVisible;
     this.voicingVisible = parentGloballyVoicingVisible && nodeVoicingVisible;
     this.relativeVisible = parentRelativelyVisible && nodeVisible;
     this.selfVisible = this.isVisibilityApplied ? true : this.relativeVisible;
+    this.inputEnabled = parentGloballyInputEnabled && nodeInputEnabled;
 
     const len = this.children.length;
     for ( let i = 0; i < len; i++ ) {
@@ -1561,7 +1567,7 @@
 
       if ( updateFullSubtree || child.visibilityDirty || child.childVisibilityDirty ) {
         // if we are a visibility root (isVisibilityApplied===true), disregard ancestor visibility
-        child.updateVisibility( this.visible, this.voicingVisible, this.isVisibilityApplied ? true : this.relativeVisible, updateFullSubtree );
+        child.updateVisibility( this.visible, this.inputEnabled, this.voicingVisible, this.isVisibilityApplied ? true : this.relativeVisible, updateFullSubtree );
       }
     }
 
@@ -1578,6 +1584,9 @@
     if ( this.selfVisible !== wasSelfVisible ) {
       this.selfVisibleEmitter.emit();
     }
+    if ( this.inputEnabled !== wasInputEnabled ) {
+      this.inputEnabledEmitter.emit();
+    }
 
     // An Instance can voice when it is globally visible and voicingVisible. Notify when this state has changed
     // based on these dependencies.
@@ -1836,6 +1845,7 @@
     this.relativeVisibleEmitter.removeAllListeners();
     this.selfVisibleEmitter.removeAllListeners();
     this.canVoiceEmitter.removeAllListeners();
+    this.inputEnabledEmitter.removeAllListeners();
 
     this.freeToPool();
 
Index: wilder/js/wilder/view/WilderScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/wilder/js/wilder/view/WilderScreenView.ts b/wilder/js/wilder/view/WilderScreenView.ts
--- a/wilder/js/wilder/view/WilderScreenView.ts	(revision 4812e57d2a01b566c4dbe16105fcd070b505eb89)
+++ b/wilder/js/wilder/view/WilderScreenView.ts	(date 1712008865313)
@@ -11,7 +11,7 @@
 import WilderModel from '../model/WilderModel.js';
 import { PhetioObjectOptions } from '../../../../tandem/js/PhetioObject.js';
 import PickRequired from '../../../../phet-core/js/types/PickRequired.js';
-import { globalHotkeyRegistry, Hotkey, Text } from '../../../../scenery/js/imports.js';
+import { globalHotkeyRegistry, Hotkey, KeyboardListener, Text } from '../../../../scenery/js/imports.js';
 import BooleanProperty from '../../../../axon/js/BooleanProperty.js';
 import ABSwitch from '../../../../sun/js/ABSwitch.js';
 import PhetFont from '../../../../scenery-phet/js/PhetFont.js';
@@ -119,37 +119,51 @@
       } )
     } ) );
 
-    resetAllButton.addInputListener( {
-      hotkeys: [
-        new Hotkey( {
-          key: 'x',
-          fire: () => console.log( 'fire: x' ),
-          enabledProperty: extraEnabledProperty
-        } ),
-        new Hotkey( {
-          key: 'x',
-          modifierKeys: [ 'b' ],
-          fire: () => console.log( 'fire: b+x' ),
-          enabledProperty: extraEnabledProperty
-        } ),
-        new Hotkey( {
-          key: 'w',
-          fire: () => console.log( 'fire: w' )
-        } ),
-        new Hotkey( {
-          key: 'a',
-          fire: () => console.log( 'fire: a' )
-        } ),
-        new Hotkey( {
-          key: 's',
-          fire: () => console.log( 'fire: s' )
-        } ),
-        new Hotkey( {
-          key: 'd',
-          fire: () => console.log( 'fire: d' )
-        } )
-      ]
-    } );
+    resetAllButton.addInputListener( new KeyboardListener( {
+      keys: [ 'x' ],
+      fire: () => console.log( 'fire: x from KeyboardListener' )
+    } ) );
+
+    KeyboardListener.createGlobalKeyboardListener( {
+      keys: [ 'x' ],
+      fire: () => console.log( 'fire: h from global KeyboardListener' )
+    }, resetAllButton );
+
+    window.setInterval( () => {
+      resetAllButton.inputEnabled = !resetAllButton.inputEnabled;
+    }, 2000 );
+
+    // resetAllButton.addInputListener( {
+    //   hotkeys: [
+    //     new Hotkey( {
+    //       key: 'x',
+    //       fire: () => console.log( 'fire: x' ),
+    //       enabledProperty: extraEnabledProperty
+    //     } ),
+    //     new Hotkey( {
+    //       key: 'x',
+    //       modifierKeys: [ 'b' ],
+    //       fire: () => console.log( 'fire: b+x' ),
+    //       enabledProperty: extraEnabledProperty
+    //     } ),
+    //     new Hotkey( {
+    //       key: 'w',
+    //       fire: () => console.log( 'fire: w' )
+    //     } ),
+    //     new Hotkey( {
+    //       key: 'a',
+    //       fire: () => console.log( 'fire: a' )
+    //     } ),
+    //     new Hotkey( {
+    //       key: 's',
+    //       fire: () => console.log( 'fire: s' )
+    //     } ),
+    //     new Hotkey( {
+    //       key: 'd',
+    //       fire: () => console.log( 'fire: d' )
+    //     } )
+    //   ]
+    // } );
   }
 }
 
Index: scenery/js/util/DisplayedProperty.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/scenery/js/util/DisplayedProperty.js b/scenery/js/util/DisplayedProperty.js
--- a/scenery/js/util/DisplayedProperty.js	(revision df9ab1245ac2fd0a5f2e1832bf54e1f5c750ba5e)
+++ b/scenery/js/util/DisplayedProperty.js	(date 1712008905100)
@@ -32,7 +32,10 @@
   constructor( node, options ) {
 
     options = merge( {
-      display: null // {Display|null} if null, this will check on any Display
+      display: null, // {Display|null} if null, this will check on any Display
+
+      watchPDOMDisplayed: false,
+      watchInputEnabled: false
     }, options );
 
     super( false, options );
@@ -48,7 +51,7 @@
     this.changedInstanceListener = this.changedInstance.bind( this );
 
     node.changedInstanceEmitter.addListener( this.changedInstanceListener );
-    // node.pdomDisplaysEmitter.addListener( this.updateListener ); // TODO support pdom visibility, https://github.com/phetsims/scenery/issues/1167
+    node.pdomDisplaysEmitter.addListener( this.updateListener ); // TODO support pdom visibility, https://github.com/phetsims/scenery/issues/1167
 
     // Add any instances the node may already have/
     const instances = node.instances;
@@ -62,10 +65,11 @@
    * @private
    */
   updateValue() {
-    this.value = this.node.wasVisuallyDisplayed( this.display );
+    // this.value = this.node.wasVisuallyDisplayed( this.display );
 
     // TODO support pdom visibility, https://github.com/phetsims/scenery/issues/1167
-    // this.value = this.node.wasVisuallyDisplayed( this.display ) || this.node.isPDOMDisplayed();
+    this.value = this.node.wasVisuallyDisplayed( this.display ) && this.node.isPDOMDisplayed() && this.node.isInputEnabled();
+    console.log( this.value );
   }
 
   /**
@@ -78,9 +82,11 @@
   changedInstance( instance, added ) {
     if ( added ) {
       instance.visibleEmitter.addListener( this.updateListener );
+      instance.inputEnabledEmitter.addListener( this.updateListener );
     }
     else {
       instance.visibleEmitter.removeListener( this.updateListener );
+      instance.inputEnabledEmitter.removeListener( this.updateListener );
     }
 
     this.updateValue();
@@ -101,7 +107,7 @@
     this.node.changedInstanceEmitter.removeListener( this.changedInstanceListener );
 
     // TODO support pdom visibility, https://github.com/phetsims/scenery/issues/1167
-    // this.node.pdomDisplaysEmitter.removeListener( this.updateListener );
+    this.node.pdomDisplaysEmitter.removeListener( this.updateListener );
 
     super.dispose();
   }

@jessegreenberg
Copy link
Contributor Author

jessegreenberg commented Apr 1, 2024

Here is a patch with a brute force method.

  1. A new CanReceiveInputEventsProperty that manually tracks the Trail for changes to inputEnabled. Composed with DisplayedProperty as well.
  2. A static in KeyboardListener that creates a global one with hotkeys added to the global registry.
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 df9ab1245ac2fd0a5f2e1832bf54e1f5c750ba5e)
+++ b/scenery/js/listeners/KeyboardListener.ts	(date 1712014053410)
@@ -16,7 +16,7 @@
  *
  * this.addInputListener( new KeyboardListener( {
  *   keys: [ 'a+b', 'a+c', 'shift+arrowLeft', 'alt+g+t', 'ctrl+3', 'alt+ctrl+t' ],
- *   callback: ( event, keysPressed, listener ) => {
+ *   fire: ( event, keysPressed, listener ) => {
  *     if ( keysPressed === 'a+b' ) {
  *       console.log( 'you just pressed a+b!' );
  *     }
@@ -35,11 +35,11 @@
  *   }
  * } ) );
  *
- * By default the callback will fire when the last key is pressed down. See additional options for firing on key
+ * By default the fire callback will fire when the last key is pressed down. See additional options for firing on key
  * up or other press and hold behavior.
  *
  * **Potential Pitfall!**
- * The callback is only called if exactly the keys in a group are pressed. If you need to listen to a modifier key,
+ * The fire callback is only called if exactly the keys in a group are pressed. If you need to listen to a modifier key,
  * you must include it in the keys array. For example if you add a listener for 'tab', you must ALSO include
  * 'shift+tab' in the array to observe 'shift+tab' presses. If you provide 'tab' alone, the callback will not fire
  * if 'shift' is also pressed.
@@ -49,8 +49,7 @@
 
 import CallbackTimer from '../../../axon/js/CallbackTimer.js';
 import optionize from '../../../phet-core/js/optionize.js';
-import { EnglishStringToCodeMap, FocusManager, globalKeyStateTracker, scenery, SceneryEvent, TInputListener } from '../imports.js';
-import KeyboardUtils from '../accessibility/KeyboardUtils.js';
+import { CanReceiveInputEventsProperty, EnglishKey, EnglishStringToCodeMap, globalHotkeyRegistry, Hotkey, HotkeyOptions, Node, scenery, TInputListener } from '../imports.js';
 
 // NOTE: The typing for ModifierKey and OneKeyStroke is limited TypeScript, there is a limitation to the number of
 //       entries in a union type. If that limitation is not acceptable remove this typing. OR maybe TypeScript will
@@ -85,29 +84,11 @@
 
   // If true, the listener will fire for keys regardless of where focus is in the document. Use this when you want
   // to add some key press behavior that will always fire no matter what the event target is. If this listener
-  // is added to a Node, it will only fire if the Node (and all of its ancestors) are visible with inputEnabled: true.
-  // More specifically, this uses `globalKeyUp` and `globalKeyDown`. See definitions in Input.ts for more information.
+  // is added to a Node, it will only fire if the Node (and all of its ancestors) can receive input events.
   global?: boolean;
 
-  // If true, this listener is fired during the 'capture' phase. Only relevant for `global` key events.
-  // When a listener uses capture, the callbacks will be fired BEFORE the dispatch through the scene graph
-  // (very similar to DOM's addEventListener with `useCapture` set to true - see
-  // https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener).
-  capture?: boolean;
-
-  // If true, all SceneryEvents that trigger this listener (keydown and keyup) will be `handled` (no more
-  // event bubbling). See `manageEvent` for more information.
-  handle?: boolean;
-
-  // If true, all SceneryEvents that trigger this listener (keydown and keyup) will be `aborted` (no more
-  // event bubbling, no more listeners fire). See `manageEvent` for more information.
-  abort?: boolean;
-
   // Called when the listener detects that the set of keys are pressed.
-  callback?: ( event: SceneryEvent<KeyboardEvent> | null, keysPressed: Keys[number], listener: KeyboardListener<Keys> ) => void;
-
-  // Called when the listener is cancelled/interrupted.
-  cancel?: ( listener: KeyboardListener<Keys> ) => void;
+  fire?: ( event: KeyboardEvent | null, keysPressed: Keys[number], listener: KeyboardListener<Keys> ) => void;
 
   // Called when the listener target receives focus.
   focus?: ( listener: KeyboardListener<Keys> ) => void;
@@ -127,6 +108,10 @@
   // Possible input types that decide when callbacks of the listener fire in response to input. See
   // ListenerFireTrigger type documentation.
   listenerFireTrigger?: ListenerFireTrigger;
+
+  // The Node that will be the target for global key events. Listener will only fire if this Node can
+  // receive input events. If not provided, the listener must be added to a specific Node with addInputListener().
+  globalTargetNode?: Node | null;
 };
 
 type KeyGroup<Keys extends readonly OneKeyStroke[]> = {
@@ -149,10 +134,7 @@
   // The function called when a KeyGroup is pressed (or just released). Provides the SceneryEvent that fired the input
   // listeners and this the keys that were pressed from the active KeyGroup. The event may be null when using
   // fireOnHold or in cases of cancel or interrupt. A reference to the listener is provided for other state.
-  private readonly _callback: ( event: SceneryEvent<KeyboardEvent> | null, keysPressed: Keys[number], listener: KeyboardListener<Keys> ) => void;
-
-  // The optional function called when this listener is cancelled.
-  private readonly _cancel: ( listener: KeyboardListener<Keys> ) => void;
+  private readonly _fire: ( event: KeyboardEvent | null, keysPressed: Keys[number], listener: KeyboardListener<Keys> ) => void;
 
   // The optional function called when this listener's target receives focus.
   private readonly _focus: ( listener: KeyboardListener<Keys> ) => void;
@@ -166,9 +148,6 @@
   // Does the listener fire the callback continuously when keys are held down?
   private readonly _fireOnHold: boolean;
 
-  // (scenery-internal) All the KeyGroups of this listener from the keys provided in natural language.
-  public readonly _keyGroups: KeyGroup<Keys>[];
-
   // All the KeyGroups that are currently firing
   private readonly _activeKeyGroups: KeyGroup<Keys>[];
 
@@ -182,29 +161,21 @@
 
   // see options documentation
   private readonly _global: boolean;
-  private readonly _handle: boolean;
-  private readonly _abort: boolean;
-
-  private readonly _windowFocusListener: ( windowHasFocus: boolean ) => void;
 
   public constructor( providedOptions: KeyboardListenerOptions<Keys> ) {
     const options = optionize<KeyboardListenerOptions<Keys>>()( {
-      callback: _.noop,
-      cancel: _.noop,
+      fire: _.noop,
       focus: _.noop,
       blur: _.noop,
       global: false,
-      capture: false,
-      handle: false,
-      abort: false,
       listenerFireTrigger: 'down',
       fireOnHold: false,
       fireOnHoldDelay: 400,
-      fireOnHoldInterval: 100
+      fireOnHoldInterval: 100,
+      globalTargetNode: null
     }, providedOptions );
 
-    this._callback = options.callback;
-    this._cancel = options.cancel;
+    this._fire = options.fire;
     this._focus = options.focus;
     this._blur = options.blur;
 
@@ -218,293 +189,26 @@
     this.keysDown = false;
 
     this._global = options.global;
-    this._handle = options.handle;
-    this._abort = options.abort;
 
-    // convert the provided keys to data that we can respond to with scenery's Input system
-    this._keyGroups = this.convertKeysToKeyGroups( options.keys );
-
-    // Assign listener and capture to this, implementing TInputListener
+    // Assign listener to this, implementing TInputListener
     ( this as unknown as TInputListener ).listener = this;
-    ( this as unknown as TInputListener ).capture = options.capture;
 
-    this._windowFocusListener = this.handleWindowFocusChange.bind( this );
-    FocusManager.windowHasFocusProperty.link( this._windowFocusListener );
+    // convert the provided keys to data that we can respond to with scenery's Input system
+    ( this as unknown as TInputListener ).hotkeys = this.createHotkeys( options.keys, options.globalTargetNode );
   }
 
   /**
    * 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 );
-  }
-
-  /**
-   * Responding to a keydown event, update active KeyGroups and potentially fire callbacks and start CallbackTimers.
-   */
-  private handleKeyDown( event: SceneryEvent<KeyboardEvent> ): void {
-    if ( this._listenerFireTrigger === 'down' || this._listenerFireTrigger === 'both' ) {
-
-      // modifier keys can be pressed in any order but the last key in the group must be pressed last
-      this._keyGroups.forEach( keyGroup => {
-
-        if ( !this._activeKeyGroups.includes( keyGroup ) ) {
-          if ( this.areKeysDownForListener( keyGroup ) &&
-               keyGroup.keys.includes( globalKeyStateTracker.getLastKeyDown()! ) ) {
-
-            this._activeKeyGroups.push( keyGroup );
-
-            this.keysDown = true;
-
-            // reserve the event for this listener, disabling more 'global' input listeners such as
-            // those for pan and zoom (this is similar to DOM event.preventDefault).
-            event.pointer.reserveForKeyboardDrag();
-
-            if ( keyGroup.timer ) {
-              keyGroup.timer.start();
-            }
-
-            this.fireCallback( event, keyGroup );
-          }
-        }
-      } );
-    }
-
-    this.manageEvent( event );
-  }
-
-  /**
-   * If there are any active KeyGroup firing stop and remove if KeyGroup keys are no longer down. Also, potentially
-   * fires a KeyGroup callback if the key that was released has all other modifier keys down.
-   */
-  private handleKeyUp( event: SceneryEvent<KeyboardEvent> ): void {
-
-    if ( this._activeKeyGroups.length > 0 ) {
-      this._activeKeyGroups.forEach( ( activeKeyGroup, index ) => {
-        if ( !this.areKeysDownForListener( activeKeyGroup ) ) {
-          if ( activeKeyGroup.timer ) {
-            activeKeyGroup.timer.stop( false );
-          }
-          this._activeKeyGroups.splice( index, 1 );
-        }
-      } );
-    }
-
-    if ( this._listenerFireTrigger === 'up' || this._listenerFireTrigger === 'both' ) {
-      const eventCode = KeyboardUtils.getEventCode( event.domEvent )!;
-
-      // Screen readers may send key events with no code for unknown reasons, we need to be graceful when that
-      // happens, see https://github.com/phetsims/scenery/issues/1534.
-      if ( eventCode ) {
-        this._keyGroups.forEach( keyGroup => {
-          if ( this.areModifierKeysDownForListener( keyGroup ) &&
-               keyGroup.keys.includes( eventCode ) ) {
-            this.keysDown = false;
-            this.fireCallback( event, keyGroup );
-          }
-        } );
-      }
-    }
-
-    this.manageEvent( event );
-  }
-
-  /**
-   * Returns an array of KeyboardEvent.codes from the provided key group that are currently pressed down.
-   */
-  private getDownModifierKeys( keyGroup: KeyGroup<Keys> ): string[] {
-
-    // Remember, this is a 2D array. The inner array is the list of 'equivalent' keys to be pressed for the required
-    // modifier key. For example [ 'shiftLeft', 'shiftRight' ]. If any of the keys in that inner array are pressed,
-    // that set of modifier keys is considered pressed.
-    const modifierKeysCollection = keyGroup.modifierKeys;
-
-    // The list of modifier keys that are actually pressed
-    const downModifierKeys: string[] = [];
-    modifierKeysCollection.forEach( modifierKeys => {
-      for ( const modifierKey of modifierKeys ) {
-        if ( globalKeyStateTracker.isKeyDown( modifierKey ) ) {
-          downModifierKeys.push( modifierKey );
-
-          // One modifier key from this inner set is down, stop looking
-          break;
-        }
-      }
-    } );
-
-    return downModifierKeys;
-  }
-
-  /**
-   * Returns true if keys are pressed such that the listener should fire. In order to fire, all modifier keys
-   * should be down and the final key of the group should be down. If any extra modifier keys are down that are
-   * not specified in the keyGroup, the listener will not fire.
-   */
-  private areKeysDownForListener( keyGroup: KeyGroup<Keys> ): boolean {
-    const downModifierKeys = this.getDownModifierKeys( keyGroup );
-
-    // modifier keys are down if one key from each inner array is down
-    const areModifierKeysDown = downModifierKeys.length === keyGroup.modifierKeys.length;
-
-    // The final key of the group is down if any of them are pressed
-    const finalDownKey = keyGroup.keys.find( key => globalKeyStateTracker.isKeyDown( key ) );
-
-    if ( areModifierKeysDown && !!finalDownKey ) {
-
-      // 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 );
-    }
-    else {
-      return false;
-    }
-  }
-
-  /**
-   * Returns true if the modifier keys of the provided key group are currently down. If any extra modifier keys are
-   * down that are not specified in the keyGroup, the listener will not fire.
-   */
-  private areModifierKeysDownForListener( keyGroup: KeyGroup<Keys> ): boolean {
-    const downModifierKeys = this.getDownModifierKeys( keyGroup );
-
-    // modifier keys are down if one key from each inner array is down
-    const modifierKeysDown = downModifierKeys.length === keyGroup.modifierKeys.length;
-
-    if ( modifierKeysDown ) {
-
-      // If there are any extra modifier keys down, the listener will not fire
-      return globalKeyStateTracker.areKeysDownWithoutExtraModifiers( downModifierKeys );
-    }
-    else {
-      return false;
-    }
-  }
-
-  /**
-   * In response to every SceneryEvent, handle and/or abort depending on listener options. This cannot be done in
-   * the callbacks because press-and-hold behavior triggers many keydown events. We need to handle/abort each, not
-   * just the event that triggered the callback. Also, callbacks can be called without a SceneryEvent from the
-   * CallbackTimer.
-   */
-  private manageEvent( event: SceneryEvent<KeyboardEvent> ): void {
-    this._handle && event.handle();
-    this._abort && event.abort();
-  }
-
-  /**
-   * This is part of the scenery Input API (implementing TInputListener). Handle the keydown event when not
-   * 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 ) {
-      this.handleKeyDown( event );
-    }
-  }
-
-  /**
-   * This is part of the scenery Input API (implementing TInputListener). Handle the keyup event when not
-   * 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 ) {
-      this.handleKeyUp( event );
-    }
-  }
-
-  /**
-   * This is part of the scenery Input API (implementing TInputListener). Handle the global keydown event.
-   * Event has no target.
-   */
-  public globalkeydown( event: SceneryEvent<KeyboardEvent> ): void {
-    if ( this._global ) {
-      this.handleKeyDown( event );
-    }
-  }
-
-  /**
-   * This is part of the scenery Input API (implementing TInputListener). Handle the global keyup event.
-   * Event has no target.
-   */
-  public globalkeyup( event: SceneryEvent<KeyboardEvent> ): void {
-    if ( this._global ) {
-      this.handleKeyUp( event );
-    }
-  }
-
-  /**
-   * Work to be done on both cancel and interrupt.
-   */
-  private handleCancel(): void {
-    this.clearActiveKeyGroups();
-    this._cancel( this );
-  }
-
-  /**
-   * When the window loses focus, cancel.
-   */
-  private handleWindowFocusChange( windowHasFocus: boolean ): void {
-    if ( !windowHasFocus ) {
-      this.handleCancel();
-    }
-  }
-
-  /**
-   * Part of the scenery listener API. On cancel, clear active KeyGroups and stop their behavior.
-   */
-  public cancel(): void {
-    this.handleCancel();
-  }
-
-  /**
-   * Part of the scenery listener API. Clear active KeyGroups and stop their callbacks.
-   */
-  public interrupt(): void {
-    this.handleCancel();
-  }
-
-  /**
-   * Interrupts and resets the listener on blur so that state is reset and active keyGroups are cleared.
-   * Public because this is called with the scenery listener API. Do not call this directly.
-   */
-  public focusout( event: SceneryEvent ): void {
-    this.interrupt();
-
-    // Optional work to do on blur.
-    this._blur( this );
-  }
-
-  /**
-   * Public because this is called through the scenery listener API. Do not call this directly.
-   */
-  public focusin( event: SceneryEvent ): void {
-
-    // Optional work to do on focus.
-    this._focus( this );
+  public fireCallback( event: KeyboardEvent | null, naturalKeys: Keys[number] ): void {
+    this._fire( event, naturalKeys, this );
   }
 
   /**
    * Dispose of this listener by disposing of any Callback timers. Then clear all KeyGroups.
    */
   public dispose(): void {
-    this._keyGroups.forEach( activeKeyGroup => {
-      activeKeyGroup.timer && activeKeyGroup.timer.dispose();
-    } );
-    this._keyGroups.length = 0;
-
-    FocusManager.windowHasFocusProperty.unlink( this._windowFocusListener );
-  }
-
-  /**
-   * Clear the active KeyGroups on this listener. Stopping any active groups if they use a CallbackTimer.
-   */
-  private clearActiveKeyGroups(): void {
-    this._activeKeyGroups.forEach( activeKeyGroup => {
-      activeKeyGroup.timer && activeKeyGroup.timer.stop( false );
-    } );
-
-    this._activeKeyGroups.length = 0;
+    ( this as unknown as TInputListener ).hotkeys!.forEach( hotkey => hotkey.dispose() );
   }
 
   /**
@@ -512,45 +216,41 @@
    * will take a string that defines the keys for this listener like 'a+c|1+2+3+4|shift+leftArrow' and return an array
    * with three KeyGroups, one describing 'a+c', one describing '1+2+3+4' and one describing 'shift+leftArrow'.
    */
-  private convertKeysToKeyGroups( keys: Keys ): KeyGroup<Keys>[] {
+  private createHotkeys( keys: Keys, globalTargetNode: Node | null ): Hotkey[] {
 
-    const keyGroups = keys.map( naturalKeys => {
+    return keys.map( naturalKeys => {
 
-      // all of the keys in this group in an array
-      const groupKeys = naturalKeys.split( '+' );
-      assert && assert( groupKeys.length > 0, 'no keys provided?' );
+      // Split the keys into the main key and the modifier keys
+      const keys = naturalKeys.split( '+' );
+      const modifierKeys = keys.slice( 0, keys.length - 1 );
+      const naturalKey = keys[ keys.length - 1 ];
 
-      const naturalKey = groupKeys.slice( -1 )[ 0 ] as AllowedKeys;
-      const keys = EnglishStringToCodeMap[ naturalKey ]!;
-      assert && assert( keys, `Codes were not found, do you need to add it to EnglishStringToCodeMap? ${naturalKey}` );
+      const hotkeyOptions: HotkeyOptions = {
+        key: naturalKey as EnglishKey,
+        modifierKeys: modifierKeys as EnglishKey[],
+        fire: ( event: KeyboardEvent | null ) => {
+          this.fireCallback( event, naturalKeys );
+        }
+      };
+      if ( globalTargetNode ) {
+        hotkeyOptions.enabledProperty = new CanReceiveInputEventsProperty( globalTargetNode );
+      }
 
-      let modifierKeys: string[][] = [];
-      if ( groupKeys.length > 1 ) {
-        const naturalModifierKeys = groupKeys.slice( 0, groupKeys.length - 1 ) as ModifierKey[];
-        modifierKeys = naturalModifierKeys.map( naturalModifierKey => {
-          const modifierKeys = EnglishStringToCodeMap[ naturalModifierKey ]!;
-          assert && assert( modifierKeys, `Key not found, do you need to add it to EnglishStringToCodeMap? ${naturalModifierKey}` );
-          return modifierKeys;
-        } );
-      }
+      const hotkey = new Hotkey( hotkeyOptions );
+      globalHotkeyRegistry.add( hotkey );
+      return hotkey;
+    } );
+  }
 
-      // Set up the timer for triggering callbacks if this listener supports press and hold behavior
-      const timer = this._fireOnHold ? new CallbackTimer( {
-        callback: () => this.fireCallback( null, keyGroup ),
-        delay: this._fireOnHoldDelay,
-        interval: this._fireOnHoldInterval
-      } ) : null;
+  public static createGlobalKeyboardListener( providedOptions: KeyboardListenerOptions<OneKeyStroke[]>, target: Node ): KeyboardListener<OneKeyStroke[]> {
 
-      const keyGroup: KeyGroup<Keys> = {
-        keys: keys,
-        modifierKeys: modifierKeys,
-        naturalKeys: naturalKeys,
-        timer: timer
-      };
-      return keyGroup;
-    } );
+    // Need to set the enabled Property on the HOtkey.
+    // Need to update that enabled Property when the Node state for how it can receive inptu changes.
+    const listenerOptions = _.merge( {
+      globalTargetNode: target
+    }, providedOptions );
 
-    return keyGroups;
+    return new KeyboardListener( listenerOptions );
   }
 }
 
Index: scenery/js/input/Hotkey.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/scenery/js/input/Hotkey.ts b/scenery/js/input/Hotkey.ts
--- a/scenery/js/input/Hotkey.ts	(revision df9ab1245ac2fd0a5f2e1832bf54e1f5c750ba5e)
+++ b/scenery/js/input/Hotkey.ts	(date 1712003324171)
@@ -67,6 +67,8 @@
   // For each main `key`, the hotkey system will only allow one hotkey with allowOverlap:false to be active at any time.
   // This is provided to allow multiple hotkeys with the same keys to fire. Default is false.
   allowOverlap?: boolean;
+
+  isGloballyEnabled?: () => boolean;
 };
 
 export type HotkeyOptions = SelfOptions & EnabledComponentOptions;
@@ -82,6 +84,7 @@
   public readonly fireOnHold: boolean;
   public readonly fireOnHoldTiming: HotkeyFireOnHoldTiming;
   public readonly allowOverlap: boolean;
+  public readonly isGloballyEnabled: () => boolean;
 
   // All keys that are part of this hotkey (key + modifierKeys)
   public readonly keys: EnglishKey[];
@@ -118,7 +121,8 @@
       fireOnHoldTiming: 'browser',
       fireOnHoldCustomDelay: 400,
       fireOnHoldCustomInterval: 100,
-      allowOverlap: false
+      allowOverlap: false,
+      isGloballyEnabled: () => true
     }, providedOptions );
 
     super( options );
@@ -132,6 +136,7 @@
     this.fireOnHold = options.fireOnHold;
     this.fireOnHoldTiming = options.fireOnHoldTiming;
     this.allowOverlap = options.allowOverlap;
+    this.isGloballyEnabled = options.isGloballyEnabled;
 
     this.keys = _.uniq( [ this.key, ...this.modifierKeys ] );
 
Index: scenery/js/imports.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/scenery/js/imports.ts b/scenery/js/imports.ts
--- a/scenery/js/imports.ts	(revision df9ab1245ac2fd0a5f2e1832bf54e1f5c750ba5e)
+++ b/scenery/js/imports.ts	(date 1712013574409)
@@ -33,6 +33,7 @@
 export { default as FullScreen } from './util/FullScreen.js';
 export { default as CountMap } from './util/CountMap.js';
 export { default as DisplayedProperty } from './util/DisplayedProperty.js';
+export { default as CanReceiveInputEventsProperty } from './util/CanReceiveInputEventsProperty.js';
 export { default as SceneImage } from './util/SceneImage.js';
 export { default as allowLinksProperty } from './util/allowLinksProperty.js';
 export { default as openPopup } from './util/openPopup.js';
Index: wilder/js/wilder/view/WilderScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/wilder/js/wilder/view/WilderScreenView.ts b/wilder/js/wilder/view/WilderScreenView.ts
--- a/wilder/js/wilder/view/WilderScreenView.ts	(revision 4812e57d2a01b566c4dbe16105fcd070b505eb89)
+++ b/wilder/js/wilder/view/WilderScreenView.ts	(date 1712008865313)
@@ -11,7 +11,7 @@
 import WilderModel from '../model/WilderModel.js';
 import { PhetioObjectOptions } from '../../../../tandem/js/PhetioObject.js';
 import PickRequired from '../../../../phet-core/js/types/PickRequired.js';
-import { globalHotkeyRegistry, Hotkey, Text } from '../../../../scenery/js/imports.js';
+import { globalHotkeyRegistry, Hotkey, KeyboardListener, Text } from '../../../../scenery/js/imports.js';
 import BooleanProperty from '../../../../axon/js/BooleanProperty.js';
 import ABSwitch from '../../../../sun/js/ABSwitch.js';
 import PhetFont from '../../../../scenery-phet/js/PhetFont.js';
@@ -119,37 +119,51 @@
       } )
     } ) );
 
-    resetAllButton.addInputListener( {
-      hotkeys: [
-        new Hotkey( {
-          key: 'x',
-          fire: () => console.log( 'fire: x' ),
-          enabledProperty: extraEnabledProperty
-        } ),
-        new Hotkey( {
-          key: 'x',
-          modifierKeys: [ 'b' ],
-          fire: () => console.log( 'fire: b+x' ),
-          enabledProperty: extraEnabledProperty
-        } ),
-        new Hotkey( {
-          key: 'w',
-          fire: () => console.log( 'fire: w' )
-        } ),
-        new Hotkey( {
-          key: 'a',
-          fire: () => console.log( 'fire: a' )
-        } ),
-        new Hotkey( {
-          key: 's',
-          fire: () => console.log( 'fire: s' )
-        } ),
-        new Hotkey( {
-          key: 'd',
-          fire: () => console.log( 'fire: d' )
-        } )
-      ]
-    } );
+    resetAllButton.addInputListener( new KeyboardListener( {
+      keys: [ 'x' ],
+      fire: () => console.log( 'fire: x from KeyboardListener' )
+    } ) );
+
+    KeyboardListener.createGlobalKeyboardListener( {
+      keys: [ 'x' ],
+      fire: () => console.log( 'fire: h from global KeyboardListener' )
+    }, resetAllButton );
+
+    window.setInterval( () => {
+      resetAllButton.inputEnabled = !resetAllButton.inputEnabled;
+    }, 2000 );
+
+    // resetAllButton.addInputListener( {
+    //   hotkeys: [
+    //     new Hotkey( {
+    //       key: 'x',
+    //       fire: () => console.log( 'fire: x' ),
+    //       enabledProperty: extraEnabledProperty
+    //     } ),
+    //     new Hotkey( {
+    //       key: 'x',
+    //       modifierKeys: [ 'b' ],
+    //       fire: () => console.log( 'fire: b+x' ),
+    //       enabledProperty: extraEnabledProperty
+    //     } ),
+    //     new Hotkey( {
+    //       key: 'w',
+    //       fire: () => console.log( 'fire: w' )
+    //     } ),
+    //     new Hotkey( {
+    //       key: 'a',
+    //       fire: () => console.log( 'fire: a' )
+    //     } ),
+    //     new Hotkey( {
+    //       key: 's',
+    //       fire: () => console.log( 'fire: s' )
+    //     } ),
+    //     new Hotkey( {
+    //       key: 'd',
+    //       fire: () => console.log( 'fire: d' )
+    //     } )
+    //   ]
+    // } );
   }
 }
 
Index: scenery/js/util/DisplayedProperty.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/scenery/js/util/DisplayedProperty.js b/scenery/js/util/DisplayedProperty.js
--- a/scenery/js/util/DisplayedProperty.js	(revision df9ab1245ac2fd0a5f2e1832bf54e1f5c750ba5e)
+++ b/scenery/js/util/DisplayedProperty.js	(date 1712012610255)
@@ -32,7 +32,8 @@
   constructor( node, options ) {
 
     options = merge( {
-      display: null // {Display|null} if null, this will check on any Display
+      display: null, // {Display|null} if null, this will check on any Display
+      watchPDOMDisplayed: false
     }, options );
 
     super( false, options );
@@ -43,12 +44,17 @@
     // @private {Display|null}
     this.display = options.display;
 
+    this.watchPDOMDisplayed = options.watchPDOMDisplayed;
+
     // @private {function}
     this.updateListener = this.updateValue.bind( this );
     this.changedInstanceListener = this.changedInstance.bind( this );
 
     node.changedInstanceEmitter.addListener( this.changedInstanceListener );
-    // node.pdomDisplaysEmitter.addListener( this.updateListener ); // TODO support pdom visibility, https://github.com/phetsims/scenery/issues/1167
+
+    if ( options.watchPDOMDisplayed ) {
+      node.pdomDisplaysEmitter.addListener( this.updateListener );
+    }
 
     // Add any instances the node may already have/
     const instances = node.instances;
@@ -62,10 +68,12 @@
    * @private
    */
   updateValue() {
-    this.value = this.node.wasVisuallyDisplayed( this.display );
-
-    // TODO support pdom visibility, https://github.com/phetsims/scenery/issues/1167
-    // this.value = this.node.wasVisuallyDisplayed( this.display ) || this.node.isPDOMDisplayed();
+    if ( this.watchPDOMDisplayed ) {
+      this.value = this.node.wasVisuallyDisplayed( this.display ) && this.node.isPDOMDisplayed();
+    }
+    else {
+      this.value = this.node.wasVisuallyDisplayed( this.display );
+    }
   }
 
   /**
@@ -101,7 +109,9 @@
     this.node.changedInstanceEmitter.removeListener( this.changedInstanceListener );
 
     // TODO support pdom visibility, https://github.com/phetsims/scenery/issues/1167
-    // this.node.pdomDisplaysEmitter.removeListener( this.updateListener );
+    if ( this.watchPDOMDisplayed ) {
+      this.node.pdomDisplaysEmitter.removeListener( this.updateListener );
+    }
 
     super.dispose();
   }

pixelzoom added a commit to phetsims/wilder that referenced this issue Apr 2, 2024
jonathanolson added a commit to phetsims/axon that referenced this issue Apr 2, 2024
…ng Node rootedDisplayChangedEmitter, pdomVisibleProperty, see phetsims/scenery#1621
jonathanolson added a commit that referenced this issue Apr 2, 2024
…ng Node rootedDisplayChangedEmitter, pdomVisibleProperty, see #1621
jonathanolson added a commit to phetsims/wilder that referenced this issue Apr 4, 2024
@jessegreenberg
Copy link
Contributor Author

The above commits added Hotkey and hotkeyManager. Additional improvements were added in #1570 so that KeyboardListener could be implemented with Hotkey. We also added a new DisplayedTrailsProperty to support this (continued in #1620).

Unit tests helped me find a problem with hotkeyRegistry. The available hotkeys are updated when focus changes and when Nodes along the trail have a change to inputEnabledProperty. But there are no updates when input listeners are added or removed. So the following is possible:

1) focus a Node
2) add an input listener with Hotkeys to the Node
3) press keys that should trigger the Hotkeys
4) Hotkeys do not fire because availableHotkeys was not updated

Here is a proposal to fix this but since it requres a change to Node I think it should be reviewed before committing.

Subject: [PATCH] Update usages of KeyboardListener and KeyboardDragListener after changes from https://github.com/phetsims/scenery/issues/1570
---
Index: js/nodes/Node.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/nodes/Node.ts b/js/nodes/Node.ts
--- a/js/nodes/Node.ts	(revision b2506ab1ab326c6df8d07557b733f1cb2172f5bc)
+++ b/js/nodes/Node.ts	(date 1714084395407)
@@ -170,7 +170,7 @@
 import BooleanIO from '../../../tandem/js/types/BooleanIO.js';
 import IOType from '../../../tandem/js/types/IOType.js';
 import TProperty from '../../../axon/js/TProperty.js';
-import { ACCESSIBILITY_OPTION_KEYS, CanvasContextWrapper, CanvasSelfDrawable, Display, DOMSelfDrawable, Drawable, Features, Filter, Image, ImageOptions, Instance, isHeightSizable, isWidthSizable, LayoutConstraint, Mouse, ParallelDOM, ParallelDOMOptions, Picker, Pointer, Renderer, RendererSummary, scenery, serializeConnectedNodes, SVGSelfDrawable, TInputListener, TLayoutOptions, Trail, WebGLSelfDrawable } from '../imports.js';
+import { ACCESSIBILITY_OPTION_KEYS, CanvasContextWrapper, CanvasSelfDrawable, Display, DOMSelfDrawable, Drawable, Features, Filter, hotkeyManager, Image, ImageOptions, Instance, isHeightSizable, isWidthSizable, LayoutConstraint, Mouse, ParallelDOM, ParallelDOMOptions, Picker, Pointer, Renderer, RendererSummary, scenery, serializeConnectedNodes, SVGSelfDrawable, TInputListener, TLayoutOptions, Trail, WebGLSelfDrawable } from '../imports.js';
 import optionize, { combineOptions, EmptySelfOptions, optionize3 } from '../../../phet-core/js/optionize.js';
 import IntentionalAny from '../../../phet-core/js/types/IntentionalAny.js';
 import Utils from '../../../dot/js/Utils.js';
@@ -2333,6 +2333,10 @@
       this._inputListeners.push( listener );
       this._picker.onAddInputListener();
       if ( assertSlow ) { this._picker.audit(); }
+
+      if ( listener.hotkeys ) {
+        hotkeyManager.updateHotkeysFromInputListenerChange( this );
+      }
     }
     return this;
   }
@@ -2349,6 +2353,10 @@
       this._inputListeners.splice( index, 1 );
       this._picker.onRemoveInputListener();
       if ( assertSlow ) { this._picker.audit(); }
+
+      if ( listener.hotkeys ) {
+        hotkeyManager.updateHotkeysFromInputListenerChange( this );
+      }
     }
 
     return this;
Index: js/input/hotkeyManager.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/input/hotkeyManager.ts b/js/input/hotkeyManager.ts
--- a/js/input/hotkeyManager.ts	(revision b2506ab1ab326c6df8d07557b733f1cb2172f5bc)
+++ b/js/input/hotkeyManager.ts	(date 1714084395404)
@@ -24,7 +24,7 @@
  * @author Jonathan Olson <jonathan.olson@colorado.edu>
  */
 
-import { EnglishKey, eventCodeToEnglishString, FocusManager, globalHotkeyRegistry, globalKeyStateTracker, Hotkey, KeyboardUtils, metaEnglishKeys, scenery } from '../imports.js';
+import { EnglishKey, eventCodeToEnglishString, FocusManager, globalHotkeyRegistry, globalKeyStateTracker, Hotkey, KeyboardUtils, metaEnglishKeys, Node, scenery } from '../imports.js';
 import DerivedProperty, { UnknownDerivedProperty } from '../../../axon/js/DerivedProperty.js';
 import TProperty from '../../../axon/js/TProperty.js';
 import TinyProperty from '../../../axon/js/TinyProperty.js';
@@ -214,6 +214,22 @@
     } );
   }
 
+  /**
+   * Scenery-internal. If a Node along the focused trail changes its input listeners we need to manually recompute
+   * the available hotkeys. There is no other way at this time to observe the input listeners of a Node. Otherwise,
+   * the available hotkeys will be recomputed when focus changes, inputEnabledProperty changes for Nodes along the
+   * trail, or when global hotkeys change.
+   *
+   * Called by Node.addInputListener/Node.removeInputListener.
+   *
+   * (scenery-internal)
+   */
+  public updateHotkeysFromInputListenerChange( node: Node ): void {
+    if ( FocusManager.pdomFocusProperty.value && FocusManager.pdomFocusProperty.value.trail.nodes.includes( node ) ) {
+      this.availableHotkeysProperty.recomputeDerivation();
+    }
+  }
+
   /**
    * Given a main `key`, see if there is a hotkey that should be considered "active/pressed" for it.
    *

@jessegreenberg
Copy link
Contributor Author

@jonathanolson reviewed #1621 (comment) with me and confirmed it is OK. I committed it.

hotkeyManager and globalHotkeyRegistry are looking great and have been working well for a while. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants