Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Reorganise keycode file
  • Loading branch information
ellatrix committed May 6, 2018
commit 3fca8d04e0a7429f278e75d5151aac1f9b463d4f
11 changes: 5 additions & 6 deletions blocks/rich-text/format-toolbar/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,36 +16,35 @@ import { prependHTTP } from '@wordpress/url';
/**
* Internal dependencies
*/
import { accessShortcut, primaryShortcut } from 'utils/keycodes';
import './style.scss';
import UrlInput from '../../url-input';
import { filterURLForDisplay } from '../../../editor/utils/url';

const { ESCAPE, LEFT, RIGHT, UP, DOWN, BACKSPACE, ENTER } = keycodes;
const { ESCAPE, LEFT, RIGHT, UP, DOWN, BACKSPACE, ENTER, displayShortcut } = keycodes;

const FORMATTING_CONTROLS = [
{
icon: 'editor-bold',
title: __( 'Bold' ),
shortcut: primaryShortcut( 'B' ),
shortcut: displayShortcut.primary( 'b' ),
format: 'bold',
},
{
icon: 'editor-italic',
title: __( 'Italic' ),
shortcut: primaryShortcut( 'I' ),
shortcut: displayShortcut.primary( 'i' ),
format: 'italic',
},
{
icon: 'editor-strikethrough',
title: __( 'Strikethrough' ),
shortcut: accessShortcut( 'D' ),
shortcut: displayShortcut.access( 'd' ),
format: 'strikethrough',
},
{
icon: 'admin-links',
title: __( 'Link' ),
shortcut: primaryShortcut( 'K' ),
shortcut: displayShortcut.primary( 'k' ),
format: 'link',
},
];
Expand Down
12 changes: 6 additions & 6 deletions blocks/rich-text/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ import { EVENTS } from './constants';
import { withBlockEditContext } from '../block-edit/context';
import { domToFormat, valueToString } from './format';

const { BACKSPACE, DELETE, ENTER } = keycodes;
const { BACKSPACE, DELETE, ENTER, rawShortcut } = keycodes;

/**
* Returns true if the node is the inline node boundary. This is used in node
Expand Down Expand Up @@ -194,11 +194,11 @@ export class RichText extends Component {
this.props.onSetup( editor );
}

editor.shortcuts.add( 'meta+k', '', () => this.changeFormats( { link: { isAdding: true } } ) );
editor.shortcuts.add( 'access+a', '', () => this.changeFormats( { link: { isAdding: true } } ) );
editor.shortcuts.add( 'access+s', '', () => this.changeFormats( { link: undefined } ) );
editor.shortcuts.add( 'access+d', '', () => this.changeFormats( { strikethrough: ! this.state.formats.strikethrough } ) );
editor.shortcuts.add( 'access+x', '', () => this.changeFormats( { code: ! this.state.formats.code } ) );
editor.shortcuts.add( rawShortcut.primary( 'k' ), '', () => this.changeFormats( { link: { isAdding: true } } ) );
editor.shortcuts.add( rawShortcut.access( 'a' ), '', () => this.changeFormats( { link: { isAdding: true } } ) );
Copy link
Member

Choose a reason for hiding this comment

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

Yay, when I saw these I thought "this could be refactored". This is great 👍

editor.shortcuts.add( rawShortcut.access( 's' ), '', () => this.changeFormats( { link: undefined } ) );
editor.shortcuts.add( rawShortcut.access( 'd' ), '', () => this.changeFormats( { strikethrough: ! this.state.formats.strikethrough } ) );
editor.shortcuts.add( rawShortcut.access( 'x' ), '', () => this.changeFormats( { code: ! this.state.formats.code } ) );
}

/**
Expand Down
10 changes: 6 additions & 4 deletions edit-post/keyboard-shortcuts.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
/**
* Internal dependencies
* WordPress dependencies
*/
import { secondaryKeyCode, secondaryShortcut } from 'utils/keycodes';
import { keycodes } from '@wordpress/utils';

const { rawShortcut, displayShortcut } = keycodes;

export default {
toggleEditorMode: {
value: secondaryKeyCode( 'm' ),
label: secondaryShortcut( 'M' ),
value: rawShortcut.secondary( 'm' ),
label: displayShortcut.secondary( 'm' ),
Copy link
Member

Choose a reason for hiding this comment

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

Nice, the consistency here is better 👍

},
};
8 changes: 3 additions & 5 deletions editor/components/editor-history/redo.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,16 @@ import { __ } from '@wordpress/i18n';
import { IconButton } from '@wordpress/components';
import { withSelect, withDispatch } from '@wordpress/data';
import { compose } from '@wordpress/element';
import { keycodes } from '@wordpress/utils';

/**
* Internal dependencies
*/
import { primaryShortcut } from 'utils/keycodes';
const { displayShortcut } = keycodes;

