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

KeyboardShortcuts: Convert to TypeScript #47429

Merged
merged 9 commits into from
Jan 30, 2023
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
1 change: 1 addition & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
- `DropdownMenu`: migrate Storybook to controls ([47149](https://github.com/WordPress/gutenberg/pull/47149)).
- Removed deprecated `@storybook/addon-knobs` dependency from the package ([47152](https://github.com/WordPress/gutenberg/pull/47152)).
- `ColorListPicker`: Convert to TypeScript ([#46358](https://github.com/WordPress/gutenberg/pull/46358)).
- `KeyboardShortcuts`: Convert to TypeScript ([#47429](https://github.com/WordPress/gutenberg/pull/47429)).
- `ColorPalette`, `BorderControl`, `GradientPicker`: refine types and logic around single vs multiple palettes
([#47384](https://github.com/WordPress/gutenberg/pull/47384)).
- `Button`: Convert to TypeScript ([#46997](https://github.com/WordPress/gutenberg/pull/46997)).
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/keyboard-shortcuts/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ The component accepts the following props:

Elements to render, upon whom key events are to be monitored.

- Type: `Element` | `Element[]`
- Type: `ReactNode`
- Required: No

### shortcuts
Expand Down
53 changes: 0 additions & 53 deletions packages/components/src/keyboard-shortcuts/index.js

This file was deleted.

93 changes: 93 additions & 0 deletions packages/components/src/keyboard-shortcuts/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
/**
* WordPress dependencies
*/
import { useRef, Children } from '@wordpress/element';
import { useKeyboardShortcut } from '@wordpress/compose';

/**
* Internal dependencies
*/
import type { KeyboardShortcutProps, KeyboardShortcutsProps } from './types';

function KeyboardShortcut( {
target,
callback,
shortcut,
bindGlobal,
eventName,
}: KeyboardShortcutProps ) {
useKeyboardShortcut( shortcut, callback, {
bindGlobal,
target,
eventName,
} );

return null;
}

/**
* `KeyboardShortcuts` is a component which handles keyboard sequences during the lifetime of the rendering element.
*
* When passed children, it will capture key events which occur on or within the children. If no children are passed, events are captured on the document.
*
* It uses the [Mousetrap](https://craig.is/killing/mice) library to implement keyboard sequence bindings.
*
* ```jsx
* import { KeyboardShortcuts } from '@wordpress/components';
* import { useState } from '@wordpress/element';
*
* const MyKeyboardShortcuts = () => {
* const [ isAllSelected, setIsAllSelected ] = useState( false );
* const selectAll = () => {
* setIsAllSelected( true );
* };
*
* return (
* <div>
* <KeyboardShortcuts
* shortcuts={ {
* 'mod+a': selectAll,
* } }
* />
* [cmd/ctrl + A] Combination pressed? { isAllSelected ? 'Yes' : 'No' }
* </div>
* );
* };
* ```
*/
function KeyboardShortcuts( {
children,
shortcuts,
bindGlobal,
eventName,
}: KeyboardShortcutsProps ) {
const target = useRef( null );
Copy link
Member Author

Choose a reason for hiding this comment

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

Runtime change. Should be safe.

- const target = useRef();
+ const target = useRef( null );


const element = Object.entries( shortcuts ?? {} ).map(
( [ shortcut, callback ] ) => (
<KeyboardShortcut
key={ shortcut }
shortcut={ shortcut }
callback={ callback }
bindGlobal={ bindGlobal }
eventName={ eventName }
target={ target }
/>
)
);

// Render as non-visual if there are no children pressed. Keyboard
// events will be bound to the document instead.
if ( ! Children.count( children ) ) {
return <>{ element }</>;
}
Comment on lines +81 to +83
Copy link
Member Author

Choose a reason for hiding this comment

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

Runtime change. Without it, KeyboardShortcuts basically becomes invalid as a React component.

- return element;
+ return <>{ element }</>;


return (
<div ref={ target }>
{ element }
{ children }
</div>
);
}

export default KeyboardShortcuts;
60 changes: 60 additions & 0 deletions packages/components/src/keyboard-shortcuts/stories/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/**
* External dependencies
*/
import type { ComponentMeta, ComponentStory } from '@storybook/react';

/**
* Internal dependencies
*/
import KeyboardShortcuts from '..';

const meta: ComponentMeta< typeof KeyboardShortcuts > = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for adding Storybook examples!

component: KeyboardShortcuts,
title: 'Components/KeyboardShortcuts',
parameters: {
controls: { expanded: true },
docs: { source: { state: 'open' } },
},
};
export default meta;

const Template: ComponentStory< typeof KeyboardShortcuts > = ( props ) => (
<KeyboardShortcuts { ...props } />
);

export const Default = Template.bind( {} );
Default.args = {
shortcuts: {
// eslint-disable-next-line no-alert
a: () => window.alert( 'You hit "a"!' ),
// eslint-disable-next-line no-alert
b: () => window.alert( 'You hit "b"!' ),
},
children: (
<div>
<p>{ `Hit the "a" or "b" key in this textarea:` }</p>
<textarea />
</div>
),
};
Default.parameters = {
docs: {
source: {
code: `
<KeyboardShortcuts
shortcuts={{
a: () => window.alert('You hit "a"!'),
b: () => window.alert('You hit "b"!'),
}}
>
<div>
<p>
Hit the "a" or "b" key in this textarea:
</p>
<textarea />
</div>
</KeyboardShortcuts>
`,
},
},
};
Original file line number Diff line number Diff line change
@@ -1,19 +1,29 @@
/**
* External dependencies
*/
import { fireEvent, render, screen } from '@testing-library/react';
import { createEvent, fireEvent, render, screen } from '@testing-library/react';

/**
* Internal dependencies
*/
import KeyboardShortcuts from '../';
import KeyboardShortcuts from '..';

describe( 'KeyboardShortcuts', () => {
function keyPress( which, target ) {
function keyPress(
which: KeyboardEvent[ 'which' ],
target: Parameters< typeof fireEvent >[ 0 ]
) {
[ 'keydown', 'keypress', 'keyup' ].forEach( ( eventName ) => {
const event = new window.Event( eventName, { bubbles: true } );
event.keyCode = which;
event.which = which;
const event = createEvent(
eventName,
target,
{
bubbles: true,
keyCode: which,
which,
},
{ EventType: 'KeyboardEvent' }
);
fireEvent( target, event );
} );
}
Expand Down
51 changes: 51 additions & 0 deletions packages/components/src/keyboard-shortcuts/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/**
* WordPress dependencies
*/
import type { useKeyboardShortcut } from '@wordpress/compose';

// TODO: We wouldn't have to do this if this type was exported from `@wordpress/compose`.
type WPKeyboardShortcutConfig = NonNullable<
Parameters< typeof useKeyboardShortcut >[ 2 ]
>;
Comment on lines +7 to +9
Copy link
Member Author

Choose a reason for hiding this comment

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

A bit weird, but the WPKeyboardShortcutConfig type is not directly exported from the wp-compose package. I generally want to avoid doing fragile index-based access on Parameters<>, but 🤷

I've tried to balance readability and DRYness in this types file. Hope it doesn't look too random 😅

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, this looks like a good compromise. The only improvement that comes to mind is to add a little inline comment, basically explaining that the reason why we're doing this, is because that type is not exported.

As a small follow-up PR, we may even consider exporting that type from compose package and therefore update this line of code too.


export type KeyboardShortcutProps = {
shortcut: string | string[];
/**
* @see {@link https://craig.is/killing/mice Mousetrap documentation}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

*/
callback: ( event: Mousetrap.ExtendedKeyboardEvent, combo: string ) => void;
} & Pick< WPKeyboardShortcutConfig, 'bindGlobal' | 'eventName' | 'target' >;

export type KeyboardShortcutsProps = {
/**
* Elements to render, upon whom key events are to be monitored.
*/
children?: React.ReactNode;
/**
* An object of shortcut bindings, where each key is a keyboard combination,
* the value of which is the callback to be invoked when the key combination is pressed.
*
* The value of each shortcut should be a consistent function reference, not an anonymous function.
* Otherwise, the callback will not be correctly unbound when the component unmounts.
*
* The `KeyboardShortcuts` component will not update to reflect a changed `shortcuts` prop.
* If you need to change shortcuts, mount a separate `KeyboardShortcuts` element,
* which can be achieved by assigning a unique `key` prop.
*
* @see {@link https://craig.is/killing/mice Mousetrap documentation}
*/
shortcuts: Record< string, KeyboardShortcutProps[ 'callback' ] >;
ciampo marked this conversation as resolved.
Show resolved Hide resolved
/**
* By default, a callback will not be invoked if the key combination occurs in an editable field.
* Pass `bindGlobal` as `true` if the key events should be observed globally, including within editable fields.
*
* Tip: If you need some but not all keyboard events to be observed globally,
* simply render two distinct `KeyboardShortcuts` elements, one with and one without the `bindGlobal` prop.
*/
bindGlobal?: KeyboardShortcutProps[ 'bindGlobal' ];
/**
* By default, a callback is invoked in response to the `keydown` event.
* To override this, pass `eventName` with the name of a specific keyboard event.
*/
eventName?: KeyboardShortcutProps[ 'eventName' ];
};
1 change: 0 additions & 1 deletion packages/components/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@
"src/higher-order/with-filters",
"src/higher-order/with-focus-return",
"src/higher-order/with-notices",
"src/keyboard-shortcuts",
"src/menu-items-choice",
"src/navigation",
"src/notice",
Expand Down