twosigma / twosigma/beakerx_widgets

Overriding the command registry keydown handler causes shortcut execution errors.

Open
#55 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
3
Forks
9
PR merge metrics
No merged PRs in 30d

Description

There is a spot where we monkey-patch the Lumino `CommandRegistry`'s `processKeydownEvent` method and it is occasionally brittle, causing command invocations to fail. This seems like an anti-pattern for two reasons:

1. We are overriding the entire application's command registry and can cause other extensions to fail.
2. We are relying on an object we've attached to the `window` object instead of a local variable.

```typescript
app.commands.processKeydownEvent = (event) => {
if (window.beakerx.tableFocused) {
return false;
}

return originalProcessFn.call(app.commands, event);
};
```
https://github.com/twosigma/beakerx_widgets/blob/master/beakerx_widgets/js/src/lab/BeakerxWidgetExtension.ts#L67-L69

This problem is sometimes triggered when a user changes the kernel on an active notebook multiple times. One user reported that after switching the kernel a second or third time, the `window.beakerx` object is `undefined` so the handler fails while trying to access the `tableFocused` attribute.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.