From 8d2b95d550413aeb253545c4bfb66d80cfdcfa13 Mon Sep 17 00:00:00 2001 From: Cory Forsyth Date: Mon, 9 Nov 2015 17:45:52 -0500 Subject: [PATCH] Match key commands to modifiers exactly * Add `modiferMask` property to key and key commands * Changes buildKeyCommand to allow multiple modifiers, i.e. `str: 'META+SHIFT+K' * Add additional assertions to ensure key commands are valid * Deprecate using a `modifier` property when creating a key command fixes #216 --- README.md | 6 +- src/js/editor/key-commands.js | 60 ++++++++++---------- src/js/utils/key.js | 40 ++++++++----- tests/helpers/dom.js | 3 +- tests/unit/editor/key-commands-test.js | 78 ++++++++++++++++++++++---- 5 files changed, 127 insertions(+), 60 deletions(-) diff --git a/README.md b/README.md index 5ad4b9b0f..eeace1016 100644 --- a/README.md +++ b/README.md @@ -145,9 +145,9 @@ editor.registerKeyCommand(boldKeyCommand); All key commands must have `str` and `run` properties as shown above. -`str` describes the key combination to use and may be a single key, or a modifier and a key separated by `+`. +`str` describes the key combination to use and may be a single key, or modifier(s) and a key separated by `+`, e.g.: `META+K` (cmd-K), `META+SHIFT+K` (cmd-shift-K) -Modifiers can be one of `CTRL`, `META` or `SHIFT`. +Modifiers can be any of `CTRL`, `META`, `SHIFT`, or `ALT`. The key can be any of the alphanumeric characters on the keyboard, or one of the following special keys: @@ -182,7 +182,7 @@ const enterKeyCommand = { editor.registerKeyCommand(enterKeyCommand); ``` -To fall-back to the default behavior, simply return `false` from `run`. +To fall-back to the default behavior, return `false` from `run`. ### Configuring text expansions diff --git a/src/js/editor/key-commands.js b/src/js/editor/key-commands.js index 01a3c58ff..68a888b92 100644 --- a/src/js/editor/key-commands.js +++ b/src/js/editor/key-commands.js @@ -1,11 +1,11 @@ import Key from '../utils/key'; import { MODIFIERS, SPECIAL_KEYS } from '../utils/key'; -import { filter } from '../utils/array-utils'; +import { filter, reduce } from '../utils/array-utils'; import Position from '../utils/cursor/position'; +import assert from '../utils/assert'; export const DEFAULT_KEY_COMMANDS = [{ - modifier: MODIFIERS.META, - str: 'B', + str: 'META+B', run(editor) { if (editor.cursor.hasSelection()) { editor.run(postEditor => postEditor.toggleMarkup('strong')); @@ -14,8 +14,7 @@ export const DEFAULT_KEY_COMMANDS = [{ } } }, { - modifier: MODIFIERS.CTRL, - str: 'B', + str: 'CTRL+B', run(editor) { if (editor.cursor.hasSelection()) { editor.run(postEditor => postEditor.toggleMarkup('strong')); @@ -24,8 +23,7 @@ export const DEFAULT_KEY_COMMANDS = [{ } } }, { - modifier: MODIFIERS.META, - str: 'I', + str: 'META+I', run(editor) { if (editor.cursor.hasSelection()) { editor.run(postEditor => postEditor.toggleMarkup('em')); @@ -34,8 +32,7 @@ export const DEFAULT_KEY_COMMANDS = [{ } } }, { - modifier: MODIFIERS.CTRL, - str: 'I', + str: 'CTRL+I', run(editor) { if (editor.cursor.hasSelection()) { editor.run(postEditor => postEditor.toggleMarkup('em')); @@ -44,8 +41,7 @@ export const DEFAULT_KEY_COMMANDS = [{ } } }, { - modifier: MODIFIERS.CTRL, - str: 'K', + str: 'CTRL+K', run(editor) { let range = editor.cursor.offsets; if (!editor.cursor.hasSelection()) { @@ -55,8 +51,7 @@ export const DEFAULT_KEY_COMMANDS = [{ editor.cursor.moveToPosition(nextPosition); } }, { - modifier: MODIFIERS.META, - str: 'K', + str: 'META+K', run(editor) { if (!editor.cursor.hasSelection()) { return; @@ -77,8 +72,15 @@ export const DEFAULT_KEY_COMMANDS = [{ } }]; -function stringToModifier(string) { - return MODIFIERS[string.toUpperCase()]; +function modifierNamesToMask(modiferNames) { + let defaultVal = 0; + return reduce(modiferNames, + (sum, name) => { + let modifier = MODIFIERS[name.toUpperCase()]; + assert(`No modifier named "${name}" found`, !!modifier); + return sum + modifier; + }, + defaultVal); } function characterToCode(character) { @@ -87,23 +89,25 @@ function characterToCode(character) { if (special) { return special; } else { + assert(`Only 1 character can be used in a key command str (got "${character}")`, + character.length === 1); return upperCharacter.charCodeAt(0); } } export function buildKeyCommand(keyCommand) { - if (!keyCommand.str) { + let { str } = keyCommand; + + if (!str) { return keyCommand; } + assert('[deprecation] Key commands no longer use the `modifier` property', + !keyCommand.modifier); - const str = keyCommand.str; - if (str.indexOf('+') !== -1) { - const [modifierName, character] = str.split('+'); - keyCommand.modifier = stringToModifier(modifierName); - keyCommand.code = characterToCode(character); - } else { - keyCommand.code = characterToCode(str); - } + let [character, ...modifierNames] = str.split('+').reverse(); + + keyCommand.modifierMask = modifierNamesToMask(modifierNames); + keyCommand.code = characterToCode(character); return keyCommand; } @@ -115,11 +119,7 @@ export function validateKeyCommand(keyCommand) { export function findKeyCommands(keyCommands, keyEvent) { const key = Key.fromEvent(keyEvent); - return filter(keyCommands, ({modifier, code}) => { - if (key.keyCode !== code) { - return false; - } - - return (modifier && key.hasModifier(modifier)) || (!modifier && !key.hasAnyModifier()); + return filter(keyCommands, ({modifierMask, code}) => { + return key.keyCode === code && key.modifierMask === modifierMask; }); } diff --git a/src/js/utils/key.js b/src/js/utils/key.js index 38ed1330f..3fac14550 100644 --- a/src/js/utils/key.js +++ b/src/js/utils/key.js @@ -8,9 +8,23 @@ import assert from './assert'; export const MODIFIERS = { META: 1, // also called "command" on OS X CTRL: 2, - SHIFT: 3 + SHIFT: 4, + ALT: 8 // also called "option" on OS X }; +export function modifierMask(event) { + let { + metaKey, shiftKey, ctrlKey, altKey + } = event; + let modVal = (val, modifier) => { + return (val && modifier) || 0; + }; + return modVal(metaKey, MODIFIERS.META) + + modVal(shiftKey, MODIFIERS.SHIFT) + + modVal(ctrlKey, MODIFIERS.CTRL) + + modVal(altKey, MODIFIERS.ALT); +} + export const SPECIAL_KEYS = { BACKSPACE: Keycodes.BACKSPACE, TAB: Keycodes.TAB, @@ -46,6 +60,7 @@ const Key = class Key { constructor(event) { this.keyCode = event.keyCode; this.event = event; + this.modifierMask = modifierMask(event); } static fromEvent(event) { @@ -102,32 +117,27 @@ const Key = class Key { } hasModifier(modifier) { - switch (modifier) { - case MODIFIERS.META: - return this.metaKey; - case MODIFIERS.CTRL: - return this.ctrlKey; - case MODIFIERS.SHIFT: - return this.shiftKey; - default: - throw new Error(`Cannot check for unknown modifier ${modifier}`); - } + return modifier & this.modifierMask; } hasAnyModifier() { - return this.metaKey || this.ctrlKey || this.shiftKey; + return !!this.modifierMask; } get ctrlKey() { - return this.event.ctrlKey; + return MODIFIERS.CTRL & this.modifierMask; } get metaKey() { - return this.event.metaKey; + return MODIFIERS.META & this.modifierMask; } get shiftKey() { - return this.event.shiftKey; + return MODIFIERS.SHIFT & this.modifierMask; + } + + get altKey() { + return MODIFIERS.ALT & this.modifierMask; } isChar(string) { diff --git a/tests/helpers/dom.js b/tests/helpers/dom.js index fa6d2d196..1db5d1bf0 100644 --- a/tests/helpers/dom.js +++ b/tests/helpers/dom.js @@ -276,7 +276,8 @@ const DOMHelper = { triggerCopyEvent, triggerCutEvent, triggerPasteEvent, - getCopyData + getCopyData, + createMockEvent }; export { triggerEvent }; diff --git a/tests/unit/editor/key-commands-test.js b/tests/unit/editor/key-commands-test.js index 5098c2ae8..51b3c8a7f 100644 --- a/tests/unit/editor/key-commands-test.js +++ b/tests/unit/editor/key-commands-test.js @@ -1,5 +1,5 @@ -import { buildKeyCommand } from 'content-kit-editor/editor/key-commands'; -import { MODIFIERS, SPECIAL_KEYS } from 'content-kit-editor/utils/key'; +import { buildKeyCommand, findKeyCommands } from 'content-kit-editor/editor/key-commands'; +import { MODIFIERS, SPECIAL_KEYS, modifierMask as createModifierMask } from 'content-kit-editor/utils/key'; import Keycodes from 'content-kit-editor/utils/keycodes'; import Helpers from '../../test-helpers'; @@ -24,27 +24,36 @@ test('leaves modifier, code and run in place if they exist', (assert) => { assert.equal(run, fn, 'keeps run'); }); -test('translates MODIFIER+CHARACTER string to modifier and code', (assert) => { +test('translates MODIFIER+CHARACTER string to modifierMask and code', (assert) => { - const { modifier, code } = buildKeyCommand({ str: 'meta+k' }); + const { modifierMask, code } = buildKeyCommand({ str: 'meta+k' }); - assert.equal(modifier, MODIFIERS.META, 'translates string to modifier'); + assert.equal(modifierMask, createModifierMask({metaKey: true}), + 'calculates correct modifierMask'); assert.equal(code, 75, 'translates string to code'); }); -test('translates modifier+character string to modifier and code', (assert) => { +test('translates modifier+character string to modifierMask and code', (assert) => { - const { modifier, code } = buildKeyCommand({ str: 'META+K' }); + const { modifierMask, code } = buildKeyCommand({ str: 'META+K' }); - assert.equal(modifier, MODIFIERS.META, 'translates string to modifier'); + assert.equal(modifierMask, createModifierMask({metaKey: true}), + 'calculates correct modifierMask'); + assert.equal(code, 75, 'translates string to code'); +}); + +test('translates multiple modifiers to modifierMask', (assert) => { + const { modifierMask, code } = buildKeyCommand({ str: 'META+SHIFT+K' }); + assert.equal(modifierMask, createModifierMask({metaKey: true, shiftKey: true}), + 'calculates correct modifierMask'); assert.equal(code, 75, 'translates string to code'); }); test('translates uppercase character string to code', (assert) => { - const { modifier, code } = buildKeyCommand({ str: 'K' }); + const { modifierMask, code } = buildKeyCommand({ str: 'K' }); - assert.equal(modifier, undefined, 'no modifier given'); + assert.equal(modifierMask, 0, 'no modifier given'); assert.equal(code, 75, 'translates string to code'); }); @@ -57,6 +66,24 @@ test('translates lowercase character string to code', (assert) => { }); +test('throws when given invalid modifier', (assert) => { + assert.throws(() => { + buildKeyCommand({str: 'MEAT+K'}); + }, /No modifier named.*MEAT.*/); +}); + +test('throws when given `modifier` property (deprecation)', (assert) => { + assert.throws(() => { + buildKeyCommand({str: 'K', modifier: MODIFIERS.META}); + }, /Key commands no longer use.*modifier.* property/); +}); + +test('throws when given str with too many characters', (assert) => { + assert.throws(() => { + buildKeyCommand({str: 'abc'}); + }, /Only 1 character/); +}); + test('translates uppercase special key names to codes', (assert) => { Object.keys(SPECIAL_KEYS).forEach(name => { const { code } = buildKeyCommand({ str: name.toUpperCase() }); @@ -69,4 +96,33 @@ test('translates lowercase special key names to codes', (assert) => { const { code } = buildKeyCommand({ str: name.toLowerCase() }); assert.equal(code, SPECIAL_KEYS[name], `translates ${name} string to code`); }); -}); \ No newline at end of file +}); + +test('`findKeyCommands` matches modifiers exactly', (assert) => { + let cmdK = buildKeyCommand({ + str: 'META+K' + }); + let cmdShiftK = buildKeyCommand({ + str: 'META+SHIFT+K' + }); + let commands = [cmdK, cmdShiftK]; + + let element = null; + let cmdKEvent = Helpers.dom.createMockEvent('keydown', element, { + keyCode: 75, + metaKey: true + }); + let cmdShiftKEvent = Helpers.dom.createMockEvent('keydown', element, { + keyCode: 75, + metaKey: true, + shiftKey: true + }); + + let found = findKeyCommands(commands, cmdKEvent); + assert.ok(found.length && found[0] === cmdK, + 'finds cmd-K command from cmd-k event'); + + found = findKeyCommands(commands, cmdShiftKEvent); + assert.ok(found.length && found[0] === cmdShiftK, + 'finds cmd-shift-K command from cmd-shift-k event'); +});