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

Optional prevent default and asynchronous hold for keybinding execution #689

Merged
merged 7 commits into from
Mar 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
109 changes: 90 additions & 19 deletions packages/commands/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,7 @@ export class CommandRegistry {
}

// Get the normalized keystroke for the event.
let keystroke = CommandRegistry.keystrokeForKeydownEvent(event);
const keystroke = CommandRegistry.keystrokeForKeydownEvent(event);

// If the keystroke is not valid for the keyboard layout, replay
// any suppressed events and clear the pending state.
Expand All @@ -536,29 +536,37 @@ export class CommandRegistry {
this._keystrokes.push(keystroke);

// Find the exact and partial matches for the key sequence.
let { exact, partial } = Private.matchKeyBinding(
const { exact, partial } = Private.matchKeyBinding(
this._keyBindings,
this._keystrokes,
event
);
// Whether there is any partial match.
const hasPartial = partial.length !== 0;

// If there is no exact match and no partial match, replay
// any suppressed events and clear the pending state.
if (!exact && !partial) {
if (!exact && !hasPartial) {
this._replayKeydownEvents();
this._clearPendingState();
return;
}

// Stop propagation of the event. If there is only a partial match,
// the event will be replayed if a final exact match never occurs.
event.preventDefault();
event.stopPropagation();
if (exact?.preventDefault || partial.some(match => match.preventDefault)) {
event.preventDefault();
event.stopPropagation();
}

// Store the event for possible playback in the future and for
// the use in execution hold check.
this._keydownEvents.push(event);

// If there is an exact match but no partial match, the exact match
// can be dispatched immediately. The pending state is cleared so
// the next key press starts from the default state.
if (exact && !partial) {
if (exact && !hasPartial) {
this._executeKeyBinding(exact);
this._clearPendingState();
return;
Expand All @@ -571,14 +579,26 @@ export class CommandRegistry {
this._exactKeyMatch = exact;
}

// Store the event for possible playback in the future.
this._keydownEvents.push(event);

// (Re)start the timer to dispatch the most recent exact match
// in case the partial match fails to result in an exact match.
this._startTimer();
}

/**
* Delay the execution of any command matched against the given 'keydown' event
* until the `permission` to execute is granted.
*
* @param event - The event object for a `'keydown'` event.
* @param permission - The promise with value indicating whether to proceed with the execution.
*
* ### Note
* This enables the caller of `processKeydownEvent` to asynchronously prevent the
* execution of the command based on external events.
*/
holdKeyBindingExecution(event: KeyboardEvent, permission: Promise<boolean>) {
this._holdKeyBindingPromises.set(event, permission);
}
fcollonval marked this conversation as resolved.
Show resolved Hide resolved

/**
* Start or restart the pending timeout.
*/
Expand Down Expand Up @@ -615,8 +635,38 @@ export class CommandRegistry {
* Execute the command for the given key binding.
*
* If the command is missing or disabled, a warning will be logged.
*/
private _executeKeyBinding(binding: CommandRegistry.IKeyBinding): void {
*
* The execution will not proceed if any of the events leading to
* the keybinding matching were held with the permission resolving to false.
*/
private async _executeKeyBinding(
binding: CommandRegistry.IKeyBinding
): Promise<void> {
if (this._holdKeyBindingPromises.size !== 0) {
// Copy keydown events list to ensure it is available in async code.
const keydownEvents = [...this._keydownEvents];
// Wait until all hold requests on execution are lifted.
const executionAllowed = (
await Promise.race([
Promise.all(
keydownEvents.map(
async event =>
this._holdKeyBindingPromises.get(event) ?? Promise.resolve(true)
)
),
new Promise<boolean[]>(resolve => {
setTimeout(() => resolve([false]), Private.KEYBINDING_HOLD_TIMEOUT);
})
])
).every(Boolean);
// Clear the hold requests.
this._holdKeyBindingPromises.clear();
// Do not proceed with the execution if any of the hold requests did not get the permission to proceed.
if (!executionAllowed) {
return;
}
}

let { command, args } = binding;
let newArgs: ReadonlyPartialJSONObject = {
_luminoEvent: { type: 'keybinding', keys: binding.keys },
Expand All @@ -630,7 +680,7 @@ export class CommandRegistry {
console.warn(`${msg1} ${msg2}`);
return;
}
this.execute(command, newArgs);
await this.execute(command, newArgs);
}

/**
Expand Down Expand Up @@ -675,6 +725,7 @@ export class CommandRegistry {
this,
CommandRegistry.IKeyBindingChangedArgs
>(this);
private _holdKeyBindingPromises = new Map<KeyboardEvent, Promise<boolean>>();
}

/**
Expand Down Expand Up @@ -1027,6 +1078,13 @@ export namespace CommandRegistry {
* If provided, this will override `keys` on Linux platforms.
*/
linuxKeys?: string[];

/**
* Whether to prevent default action of the keyboard events during sequence matching.
*
* The default value is `true`.
*/
preventDefault?: boolean;
}

/**
Expand Down Expand Up @@ -1055,6 +1113,13 @@ export namespace CommandRegistry {
* The arguments for the command.
*/
readonly args: ReadonlyPartialJSONObject;

/**
* Whether to prevent default action of the keyboard events during sequence matching.
*
* The default value is `true`.
*/
readonly preventDefault?: boolean;
}

/**
Expand Down Expand Up @@ -1293,6 +1358,11 @@ namespace Private {
*/
export const CHORD_TIMEOUT = 1000;

/**
* The timeout in ms for stopping the hold on keybinding execution.
*/
export const KEYBINDING_HOLD_TIMEOUT = 1000;

/**
* A convenience type alias for a command func.
*/
Expand Down Expand Up @@ -1372,7 +1442,8 @@ namespace Private {
keys: CommandRegistry.normalizeKeys(options),
selector: validateSelector(options),
command: options.command,
args: options.args || JSONExt.emptyObject
args: options.args || JSONExt.emptyObject,
preventDefault: options.preventDefault ?? true
};
}

Expand All @@ -1386,9 +1457,9 @@ namespace Private {
exact: CommandRegistry.IKeyBinding | null;

/**
* Whether there are bindings which partially match the sequence.
* The key bindings which partially match the sequence.
*/
partial: boolean;
partial: CommandRegistry.IKeyBinding[];
}

/**
Expand All @@ -1405,8 +1476,8 @@ namespace Private {
// The current best exact match.
let exact: CommandRegistry.IKeyBinding | null = null;

// Whether a partial match has been found.
let partial = false;
// Partial matches.
let partial = [];

// The match distance for the exact match.
let distance = Infinity;
Expand All @@ -1430,8 +1501,8 @@ namespace Private {
// If it is a partial match and no other partial match has been
// found, ensure the selector matches and set the partial flag.
if (sqm === SequenceMatch.Partial) {
if (!partial && targetDistance(binding.selector, event) !== -1) {
partial = true;
if (targetDistance(binding.selector, event) !== -1) {
partial.push(binding);
}
continue;
}
Expand Down
143 changes: 143 additions & 0 deletions packages/commands/tests/src/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -821,6 +821,70 @@ describe('@lumino/commands', () => {
expect(called).to.equal(false);
});

it('should prevent default on dispatch', () => {
registry.addCommand('test', {
execute: () => void 0
});
registry.addKeyBinding({
keys: ['Ctrl ;'],
selector: `#${elem.id}`,
command: 'test'
});
const event = new KeyboardEvent('keydown', {
keyCode: 59,
ctrlKey: true
});
let defaultPrevented = false;
event.preventDefault = () => {
defaultPrevented = true;
};
elem.dispatchEvent(event);
expect(defaultPrevented).to.equal(true);
});

it('should not prevent default when sequence does not match', () => {
registry.addCommand('test', {
execute: () => void 0
});
registry.addKeyBinding({
keys: ['Ctrl ;'],
selector: `#${elem.id}`,
command: 'test'
});
const event = new KeyboardEvent('keydown', {
keyCode: 59,
ctrlKey: false
});
let defaultPrevented = false;
event.preventDefault = () => {
defaultPrevented = true;
};
elem.dispatchEvent(event);
expect(defaultPrevented).to.equal(false);
});

it('should not prevent default if keybinding opts out', () => {
registry.addCommand('test', {
execute: () => void 0
});
registry.addKeyBinding({
keys: ['Ctrl ;'],
selector: `#${elem.id}`,
command: 'test',
preventDefault: false
});
const event = new KeyboardEvent('keydown', {
keyCode: 59,
ctrlKey: true
});
let defaultPrevented = false;
event.preventDefault = () => {
defaultPrevented = true;
};
elem.dispatchEvent(event);
expect(defaultPrevented).to.equal(false);
});

it('should dispatch with multiple chords in a key sequence', () => {
let count = 0;
registry.addCommand('test', {
Expand Down Expand Up @@ -1258,6 +1322,85 @@ describe('@lumino/commands', () => {
});
});

describe('.holdKeyBindingExecution()', () => {
let calledPromise: Promise<boolean>;
let execute: () => void;

beforeEach(() => {
calledPromise = Promise.race([
new Promise<boolean>(_resolve => {
execute = () => _resolve(true);
}),
new Promise<boolean>(resolve =>
setTimeout(() => resolve(false), 1000)
)
]);
});

it('should proceed with command execution if permission of the event resolves to true', async () => {
registry.addCommand('test', {
execute
});
registry.addKeyBinding({
keys: ['Ctrl ;'],
selector: `#${elem.id}`,
command: 'test'
});
const event = new KeyboardEvent('keydown', {
keyCode: 59,
ctrlKey: true
});
registry.holdKeyBindingExecution(event, Promise.resolve(true));
elem.dispatchEvent(event);
const called = await calledPromise;
expect(called).to.equal(true);
});

it('should prevent command execution if permission of the event resolves to false', async () => {
registry.addCommand('test', {
execute
});
registry.addKeyBinding({
keys: ['Ctrl ;'],
selector: `#${elem.id}`,
command: 'test'
});
const event = new KeyboardEvent('keydown', {
keyCode: 59,
ctrlKey: true
});
registry.holdKeyBindingExecution(event, Promise.resolve(false));
elem.dispatchEvent(event);
const called = await calledPromise;
expect(called).to.equal(false);
});

it('should prevent command execution if permission for any of the events resolves to false', async () => {
registry.addCommand('test', {
execute
});
registry.addKeyBinding({
keys: ['Shift ['],
selector: `#${elem.id}`,
command: 'test'
});
const shiftEvent = new KeyboardEvent('keydown', {
keyCode: 16,
shiftKey: true
});
const bracketEvent = new KeyboardEvent('keydown', {
keyCode: 219,
shiftKey: true
});
registry.holdKeyBindingExecution(shiftEvent, Promise.resolve(true));
registry.holdKeyBindingExecution(bracketEvent, Promise.resolve(false));
elem.dispatchEvent(shiftEvent);
elem.dispatchEvent(bracketEvent);
const called = await calledPromise;
expect(called).to.equal(false);
});
});

describe('.parseKeystroke()', () => {
it('should parse a keystroke into its parts', () => {
let parts = CommandRegistry.parseKeystroke('Ctrl Shift Alt S');
Expand Down
Loading
Loading