Merge pull request #219 from bustlelabs/key-command-modifiers-216

Match key commands to modifiers exactly
bustle · Nov 9, 2015 · 57280b0 · 57280b0
2 parents 0f26292 + 8d2b95d
commit 57280b0
Show file tree

Hide file tree

Showing 5 changed files with 127 additions and 60 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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`);
   });
-});
+});
+
+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');
+});