Skip to content

Commit

Permalink
Adding Hotkey.override option, see #1621
Browse files Browse the repository at this point in the history
  • Loading branch information
jonathanolson committed Apr 4, 2024
1 parent 7603e70 commit 8cb5024
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 15 deletions.
9 changes: 8 additions & 1 deletion js/input/Hotkey.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ type SelfOptions = {
// 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;

// If true, any overlapping hotkeys (either added to an ancestor's inputListener or later in the local/global order)
// will be ignored.
override?: boolean;
};

export type HotkeyOptions = SelfOptions & EnabledComponentOptions;
Expand All @@ -82,6 +86,7 @@ export default class Hotkey extends EnabledComponent {
public readonly fireOnHold: boolean;
public readonly fireOnHoldTiming: HotkeyFireOnHoldTiming;
public readonly allowOverlap: boolean;
public readonly override: boolean;

// All keys that are part of this hotkey (key + modifierKeys)
public readonly keys: EnglishKey[];
Expand Down Expand Up @@ -118,7 +123,8 @@ export default class Hotkey extends EnabledComponent {
fireOnHoldTiming: 'browser',
fireOnHoldCustomDelay: 400,
fireOnHoldCustomInterval: 100,
allowOverlap: false
allowOverlap: false,
override: false
}, providedOptions );

super( options );
Expand All @@ -132,6 +138,7 @@ export default class Hotkey extends EnabledComponent {
this.fireOnHold = options.fireOnHold;
this.fireOnHoldTiming = options.fireOnHoldTiming;
this.allowOverlap = options.allowOverlap;
this.override = options.override;

this.keys = _.uniq( [ this.key, ...this.modifierKeys ] );

Expand Down
57 changes: 43 additions & 14 deletions js/input/hotkeyManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,22 @@ import DerivedProperty, { UnknownDerivedProperty } from '../../../axon/js/Derive
import TProperty from '../../../axon/js/TProperty.js';
import TinyProperty from '../../../axon/js/TinyProperty.js';

const arrayComparator = <Key>( a: Key[], b: Key[] ): boolean => {
return a.length === b.length && a.every( ( element, index ) => element === b[ index ] );
};

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

class HotkeyManager {

// All hotkeys that are either globally or under the current focus trail
private readonly availableHotkeysProperty: UnknownDerivedProperty<Set<Hotkey>>;
// All hotkeys that are either globally or under the current focus trail. They are ordered, so that the first
// "identical key-shortcut" hotkey with override will be the one that is active.
private readonly availableHotkeysProperty: UnknownDerivedProperty<Hotkey[]>;

// Enabled hotkeys that are either global, or under the current focus trail
private readonly enabledHotkeysProperty: TProperty<Set<Hotkey>> = new TinyProperty( new Set<Hotkey>() );
private readonly enabledHotkeysProperty: TProperty<Hotkey[]> = new TinyProperty( [] );

// The set of EnglishKeys that are currently pressed.
private englishKeysDown: Set<EnglishKey> = new Set<EnglishKey>();
Expand All @@ -57,29 +62,31 @@ class HotkeyManager {
globalHotkeyRegistry.hotkeysProperty,
FocusManager.pdomFocusProperty
], ( globalHotkeys, focus ) => {
// Always include global hotkeys. Use a set since we might have duplicates.
const hotkeys = new Set<Hotkey>( globalHotkeys );
const hotkeys: Hotkey[] = [];

// If we have focus, include the hotkeys from the focus trail
if ( focus ) {
for ( const node of focus.trail.nodes ) {
for ( const node of focus.trail.nodes.slice().reverse() ) {
if ( !node.isInputEnabled() ) {
break;
}

node.inputListeners.forEach( listener => {
listener.hotkeys?.forEach( hotkey => {
hotkeys.add( hotkey );
hotkeys.push( hotkey );
} );
} );
}
}

return hotkeys;
// Always include global hotkeys. Use a set since we might have duplicates.
hotkeys.push( ...globalHotkeys );

return _.uniq( hotkeys );
}, {
// We want to not over-notify, so we compare the sets directly
valueComparisonStrategy: setComparator
} ) as UnknownDerivedProperty<Set<Hotkey>>;
valueComparisonStrategy: arrayComparator
} ) as UnknownDerivedProperty<Hotkey[]>;

// If any of the nodes in the focus trail change inputEnabled, we need to recompute availableHotkeysProperty
const onInputEnabledChanged = () => {
Expand All @@ -101,7 +108,29 @@ class HotkeyManager {

// Update enabledHotkeysProperty when availableHotkeysProperty (or any enabledProperty) changes
const rebuildHotkeys = () => {
this.enabledHotkeysProperty.value = new Set( [ ...this.availableHotkeysProperty.value ].filter( hotkey => hotkey.enabledProperty.value ) );
const overriddenHotkeyStrings = new Set<string>();
const enabledHotkeys: Hotkey[] = [];

for ( const hotkey of this.availableHotkeysProperty.value ) {
if ( hotkey.enabledProperty.value ) {
// Each hotkey will have a canonical way to represent it, so we can check for duplicates when overridden.
// Catch shift+ctrl+c and ctrl+shift+c as the same hotkey.
const hotkeyCanonicalString = [
...hotkey.modifierKeys.slice().sort(),
hotkey.key
].join( '+' );

if ( !overriddenHotkeyStrings.has( hotkeyCanonicalString ) ) {
enabledHotkeys.push( hotkey );

if ( hotkey.override ) {
overriddenHotkeyStrings.add( hotkeyCanonicalString );
}
}
}
}

this.enabledHotkeysProperty.value = enabledHotkeys;
};
// Because we can't add duplicate listeners, we create extra closures to have a unique handle for each hotkey
const hotkeyRebuildListenerMap = new Map<Hotkey, () => void>(); // eslint-disable-line no-spaced-func
Expand All @@ -112,7 +141,7 @@ class HotkeyManager {
// Any old hotkeys and aren't in new hotkeys should be unlinked
if ( oldHotkeys ) {
for ( const hotkey of oldHotkeys ) {
if ( !newHotkeys.has( hotkey ) ) {
if ( !newHotkeys.includes( hotkey ) ) {
const listener = hotkeyRebuildListenerMap.get( hotkey )!;
hotkeyRebuildListenerMap.delete( hotkey );
assert && assert( listener );
Expand All @@ -125,7 +154,7 @@ class HotkeyManager {

// Any new hotkeys that aren't in old hotkeys should be linked
for ( const hotkey of newHotkeys ) {
if ( !oldHotkeys || !oldHotkeys.has( hotkey ) ) {
if ( !oldHotkeys || !oldHotkeys.includes( hotkey ) ) {
// Unfortunate. Perhaps in the future we could have an abstraction that makes a "count" of how many times we
// are "listening" to a Property.
const listener = () => rebuildHotkeys();
Expand All @@ -152,7 +181,7 @@ class HotkeyManager {
// Remove any hotkeys that are no longer available or enabled
if ( oldHotkeys ) {
for ( const hotkey of oldHotkeys ) {
if ( !newHotkeys.has( hotkey ) && this.activeHotkeys.has( hotkey ) ) {
if ( !newHotkeys.includes( hotkey ) && this.activeHotkeys.has( hotkey ) ) {
this.removeActiveHotkey( hotkey, null, false );
}
}
Expand Down

0 comments on commit 8cb5024

Please sign in to comment.