Merge pull request #115 from github/include-shift

Include `Shift` key with uppercase letter hotkeys

README.md

@@ -111,6 +111,7 @@ for (const el of document.querySelectorAll('[data-shortcut]')) {
    3. Due to the inconsistent lowercasing of `event.key` on Mac and iOS when `Meta` is pressed along with `Shift`, it is recommended to avoid hotkey strings containing both `Mod` and `Shift`.
 7. `"Plus"` and `"Space"` are special key names to represent the `+` and ` ` keys respectively, because these symbols cannot be represented in the normal hotkey string syntax.
 8. You can use the comma key `,` as a hotkey, e.g. `a,,` would activate if the user typed `a` or `,`. `Control+,,x` would activate for `Control+,` or `x`.
+9. `"Shift"` should be included if it would be held and the key is uppercase: ie, `Shift+A` not `A`. Note however that MacOS outputs lowercase keys when `Meta+Shift` is held (ie, `Meta+Shift+a`); see 6.3 above.
 
 ### Example
 

src/hotkey.ts

@@ -38,9 +38,9 @@ export function eventToHotkeyString(
   event: KeyboardEvent,
   platform: string = navigator.platform
 ): NormalizedHotkeyString {
-  const {ctrlKey, altKey, metaKey, key} = event
+  const {ctrlKey, altKey, metaKey, shiftKey, key} = event
   const hotkeyString: string[] = []
-  const modifiers: boolean[] = [ctrlKey, altKey, metaKey, showShift(event)]
+  const modifiers: boolean[] = [ctrlKey, altKey, metaKey, shiftKey]
 
   for (const [i, mod] of modifiers.entries()) {
     if (mod) hotkeyString.push(modifierKeyNames[i])
@@ -58,12 +58,6 @@ export function eventToHotkeyString(
 
 const modifierKeyNames: string[] = ['Control', 'Alt', 'Meta', 'Shift']
 
-// We don't want to show `Shift` when `event.key` is capital
-function showShift(event: KeyboardEvent): boolean {
-  const {shiftKey, code, key} = event
-  return shiftKey && !(code.startsWith('Key') && key.toUpperCase() === key)
-}
-
 /**
  * Normalizes a hotkey string before comparing it to the serialized event
  * string produced by `eventToHotkeyString`.

test/test.js

@@ -98,25 +98,25 @@ describe('hotkey', function () {
     })
 
     it('triggers elements with capitalised key', function () {
-      setHTML('<button id="button1" data-hotkey="B">Button 1</button>')
+      setHTML('<button id="button1" data-hotkey="Shift+B">Button 1</button>')
       document.dispatchEvent(new KeyboardEvent('keydown', {shiftKey: true, code: 'KeyB', key: 'B'}))
       assert.include(elementsActivated, 'button1')
     })
 
     it('dispatches an event on the element once fired', function () {
-      setHTML('<button id="button1" data-hotkey="B">Button 1</button>')
+      setHTML('<button id="button1" data-hotkey="Shift+B">Button 1</button>')
       let fired = false
       document.querySelector('#button1').addEventListener('hotkey-fire', event => {
         fired = true
-        assert.deepEqual(event.detail.path, ['B'])
+        assert.deepEqual(event.detail.path, ['Shift+B'])
         assert.equal(event.cancelable, true)
       })
       document.dispatchEvent(new KeyboardEvent('keydown', {shiftKey: true, code: 'KeyB', key: 'B'}))
       assert.ok(fired, 'button1 did not receive a hotkey-fire event')
     })
 
     it('wont trigger action if the hotkey-fire event is cancelled', function () {
-      setHTML('<button id="button1" data-hotkey="B">Button 1</button>')
+      setHTML('<button id="button1" data-hotkey="Shift+B">Button 1</button>')
       document.querySelector('#button1').addEventListener('hotkey-fire', event => event.preventDefault())
       document.dispatchEvent(new KeyboardEvent('keydown', {shiftKey: true, code: 'KeyB', key: 'B'}))
       assert.notInclude(elementsActivated, 'button1')
@@ -262,19 +262,19 @@ describe('hotkey', function () {
 
   describe('eventToHotkeyString', function () {
     const tests = [
-      ['Control+J', {ctrlKey: true, shiftKey: true, code: 'KeyJ', key: 'J'}],
+      ['Control+Shift+J', {ctrlKey: true, shiftKey: true, code: 'KeyJ', key: 'J'}],
       ['Control+Shift+j', {ctrlKey: true, shiftKey: true, code: 'KeyJ', key: 'j'}],
       ['Control+j', {ctrlKey: true, code: 'KeyJ', key: 'j'}],
       ['Meta+Shift+p', {key: 'p', metaKey: true, shiftKey: true, code: 'KeyP'}],
       ['Meta+Shift+8', {key: '8', metaKey: true, shiftKey: true, code: 'Digit8'}],
       ['Control+Shift+7', {key: '7', ctrlKey: true, shiftKey: true, code: 'Digit7'}],
-      ['J', {shiftKey: true, code: 'KeyJ', key: 'J'}],
+      ['Shift+J', {shiftKey: true, code: 'KeyJ', key: 'J'}],
       ['/', {key: '/', code: ''}],
       ['1', {key: '1', code: 'Digit1'}],
       ['Control+Shift+`', {ctrlKey: true, shiftKey: true, key: '`'}],
       ['c', {key: 'c', code: 'KeyC'}],
-      ['S', {key: 'S', shiftKey: true, code: 'KeyS'}],
-      ['!', {key: '!', shiftKey: true, code: 'KeyS'}],
+      ['Shift+S', {key: 'S', shiftKey: true, code: 'KeyS'}],
+      ['Shift+!', {key: '!', shiftKey: true, code: 'Digit1'}],
       ['Control+Shift', {ctrlKey: true, shiftKey: true, key: 'Shift'}],
       ['Control+Shift', {ctrlKey: true, shiftKey: true, key: 'Control'}],
       ['Alt+s', {altKey: true, key: 's'}],