Skip to content

Commit

Permalink
update hotkeys when list of input listeners changes, see #1621
Browse files Browse the repository at this point in the history
  • Loading branch information
jessegreenberg committed May 8, 2024
1 parent 92e8bc6 commit b3e6c15
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 8 deletions.
18 changes: 17 additions & 1 deletion js/input/hotkeyManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
* @author Jonathan Olson <[email protected]>
*/

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';
Expand Down Expand Up @@ -213,6 +213,22 @@ class HotkeyManager {
} );
}

/**
* 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.
*
Expand Down
6 changes: 0 additions & 6 deletions js/listeners/KeyboardListenerTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,6 @@ QUnit.test( 'KeyboardListener Tests', assert => {

a.addInputListener( listenerWithOverlappingKeys );

// TODO: https://github.com/phetsims/scenery/issues/1570, This is a workaround for a problem with hotkeyManager
// where it doesn't update the availableHotkeys when new input listeners are added. We can manually update by
// triggering focus changes.
a.blur();
a.focus();

triggerKeydownEvent( domElementA, KeyboardUtils.KEY_P, true );
assert.ok( !pFired, 'p should not fire because control key is down' );
assert.ok( ctrlPFired, 'ctrl P should have fired' );
Expand Down
14 changes: 13 additions & 1 deletion js/nodes/Node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ import Tandem from '../../../tandem/js/Tandem.js';
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';
Expand Down Expand Up @@ -2333,6 +2333,12 @@ class Node extends ParallelDOM {
this._inputListeners.push( listener );
this._picker.onAddInputListener();
if ( assertSlow ) { this._picker.audit(); }

// If the listener contains hotkeys, active hotkeys may need to be updated. There is no event
// for changing input listeners. See hotkeyManager for more information.
if ( listener.hotkeys ) {
hotkeyManager.updateHotkeysFromInputListenerChange( this );
}
}
return this;
}
Expand All @@ -2349,6 +2355,12 @@ class Node extends ParallelDOM {
this._inputListeners.splice( index, 1 );
this._picker.onRemoveInputListener();
if ( assertSlow ) { this._picker.audit(); }

// If the listener contains hotkeys, active hotkeys may need to be updated. There is no event
// for changing input listeners. See hotkeyManager for more information.
if ( listener.hotkeys ) {
hotkeyManager.updateHotkeysFromInputListenerChange( this );
}
}

return this;
Expand Down

0 comments on commit b3e6c15

Please sign in to comment.