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

[TS migration] Migrate 'KeyboardShortcut' lib to TypeScript #29338

Merged
Merged
Show file tree
Hide file tree
Changes from 4 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
9 changes: 0 additions & 9 deletions src/libs/KeyboardShortcut/KeyDownPressListener/index.js

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import {AddKeyDownPressListener, RemoveKeyDownPressListener} from './types';

const addKeyDownPressListener: AddKeyDownPressListener = () => {};
const removeKeyDownPressListener: RemoveKeyDownPressListener = () => {};

export {addKeyDownPressListener, removeKeyDownPressListener};
11 changes: 11 additions & 0 deletions src/libs/KeyboardShortcut/KeyDownPressListener/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import type {AddKeyDownPressListener, RemoveKeyDownPressListener} from './types';

const addKeyDownPressListener: AddKeyDownPressListener = (callbackFunction) => {
document.addEventListener('keydown', callbackFunction);
};

const removeKeyDownPressListener: RemoveKeyDownPressListener = (callbackFunction) => {
document.removeEventListener('keydown', callbackFunction);
};

export {addKeyDownPressListener, removeKeyDownPressListener};
6 changes: 6 additions & 0 deletions src/libs/KeyboardShortcut/KeyDownPressListener/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
type KeyDownPressCallback = (event: KeyboardEvent) => void;

type AddKeyDownPressListener = (callbackFunction: KeyDownPressCallback) => void;
type RemoveKeyDownPressListener = (callbackFunction: KeyDownPressCallback) => void;

export type {AddKeyDownPressListener, RemoveKeyDownPressListener};

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import getKeyEventModifiers from '../getKeyEventModifiers';
import BindHandlerToKeydownEvent from './types';

/**
* Checks if an event for that key is configured and if so, runs it.
*/
const bindHandlerToKeydownEvent: BindHandlerToKeydownEvent = (getDisplayName, eventHandlers, keyCommandEvent, event) => {
const eventModifiers = getKeyEventModifiers(keyCommandEvent);
const displayName = getDisplayName(keyCommandEvent.input, eventModifiers);

// Loop over all the callbacks
Object.values(eventHandlers[displayName]).every((callback) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regression from this PR. Sometimes I get crash:

crash

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any specific flow/screen you're seeing this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happens on all platforms

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's simple repro step:

Screen.Recording.2023-10-30.at.9.12.47.PM.mov

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@blazejkustra can you please raise quick PR? This is now deployed to staging

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On it 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@blazejkustra this is already fixed i think #30602

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I just noticed after merging the newest main, thanks!

// Determine if the event should bubble before executing the callback (which may have side-effects)
let shouldBubble: boolean | (() => void) | void = callback.shouldBubble || false;
if (typeof callback.shouldBubble === 'function') {
shouldBubble = callback.shouldBubble();
}

if (typeof callback.callback === 'function') {
callback.callback(event);
}

// If the event should not bubble, short-circuit the loop
return shouldBubble;
});
};

export default bindHandlerToKeydownEvent;
Original file line number Diff line number Diff line change
@@ -1,51 +1,51 @@
import _ from 'underscore';
import getKeyEventModifiers from '../getKeyEventModifiers';
import isEnterWhileComposition from '../isEnterWhileComposition';
import BindHandlerToKeydownEvent from './types';

/**
* Checks if an event for that key is configured and if so, runs it.
* @param {Function} getDisplayName
* @param {Object} eventHandlers
* @param {Object} keycommandEvent
* @param {Event} event
* @private
*/
function bindHandlerToKeydownEvent(getDisplayName, eventHandlers, keycommandEvent, event) {
const bindHandlerToKeydownEvent: BindHandlerToKeydownEvent = (getDisplayName, eventHandlers, keyCommandEvent, event) => {
if (!(event instanceof KeyboardEvent) || isEnterWhileComposition(event)) {
return;
}

const eventModifiers = getKeyEventModifiers(keycommandEvent);
const displayName = getDisplayName(keycommandEvent.input, eventModifiers);
const eventModifiers = getKeyEventModifiers(keyCommandEvent);
const displayName = getDisplayName(keyCommandEvent.input, eventModifiers);

// Loop over all the callbacks
_.every(eventHandlers[displayName], (callback) => {
Object.values(eventHandlers[displayName]).every((callback) => {
const textArea = event.target as HTMLTextAreaElement;
const contentEditable = textArea?.contentEditable;
const nodeName = textArea?.nodeName;

// Early return for excludedNodes
if (_.contains(callback.excludedNodes, event.target.nodeName)) {
if (callback.excludedNodes.includes(nodeName)) {
return true;
}

// If configured to do so, prevent input text control to trigger this event
if (!callback.captureOnInputs && (event.target.nodeName === 'INPUT' || event.target.nodeName === 'TEXTAREA' || event.target.contentEditable === 'true')) {
if (!callback.captureOnInputs && (nodeName === 'INPUT' || nodeName === 'TEXTAREA' || contentEditable === 'true')) {
return true;
}

// Determine if the event should bubble before executing the callback (which may have side-effects)
let shouldBubble = callback.shouldBubble || false;
if (_.isFunction(callback.shouldBubble)) {
let shouldBubble: boolean | (() => void) | void = callback.shouldBubble || false;
if (typeof callback.shouldBubble === 'function') {
shouldBubble = callback.shouldBubble();
}

if (_.isFunction(callback.callback)) {
if (typeof callback.callback === 'function') {
callback.callback(event);
}

if (callback.shouldPreventDefault) {
event.preventDefault();
}

// If the event should not bubble, short-circuit the loop
return shouldBubble;
});
}
};

export default bindHandlerToKeydownEvent;
11 changes: 11 additions & 0 deletions src/libs/KeyboardShortcut/bindHandlerToKeydownEvent/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import type {EventHandler} from '../index';

type KeyCommandEvent = {input: string; modifierFlags?: string};

type GetDisplayName = (key: string, modifiers: string | string[]) => string;

type BindHandlerToKeydownEvent = (getDisplayName: GetDisplayName, eventHandlers: Record<string, EventHandler[]>, keyCommandEvent: KeyCommandEvent, event: KeyboardEvent) => void;

export default BindHandlerToKeydownEvent;

export type {KeyCommandEvent};
27 changes: 0 additions & 27 deletions src/libs/KeyboardShortcut/getKeyEventModifiers.js

This file was deleted.

29 changes: 29 additions & 0 deletions src/libs/KeyboardShortcut/getKeyEventModifiers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import * as KeyCommand from 'react-native-key-command';
import {KeyCommandEvent} from './bindHandlerToKeydownEvent/types';

const keyModifierControl = KeyCommand?.constants.keyModifierControl ?? 'keyModifierControl';
const keyModifierCommand = KeyCommand?.constants.keyModifierCommand ?? 'keyModifierCommand';
const keyModifierShiftControl = KeyCommand?.constants.keyModifierShiftControl ?? 'keyModifierShiftControl';
const keyModifierShiftCommand = KeyCommand?.constants.keyModifierShiftCommand ?? 'keyModifierShiftCommand';

/**
* Gets modifiers from a keyboard event.
*/
function getKeyEventModifiers(event: KeyCommandEvent): string[] {
if (event.modifierFlags === keyModifierControl) {
return ['CONTROL'];
}
if (event.modifierFlags === keyModifierCommand) {
return ['META'];
}
if (event.modifierFlags === keyModifierShiftControl) {
return ['CONTROL', 'Shift'];
}
if (event.modifierFlags === keyModifierShiftCommand) {
return ['META', 'Shift'];
}

return [];
}

export default getKeyEventModifiers;
Loading