function EditorHistoryRedo( { hasRedo, redo } ) {
return (
<IconButton
icon="redo"
label={ __( 'Redo' ) }
shortcut={ primaryShortcut( 'Y' ) }
shortcut={ displayShortcut.primaryShift( 'z' ) }
disabled={ ! hasRedo }
onClick={ redo }
className="editor-history__undo"
Expand Down
8 changes: 3 additions & 5 deletions editor/components/editor-history/undo.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,16 @@ import { __ } from '@wordpress/i18n';
import { IconButton } from '@wordpress/components';
import { withSelect, withDispatch } from '@wordpress/data';
import { compose } from '@wordpress/element';
import { keycodes } from '@wordpress/utils';

/**
* Internal dependencies
*/
import { primaryShortcut } from 'utils/keycodes';
const { displayShortcut } = keycodes;

function EditorHistoryUndo( { hasUndo, undo } ) {
return (
<IconButton
icon="undo"
label={ __( 'Undo' ) }
shortcut={ primaryShortcut( 'Z' ) }
shortcut={ displayShortcut.primary( 'z' ) }
disabled={ ! hasUndo }
onClick={ undo }
className="editor-history__undo"
Expand Down
6 changes: 4 additions & 2 deletions editor/components/post-saved-state/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@ import { __ } from '@wordpress/i18n';
import { Dashicon, IconButton, withSafeTimeout } from '@wordpress/components';
import { Component, compose } from '@wordpress/element';
import { withSelect, withDispatch } from '@wordpress/data';
import { keycodes } from '@wordpress/utils';

/**
* Internal dependencies
*/
import './style.scss';
import PostSwitchToDraftButton from '../post-switch-to-draft-button';
import { primaryShortcut } from 'utils/keycodes';

const { displayShortcut } = keycodes;

/**
* Component showing whether the post is saved or not and displaying save links.
Expand Down Expand Up @@ -69,7 +71,7 @@ export class PostSavedState extends Component {
className="editor-post-save-draft"
onClick={ onSave }
icon="cloud-upload"
shortcut={ primaryShortcut( 'S' ) }
shortcut={ displayShortcut.primary( 's' ) }
>
{ __( 'Save Draft' ) }
</IconButton>
Expand Down
134 changes: 45 additions & 89 deletions utils/keycodes.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@
* on Windows Control will usually come first. So don't provide your own
* shortcut combos directly to keyboardShortcut().
*/

/**
* External dependencies
*/
import { get, mapValues } from 'lodash';

export const BACKSPACE = 8;
export const TAB = 9;
export const ENTER = 13;
Expand All @@ -23,8 +29,8 @@ export const F10 = 121;

export const ALT = 'alt';
export const CTRL = 'ctrl';
export const PRIMARY = 'mod';
export const META = 'meta';
// Understood in both Mousetrap and TinyMCE.
export const COMMAND = 'meta';
export const SHIFT = 'shift';

/**
Expand All @@ -38,99 +44,49 @@ export function isMacOS( _window = window ) {
return _window.navigator.platform.indexOf( 'Mac' ) !== -1;
}

/**
* Create a keyboard shortcut based on a string of modifiers + key(s).
*
* This function is not intended to be used directly by developers.
* Instead, use primaryShortcut(), accessShortcut(), etc.
*
* @param {string} keys Modifier and keyboard keys, seperated by "+".
* @param {Object} _isMacOS isMacOS function by default; used for DI testing.
*
* @return {string} The keyboard shortcut.
*/
export function keyboardShortcut( keys, _isMacOS = isMacOS ) {
const isMac = _isMacOS();

const alt = isMac ? '⌥option' : 'Alt';
const meta = isMac ? '⌃control' : '⊞';
const primary = isMac ? '⌘' : 'Ctrl';
const shift = isMac ? '⇧shift' : 'Shift';

const replacementKeyMap = {
[ ALT ]: alt,
[ META ]: meta,
[ PRIMARY ]: primary,
[ SHIFT ]: shift,
};

return keys
.replace( /\s/g, '' )
.split( '+' )
.map( ( key ) => {
return replacementKeyMap.hasOwnProperty( key ) ?
replacementKeyMap[ key ] : key;
} )
.join( '+' )
// Because we use just the clover symbol for MacOS's "command" key, remove
// the key join character ("+") between it and the final character if that
// final character is alphanumeric. ⌘S looks nicer than ⌘+S.
.replace( /⌘\+([a-zA-Z0-9])$/g, '⌘$1' );
}

/**
* Create an access key shortcut based on a single character.
*
* Access key combo is:
* - Control+Alt on MacOS.
* - Shift+Alt on Windows/everywhere else.
*
* @param {string} character The character for the access combination.
* @param {Object} _isMacOS isMacOS function by default; used for DI testing.
*
* @return {string} The keyboard shortcut.
*/
export function accessShortcut( character, _isMacOS = isMacOS ) {
return keyboardShortcut( accessKeyCode( character.toUpperCase(), _isMacOS ), _isMacOS );
}

export function accessKeyCode( character, _isMacOS = isMacOS ) {
const keyCombo = _isMacOS() ? `${ META }+${ ALT }` : `${ SHIFT }+${ ALT }`;
return `${ keyCombo }+${ character }`;
}
const modifiers = {
primary: ( _isMac ) => _isMac() ? [ COMMAND ] : [ CTRL ],
Copy link
Member

Choose a reason for hiding this comment

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

This is much more immediately understandable than my old way. ✨

Copy link
Member

Choose a reason for hiding this comment

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

Why do these need to be run as functions? Will they ever change over the lifetime of the app?

Edit: And if the answer is for testing, it shouldn't really impact the implementation so much. Can be "good" for it to be made generic, though ideally we have some caching along the way in runtime use, so we're not running _window.navigator.platform.indexOf( 'Mac' ) !== -1; hundreds of times per page session.

Copy link
Member

Choose a reason for hiding this comment

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

It's all for testing; previously this was run once and stored in a constant, but it was also entirely untested. It could probably be cached somehow as a perf-versus-readability tradeoff. 😄

It shouldn't change in regular usage though, so yeah it's worth doing.

primaryShift: ( _isMac ) => _isMac() ? [ SHIFT, COMMAND ] : [ CTRL, SHIFT ],
secondary: ( _isMac ) => _isMac() ? [ SHIFT, ALT, COMMAND ] : [ CTRL, SHIFT, ALT ],
access: ( _isMac ) => _isMac() ? [ CTRL, ALT ] : [ SHIFT, ALT ],
};

/**
* Create a modifier shortcut based on a single character.
*
* This will output Ctrl+G on Windows when "G" is supplied as an argument.
* This will output Command+G on MacOS when "G" is supplied as an argument.
* An object that contains functions to get raw shortcuts.
* E.g. rawShortcut.primary( 'm' ) will return 'meta+m' on Mac.
* These are intended for user with the KeyboardShortcuts component or TinyMCE.
*
* @param {string} character The character for the key command.
* @param {Object} _isMacOS isMacOS function by default; used for DI testing.
*
* @return {string} The keyboard shortcut.
* @type {Object} Keyed map of functions to raw shortcuts.
*/
export function primaryShortcut( character, _isMacOS = isMacOS ) {
return keyboardShortcut( `${ PRIMARY }+${ character.toUpperCase() }`, _isMacOS );
}
export const rawShortcut = mapValues( modifiers, ( modifier ) => {
return ( character, _isMac = isMacOS ) => {
return [ ...modifier( _isMac ), character.toLowerCase() ].join( '+' );
};
} );

/**
* Create an access key + primary key shortcut based on a single character.
*
* Access key combo is:
* - Control+Alt+Command on MacOS.
* - Control+Shift+Alt on Windows/everywhere else.
*
* @param {string} character The character for the access combination.
* @param {Object} _isMacOS isMacOS function by default; used for DI testing.
* An object that contains functions to display shortcuts.
* E.g. displayShortcut.primary( 'm' ) will return '⌘M' on Mac.
*
* @return {string} The keyboard shortcut.
* @type {Object} Keyed map of functions to display shortcuts.
*/
export function secondaryShortcut( character, _isMacOS = isMacOS ) {
return keyboardShortcut( secondaryKeyCode( character.toUpperCase(), _isMacOS ), _isMacOS );
}
export const displayShortcut = mapValues( modifiers, ( modifier ) => {
Copy link
Member

Choose a reason for hiding this comment

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

Ah, cool. Sorry I didn't think to use this, I'm not used to having lodash around.

return ( character, _isMac = isMacOS ) => {
const isMac = _isMac();
const replacementKeyMap = {
[ ALT ]: isMac ? '⌥option' : 'Alt',
[ CTRL ]: isMac ? '⌃control' : 'Ctrl',
[ COMMAND ]: '⌘',
[ SHIFT ]: isMac ? '⇧shift' : 'Shift',
};
const shortcut = [
...modifier( _isMac ).map( ( key ) => get( replacementKeyMap, key, key ) ),
Copy link
Member

Choose a reason for hiding this comment

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

You defeated my new ESLint custom rule! (#6247)

Which is to say, I should make this rule more strict 😛

character.toUpperCase(),
].join( '+' );

export function secondaryKeyCode( character, _isMacOS = isMacOS ) {
const keyCombo = _isMacOS() ? `${ SHIFT }+${ ALT }+${ PRIMARY }` : `${ PRIMARY }+${ SHIFT }+${ ALT }`;
return `${ keyCombo }+${ character }`;
}
// Because we use just the clover symbol for MacOS's "command" key, remove
// the key join character ("+") between it and the final character if that
// final character is alphanumeric. ⌘S looks nicer than ⌘+S.
return shortcut.replace( /⌘\+([A-Z0-9])$/g, '⌘$1' );
};
} );
Loading