Skip to content

Commit

Permalink
Adding blur => clearState() and keyDownStateChangedEmitter for KeySta…
Browse files Browse the repository at this point in the history 
…teTracker, and corresponding fixes/updates in hotkeyManager, see #1621
  • Loading branch information
@jonathanolson
jonathanolson committed Mar 28, 2024
1 parent 5c9799e commit 122d1f4
Show file tree
Hide file tree
Showing 2 changed files with 99 additions and 83 deletions.
92 changes: 71 additions & 21 deletions js/accessibility/KeyStateTracker.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import PhetioAction from '../../../tandem/js/PhetioAction.js';
import Emitter from '../../../axon/js/Emitter.js';
import stepTimer from '../../../axon/js/stepTimer.js';
import EventType from '../../../tandem/js/EventType.js';
import { EnglishKey, EnglishStringToCodeMap, EventIO, KeyboardUtils, scenery } from '../imports.js';
import { EnglishKey, EnglishStringToCodeMap, eventCodeToEnglishString, EventIO, KeyboardUtils, scenery } from '../imports.js';
import { PhetioObjectOptions } from '../../../tandem/js/PhetioObject.js';
import PickOptional from '../../../phet-core/js/types/PickOptional.js';
import TEmitter from '../../../axon/js/TEmitter.js';
Expand All @@ -26,9 +26,6 @@ type KeyStateInfo = {
// The event.code string for the key.
key: string;

// Is the key currently down?
keyDown: boolean;

// How long has the key been held down, in milliseconds
timeDown: number;
};
Expand All @@ -53,6 +50,7 @@ class KeyStateTracker {
// Listeners potentially attached to the document to update the state of this KeyStateTracker, see attachToWindow()
private documentKeyupListener: null | ( ( event: KeyboardEvent ) => void ) = null;
private documentKeydownListener: null | ( ( event: KeyboardEvent ) => void ) = null;
private documentBlurListener: null | ( ( event: FocusEvent ) => void ) = null;

// If the KeyStateTracker is enabled. If disabled, keyState is cleared and listeners noop.
private _enabled = true;
Expand All @@ -62,6 +60,10 @@ class KeyStateTracker {
public readonly keydownEmitter: TEmitter<[ KeyboardEvent ]> = new Emitter( { parameters: [ { valueType: KeyboardEvent } ] } );
public readonly keyupEmitter: TEmitter<[ KeyboardEvent ]> = new Emitter( { parameters: [ { valueType: KeyboardEvent } ] } );

// Emits when any key "down" state changes. This is useful for when you want to know if any key is down or up.
// Does NOT change for timeDown changes. DOES fire if the browser sends fire-on-hold down.
public readonly keyDownStateChangedEmitter: TEmitter<[ KeyboardEvent | null ]> = new Emitter( { parameters: [ { valueType: [ KeyboardEvent, null ] } ] } );

// Action which updates the KeyStateTracker, when it is time to do so - the update is wrapped by an Action so that
// the KeyStateTracker state is captured for PhET-iO.
public readonly keydownUpdateAction: PhetioAction<[ KeyboardEvent ]>;
Expand Down Expand Up @@ -100,7 +102,6 @@ class KeyStateTracker {
const key = KeyboardUtils.getEventCode( domEvent )!;
assert && assert( key, 'Could not find key from domEvent' );
this.keyState[ key ] = {
keyDown: true,
key: key,
timeDown: 0 // in ms
};
Expand All @@ -110,6 +111,7 @@ class KeyStateTracker {

// keydown update received, notify listeners
this.keydownEmitter.emit( domEvent );
this.keyDownStateChangedEmitter.emit( domEvent );
}

}, {
Expand Down Expand Up @@ -141,11 +143,13 @@ class KeyStateTracker {
// BOTH keys are released, so this should be safe in that case.
// See https://github.com/phetsims/scenery/issues/1555
if ( platform.mac && KeyboardUtils.isMetaKey( domEvent ) ) {
this.clearState();
// Skip notification, since we will emit on the state change below
this.clearState( true );
}

// keyup event received, notify listeners
this.keyupEmitter.emit( domEvent );
this.keyDownStateChangedEmitter.emit( domEvent );
}
}, {
phetioPlayback: true,
Expand Down Expand Up @@ -187,42 +191,51 @@ class KeyStateTracker {
const key = KeyboardUtils.getEventCode( domEvent )!;
assert && assert( key, 'key not found from domEvent' );

let changed = false;

// add modifier keys if they aren't down
if ( domEvent.shiftKey && !KeyboardUtils.isShiftKey( domEvent ) && !this.shiftKeyDown ) {
changed = changed || !this.keyState[ KeyboardUtils.KEY_SHIFT_LEFT ];
this.keyState[ KeyboardUtils.KEY_SHIFT_LEFT ] = {
keyDown: true,
key: key,
timeDown: 0 // in ms
};
}
if ( domEvent.altKey && !KeyboardUtils.isAltKey( domEvent ) && !this.altKeyDown ) {
changed = changed || !this.keyState[ KeyboardUtils.KEY_ALT_LEFT ];
this.keyState[ KeyboardUtils.KEY_ALT_LEFT ] = {
keyDown: true,
key: key,
timeDown: 0 // in ms
};
}
if ( domEvent.ctrlKey && !KeyboardUtils.isControlKey( domEvent ) && !this.ctrlKeyDown ) {
changed = changed || !this.keyState[ KeyboardUtils.KEY_CONTROL_LEFT ];
this.keyState[ KeyboardUtils.KEY_CONTROL_LEFT ] = {
keyDown: true,
key: key,
timeDown: 0 // in ms
};
}

// delete modifier keys if we think they are down
if ( !domEvent.shiftKey && this.shiftKeyDown ) {
changed = changed || !!this.keyState[ KeyboardUtils.KEY_SHIFT_LEFT ] || !!this.keyState[ KeyboardUtils.KEY_SHIFT_RIGHT ];
delete this.keyState[ KeyboardUtils.KEY_SHIFT_LEFT ];
delete this.keyState[ KeyboardUtils.KEY_SHIFT_RIGHT ];
}
if ( !domEvent.altKey && this.altKeyDown ) {
changed = changed || !!this.keyState[ KeyboardUtils.KEY_ALT_LEFT ] || !!this.keyState[ KeyboardUtils.KEY_ALT_RIGHT ];
delete this.keyState[ KeyboardUtils.KEY_ALT_LEFT ];
delete this.keyState[ KeyboardUtils.KEY_ALT_RIGHT ];
}
if ( !domEvent.ctrlKey && this.ctrlKeyDown ) {
changed = changed || !!this.keyState[ KeyboardUtils.KEY_CONTROL_LEFT ] || !!this.keyState[ KeyboardUtils.KEY_CONTROL_RIGHT ];
delete this.keyState[ KeyboardUtils.KEY_CONTROL_LEFT ];
delete this.keyState[ KeyboardUtils.KEY_CONTROL_RIGHT ];
}

if ( changed ) {
this.keyDownStateChangedEmitter.emit( domEvent );
}
}

/**
Expand Down Expand Up @@ -255,13 +268,7 @@ class KeyStateTracker {
* Returns true if a key with the KeyboardEvent.code is currently down.
*/
public isKeyDown( key: string ): boolean {
if ( !this.keyState[ key ] ) {

// key hasn't been pressed once yet
return false;
}

return this.keyState[ key ].keyDown;
return !!this.keyState[ key ];
}

/**
Expand All @@ -271,6 +278,33 @@ class KeyStateTracker {
return this.isAnyKeyInListDown( EnglishStringToCodeMap[ key ] );
}

/**
* Returns the set of keys that are currently down.
*
* NOTE: Always returns a new array, so a defensive copy is not needed.
*/
public getKeysDown(): string[] {
return Object.keys( this.keyState );
}

/**
* Returns the set of EnglishKeys that are currently down.
*
* NOTE: Always returns a new Set, so a defensive copy is not needed.
*/
public getEnglishKeysDown(): Set<EnglishKey> {
const englishKeySet = new Set<EnglishKey>();

for ( const key of this.getKeysDown() ) {
const englishKey = eventCodeToEnglishString( key );
if ( englishKey ) {
englishKeySet.add( englishKey );
}
}

return englishKeySet;
}

/**
* Returns true if any of the keys in the list are currently down. Keys are the KeyboardEvent.code strings.
*/
Expand Down Expand Up @@ -410,8 +444,12 @@ class KeyStateTracker {
/**
* Clear the entire state of the key tracker, basically restarting the tracker.
*/
public clearState(): void {
public clearState( skipNotify?: boolean ): void {
this.keyState = {};

if ( !skipNotify ) {
this.keyDownStateChangedEmitter.emit( null );
}
}

/**
Expand All @@ -429,10 +467,8 @@ class KeyStateTracker {

// for each key that is still down, increment the tracked time that has been down
for ( const i in this.keyState ) {
if ( this.keyState.hasOwnProperty( i ) ) {
if ( this.keyState[ i ].keyDown ) {
this.keyState[ i ].timeDown += ms;
}
if ( this.keyState[ i ] ) {
this.keyState[ i ].timeDown += ms;
}
}
}
Expand All @@ -453,11 +489,22 @@ class KeyStateTracker {
this.keyupUpdate( event );
};

this.documentBlurListener = event => {

// As recommended for similar situations online, we clear our key state when we get a window blur, since we
// will not be able to track any key state changes during this time (and users will likely release any keys
// that are pressed).
// If shift/alt/ctrl are pressed when we regain focus, we will hopefully get a keyboard event and update their state
// with correctModifierKeys().
this.clearState();
};

const addListenersToDocument = () => {

// attach with useCapture so that the keyStateTracker is updated before the events dispatch within Scenery
window.addEventListener( 'keyup', this.documentKeyupListener!, { capture: true } );
window.addEventListener( 'keydown', this.documentKeydownListener!, { capture: true } );
window.addEventListener( 'blur', this.documentBlurListener!, { capture: true } );
this.attachedToDocument = true;
};

Expand Down Expand Up @@ -502,12 +549,15 @@ class KeyStateTracker {
assert && assert( this.attachedToDocument, 'KeyStateTracker is not attached to window.' );
assert && assert( this.documentKeyupListener, 'keyup listener was not created or attached to window' );
assert && assert( this.documentKeydownListener, 'keydown listener was not created or attached to window.' );
assert && assert( this.documentBlurListener, 'blur listener was not created or attached to window.' );

window.removeEventListener( 'keyup', this.documentKeyupListener! );
window.removeEventListener( 'keydown', this.documentKeydownListener! );
window.removeEventListener( 'blur', this.documentBlurListener! );

this.documentKeyupListener = null;
this.documentKeydownListener = null;
this.documentBlurListener = null;

this.attachedToDocument = false;
}
Expand Down
90 changes: 28 additions & 62 deletions js/input/hotkeyManager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@
* nodes in the trail)
*
* Manages key press state using EnglishKey from globalKeyStateTracker.
* TODO: We need to gracefully handle when the user presses BOTH keys that correspond to an EnglishKey, e.g. https://github.com/phetsims/scenery/issues/1621
* TODO: left-ctrl and right-ctrl, then releases one (it should still count ctrl as pressed).
*
* The "available" hotkeys are the union of the above two sources.
*
Expand All @@ -35,7 +33,7 @@ import DerivedProperty from '../../../axon/js/DerivedProperty.js';
import TProperty from '../../../axon/js/TProperty.js';
import TinyProperty from '../../../axon/js/TinyProperty.js';

const hotkeySetComparator = ( a: Set<Hotkey>, b: Set<Hotkey> ) => {
const setComparator = <Key>( a: Set<Key>, b: Set<Key> ) => {
return a.size === b.size && [ ...a ].every( element => b.has( element ) );
};

Expand All @@ -47,6 +45,7 @@ class HotkeyManager {
// Enabled hotkeys that are either global, or under the current focus trail
private readonly enabledHotkeysProperty: TProperty<Set<Hotkey>> = new TinyProperty( new Set<Hotkey>() );

// TODO: We don't need this as a Property, see https://github.com/phetsims/scenery/issues/1621
private readonly englishKeysDownProperty: TProperty<Set<EnglishKey>> = new TinyProperty( new Set<EnglishKey>() );

// The current set of modifier keys (pressed or not) based on current enabled hotkeys
Expand Down Expand Up @@ -84,7 +83,7 @@ class HotkeyManager {
return hotkeys;
}, {
// We want to not over-notify, so we compare the sets directly
valueComparisonStrategy: hotkeySetComparator
valueComparisonStrategy: setComparator
} );

// Update enabledHotkeysProperty when availableHotkeysProperty (or any enabledProperty) changes
Expand Down Expand Up @@ -147,29 +146,28 @@ class HotkeyManager {
}

// Re-check all hotkeys (since modifier keys might have changed, OR we need to validate that there are no conflicts).
this.updateHotkeyStatus();
this.updateHotkeyStatus( null );
} );

// Track keydowns
globalKeyStateTracker.keydownEmitter.addListener( keyboardEvent => {
const keyCode = KeyboardUtils.getEventCode( keyboardEvent );
// Track key state changes
globalKeyStateTracker.keyDownStateChangedEmitter.addListener( ( keyboardEvent: KeyboardEvent | null ) => {
const oldEnglishKeysDown = this.englishKeysDownProperty.value;
const newEnglishKeysDown = globalKeyStateTracker.getEnglishKeysDown();
const englishKeysChanged = !setComparator( oldEnglishKeysDown, newEnglishKeysDown );

if ( keyCode !== null ) {
const englishKey = eventCodeToEnglishString( keyCode );
if ( englishKey ) {
this.onKeyDown( englishKey, keyboardEvent );
}
}
} );
if ( englishKeysChanged ) {
this.englishKeysDownProperty.value = newEnglishKeysDown;

// Track keyups
globalKeyStateTracker.keyupEmitter.addListener( keyboardEvent => {
const keyCode = KeyboardUtils.getEventCode( keyboardEvent );
this.updateHotkeyStatus( keyboardEvent );
}
else {
// No keys changed, got the browser/OS "fire on hold". See what hotkeys have the browser fire-on-hold behavior.

if ( keyCode !== null ) {
const englishKey = eventCodeToEnglishString( keyCode );
if ( englishKey ) {
this.onKeyUp( englishKey, keyboardEvent );
// 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 );
}
}
}
} );
Expand Down Expand Up @@ -218,16 +216,21 @@ class HotkeyManager {
* Re-check all hotkey active/pressed states (since modifier keys might have changed, OR we need to validate that
* there are no conflicts).
*/
private updateHotkeyStatus(): void {
private updateHotkeyStatus( keyboardEvent: KeyboardEvent | null ): void {
// For fireOnDown on/off cases, we only want to fire the hotkeys when we have a keyboard event specifying hotkey's
// main `key`.
const pressedOrReleasedKeyCode = KeyboardUtils.getEventCode( keyboardEvent );
const pressedOrReleasedEnglishKey = pressedOrReleasedKeyCode ? eventCodeToEnglishString( pressedOrReleasedKeyCode ) : null;

for ( const hotkey of this.enabledHotkeysProperty.value ) {
const shouldBeActive = this.getHotkeysForMainKey( hotkey.key ).includes( hotkey );
const isActive = this.activeHotkeys.has( hotkey );

if ( shouldBeActive && !isActive ) {
this.addActiveHotkey( hotkey, null, false );
this.addActiveHotkey( hotkey, null, hotkey.key === pressedOrReleasedEnglishKey );
}
else if ( !shouldBeActive && isActive ) {
this.removeActiveHotkey( hotkey, null, false );
this.removeActiveHotkey( hotkey, null, hotkey.key === pressedOrReleasedEnglishKey );
}
}
}
Expand Down Expand Up @@ -258,43 +261,6 @@ class HotkeyManager {
hotkey.isPressedProperty.value = false;
this.activeHotkeys.delete( hotkey );
}

private onKeyDown( englishKey: EnglishKey, keyboardEvent: KeyboardEvent ): void {
if ( this.englishKeysDownProperty.value.has( englishKey ) ) {
// Still pressed, got the browser/OS "fire on hold". See what hotkeys have the browser fire-on-hold behavior.

// 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 );
}
}
}
else {
// Freshly pressed, was not pressed before. See if there is a hotkey to fire.

this.englishKeysDownProperty.value = new Set( [ ...this.englishKeysDownProperty.value, englishKey ] );

const hotkeys = this.getHotkeysForMainKey( englishKey );
for ( const hotkey of hotkeys ) {
this.addActiveHotkey( hotkey, keyboardEvent, true );
}
}

this.updateHotkeyStatus();
}

private onKeyUp( englishKey: EnglishKey, keyboardEvent: KeyboardEvent ): void {

const hotkeys = this.getHotkeysForMainKey( englishKey );
for ( const hotkey of hotkeys ) {
this.removeActiveHotkey( hotkey, keyboardEvent, true );
}

this.englishKeysDownProperty.value = new Set( [ ...this.englishKeysDownProperty.value ].filter( key => key !== englishKey ) );

this.updateHotkeyStatus();
}
}
scenery.register( 'HotkeyManager', HotkeyManager );

Expand Down

0 comments on commit 122d1f4

Please sign in to comment.