From 1bfa9e0906c4f5b353fb9c5cd437971740f56b0d Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Thu, 18 Feb 2021 19:03:59 +0100 Subject: [PATCH 01/16] Using Cmd on Mac instead of Ctrl. --- .../src/view/observer/keyobserver.js | 3 +- .../ckeditor5-list/src/todolistediting.js | 2 +- .../utils/injectunsafekeystrokeshandling.js | 4 +- packages/ckeditor5-utils/src/keyboard.js | 56 ++++++++++++++----- 4 files changed, 47 insertions(+), 18 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/observer/keyobserver.js b/packages/ckeditor5-engine/src/view/observer/keyobserver.js index 36a3f9d5de9..8b9e0111e01 100644 --- a/packages/ckeditor5-engine/src/view/observer/keyobserver.js +++ b/packages/ckeditor5-engine/src/view/observer/keyobserver.js @@ -29,8 +29,9 @@ export default class KeyObserver extends DomEventObserver { keyCode: domEvt.keyCode, altKey: domEvt.altKey, - ctrlKey: domEvt.ctrlKey || domEvt.metaKey, + ctrlKey: domEvt.ctrlKey, shiftKey: domEvt.shiftKey, + metaKey: domEvt.metaKey, get keystroke() { return getCode( this ); diff --git a/packages/ckeditor5-list/src/todolistediting.js b/packages/ckeditor5-list/src/todolistediting.js index ccc6fb1dbad..f64c1b5b832 100644 --- a/packages/ckeditor5-list/src/todolistediting.js +++ b/packages/ckeditor5-list/src/todolistediting.js @@ -113,7 +113,7 @@ export default class TodoListEditing extends Plugin { this.listenTo( editing.view.document, 'keydown', jumpOverCheckmarkOnSideArrowKeyPress( model, editor.locale ) ); // Toggle check state of selected to-do list items on keystroke. - editor.keystrokes.set( 'Ctrl+space', () => editor.execute( 'checkTodoList' ) ); + editor.keystrokes.set( 'Ctrl!+space', () => editor.execute( 'checkTodoList' ) ); // Remove `todoListChecked` attribute when a host element is no longer a to-do list item. const listItemsToFix = new Set(); diff --git a/packages/ckeditor5-typing/src/utils/injectunsafekeystrokeshandling.js b/packages/ckeditor5-typing/src/utils/injectunsafekeystrokeshandling.js index 73183651c04..d133f020f40 100644 --- a/packages/ckeditor5-typing/src/utils/injectunsafekeystrokeshandling.js +++ b/packages/ckeditor5-typing/src/utils/injectunsafekeystrokeshandling.js @@ -171,8 +171,8 @@ for ( let code = 112; code <= 135; code++ ) { * @returns {Boolean} */ export function isNonTypingKeystroke( keyData ) { - // Keystrokes which contain Ctrl don't represent typing. - if ( keyData.ctrlKey ) { + // Keystrokes which contain Ctrl or Cmd don't represent typing. + if ( keyData.ctrlKey || keyData.metaKey ) { return true; } diff --git a/packages/ckeditor5-utils/src/keyboard.js b/packages/ckeditor5-utils/src/keyboard.js index 1feda514de4..9b46fb2e2fe 100644 --- a/packages/ckeditor5-utils/src/keyboard.js +++ b/packages/ckeditor5-utils/src/keyboard.js @@ -13,12 +13,14 @@ import CKEditorError from './ckeditorerror'; import env from './env'; const macGlyphsToModifiers = { + '⌃': 'ctrl!', '⌘': 'ctrl', '⇧': 'shift', '⌥': 'alt' }; const modifiersToMacGlyphs = { + 'ctrl!': '⌃', 'ctrl': '⌘', 'shift': '⇧', 'alt': '⌥' @@ -66,7 +68,8 @@ export function getCode( key ) { keyCode = key.keyCode + ( key.altKey ? keyCodes.alt : 0 ) + ( key.ctrlKey ? keyCodes.ctrl : 0 ) + - ( key.shiftKey ? keyCodes.shift : 0 ); + ( key.shiftKey ? keyCodes.shift : 0 ) + + ( key.metaKey ? keyCodes.cmd : 0 ); } return keyCode; @@ -96,10 +99,35 @@ export function parseKeystroke( keystroke ) { } return keystroke - .map( key => ( typeof key == 'string' ) ? getCode( key ) : key ) + .map( key => ( typeof key == 'string' ) ? getEnvCode( key ) : key ) .reduce( ( key, sum ) => sum + key, 0 ); } +/** + * TODO + * @param key + * @returns {Number} + */ +function getEnvCode( key ) { + if ( typeof key != 'string' ) { + return getCode( key ); + } + + key = key.toLowerCase(); + + if ( key == 'ctrl!' ) { + return keyCodes.ctrl; + } + + const code = getCode( key ); + + if ( env.isMac && code == keyCodes.ctrl ) { + return keyCodes.cmd; + } + + return code; +} + /** * It translates any keystroke string text like `"CTRL+A"` to an * environment–specific keystroke, i.e. `"⌘A"` on Mac OSX. @@ -108,13 +136,15 @@ export function parseKeystroke( keystroke ) { * @returns {String} Keystroke text specific for the environment. */ export function getEnvKeystrokeText( keystroke ) { - if ( !env.isMac ) { - return keystroke; - } - return splitKeystrokeText( keystroke ) // Replace modifiers (e.g. "ctrl") with Mac glyphs (e.g. "⌘") first. - .map( key => modifiersToMacGlyphs[ key.toLowerCase() ] || key ) + .map( key => { + if ( env.isMac ) { + return modifiersToMacGlyphs[ key.toLowerCase() ] || key; + } else { + return key.toLowerCase() == 'ctrl!' ? 'Ctrl' : key; + } + } ) // Decide whether to put "+" between keys in the keystroke or not. .reduce( ( value, key ) => { @@ -203,11 +233,9 @@ function generateKnownKeyCodes() { // The idea about these numbers is that they do not collide with any real key codes, so we can use them // like bit masks. ctrl: 0x110000, - // Has the same code as ctrl, because their behaviour should be unified across the editor. - // See http://ckeditor.github.io/editor-recommendations/general-policies#ctrl-vs-cmd - cmd: 0x110000, shift: 0x220000, - alt: 0x440000 + alt: 0x440000, + cmd: 0x880000 }; // a-z @@ -249,17 +277,17 @@ function splitKeystrokeText( keystroke ) { /** * Whether the Alt modifier was pressed. * - * @member {Bolean} module:utils/keyboard~KeystrokeInfo#altKey + * @member {Boolean} module:utils/keyboard~KeystrokeInfo#altKey */ /** * Whether the Ctrl or Cmd modifier was pressed. * - * @member {Bolean} module:utils/keyboard~KeystrokeInfo#ctrlKey + * @member {Boolean} module:utils/keyboard~KeystrokeInfo#ctrlKey */ /** * Whether the Shift modifier was pressed. * - * @member {Bolean} module:utils/keyboard~KeystrokeInfo#shiftKey + * @member {Boolean} module:utils/keyboard~KeystrokeInfo#shiftKey */ From 70593a065141edd434a17e2dd4f2cfe31dfdff6c Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Fri, 19 Feb 2021 11:04:02 +0100 Subject: [PATCH 02/16] Changed way of handling ctrl!. --- packages/ckeditor5-utils/src/keyboard.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ckeditor5-utils/src/keyboard.js b/packages/ckeditor5-utils/src/keyboard.js index 9b46fb2e2fe..65bd2ecb3d8 100644 --- a/packages/ckeditor5-utils/src/keyboard.js +++ b/packages/ckeditor5-utils/src/keyboard.js @@ -142,7 +142,7 @@ export function getEnvKeystrokeText( keystroke ) { if ( env.isMac ) { return modifiersToMacGlyphs[ key.toLowerCase() ] || key; } else { - return key.toLowerCase() == 'ctrl!' ? 'Ctrl' : key; + return key.endsWith( '!' ) ? key.slice( 0, -1 ) : key; } } ) From 5c2a69aa324e411d57fc6429ce042239bb54063c Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Sat, 20 Feb 2021 17:15:45 +0100 Subject: [PATCH 03/16] Updated keystroke normalization. Added tests. --- packages/ckeditor5-enter/src/enterobserver.js | 2 +- .../ckeditor5-list/src/todolistediting.js | 2 +- packages/ckeditor5-utils/src/keyboard.js | 82 +++++------- packages/ckeditor5-utils/tests/keyboard.js | 119 +++++++++++++----- .../ckeditor5-utils/tests/keystrokehandler.js | 8 ++ 5 files changed, 124 insertions(+), 89 deletions(-) diff --git a/packages/ckeditor5-enter/src/enterobserver.js b/packages/ckeditor5-enter/src/enterobserver.js index c705a141b6c..be907f84242 100644 --- a/packages/ckeditor5-enter/src/enterobserver.js +++ b/packages/ckeditor5-enter/src/enterobserver.js @@ -23,7 +23,7 @@ export default class EnterObserver extends Observer { const doc = this.document; doc.on( 'keydown', ( evt, data ) => { - if ( this.isEnabled && data.keyCode == keyCodes.enter ) { + if ( this.isEnabled && data.keyCode == keyCodes.enter && !data.ctrlKey && !data.metaKey ) { // Save the event object to check later if it was stopped or not. let event; doc.once( 'enter', evt => ( event = evt ), { priority: 'highest' } ); diff --git a/packages/ckeditor5-list/src/todolistediting.js b/packages/ckeditor5-list/src/todolistediting.js index f64c1b5b832..3cff250bc3d 100644 --- a/packages/ckeditor5-list/src/todolistediting.js +++ b/packages/ckeditor5-list/src/todolistediting.js @@ -113,7 +113,7 @@ export default class TodoListEditing extends Plugin { this.listenTo( editing.view.document, 'keydown', jumpOverCheckmarkOnSideArrowKeyPress( model, editor.locale ) ); // Toggle check state of selected to-do list items on keystroke. - editor.keystrokes.set( 'Ctrl!+space', () => editor.execute( 'checkTodoList' ) ); + editor.keystrokes.set( 'Ctrl+Enter', () => editor.execute( 'checkTodoList' ) ); // Remove `todoListChecked` attribute when a host element is no longer a to-do list item. const listItemsToFix = new Set(); diff --git a/packages/ckeditor5-utils/src/keyboard.js b/packages/ckeditor5-utils/src/keyboard.js index 65bd2ecb3d8..dfc85216f6b 100644 --- a/packages/ckeditor5-utils/src/keyboard.js +++ b/packages/ckeditor5-utils/src/keyboard.js @@ -12,18 +12,16 @@ import CKEditorError from './ckeditorerror'; import env from './env'; -const macGlyphsToModifiers = { - '⌃': 'ctrl!', - '⌘': 'ctrl', - '⇧': 'shift', - '⌥': 'alt' +const modifiersToGlyphsMac = { + 'cmd': '⌘', + 'alt': '⌥', + 'shift': '⇧' }; -const modifiersToMacGlyphs = { - 'ctrl!': '⌃', - 'ctrl': '⌘', - 'shift': '⇧', - 'alt': '⌥' +const modifiersToGlyphsNonMac = { + 'ctrl': 'Ctrl+', + 'alt': 'Alt+', + 'shift': 'Shift+' }; /** @@ -40,6 +38,10 @@ const modifiersToMacGlyphs = { */ export const keyCodes = generateKnownKeyCodes(); +const keyCodeNames = Object.fromEntries( + Object.entries( keyCodes ).map( ( [ name, code ] ) => [ code, name.charAt( 0 ).toUpperCase() + name.slice( 1 ) ] ) +); + /** * Converts a key name or a {@link module:utils/keyboard~KeystrokeInfo keystroke info} into a key code. * @@ -99,35 +101,11 @@ export function parseKeystroke( keystroke ) { } return keystroke - .map( key => ( typeof key == 'string' ) ? getEnvCode( key ) : key ) + .map( key => ( typeof key == 'string' ) ? getCode( key ) : key ) + .map( key => env.isMac && key == keyCodes.ctrl ? keyCodes.cmd : key ) .reduce( ( key, sum ) => sum + key, 0 ); } -/** - * TODO - * @param key - * @returns {Number} - */ -function getEnvCode( key ) { - if ( typeof key != 'string' ) { - return getCode( key ); - } - - key = key.toLowerCase(); - - if ( key == 'ctrl!' ) { - return keyCodes.ctrl; - } - - const code = getCode( key ); - - if ( env.isMac && code == keyCodes.ctrl ) { - return keyCodes.cmd; - } - - return code; -} - /** * It translates any keystroke string text like `"CTRL+A"` to an * environment–specific keystroke, i.e. `"⌘A"` on Mac OSX. @@ -136,24 +114,20 @@ function getEnvCode( key ) { * @returns {String} Keystroke text specific for the environment. */ export function getEnvKeystrokeText( keystroke ) { - return splitKeystrokeText( keystroke ) - // Replace modifiers (e.g. "ctrl") with Mac glyphs (e.g. "⌘") first. - .map( key => { - if ( env.isMac ) { - return modifiersToMacGlyphs[ key.toLowerCase() ] || key; - } else { - return key.endsWith( '!' ) ? key.slice( 0, -1 ) : key; - } - } ) - - // Decide whether to put "+" between keys in the keystroke or not. - .reduce( ( value, key ) => { - if ( value.slice( -1 ) in macGlyphsToModifiers ) { - return value + key; - } else { - return value + '+' + key; - } - } ); + let keystrokeCode = parseKeystroke( keystroke ); + + const modifiersToGlyphs = Object.entries( env.isMac ? modifiersToGlyphsMac : modifiersToGlyphsNonMac ); + + const modifiers = modifiersToGlyphs.reduce( ( modifiers, [ name, glyph ] ) => { + if ( ( keystrokeCode & keyCodes[ name ] ) != 0 ) { + keystrokeCode &= ~keyCodes[ name ]; + modifiers += glyph; + } + + return modifiers; + }, '' ); + + return modifiers + ( keystrokeCode ? keyCodeNames[ keystrokeCode ] : '' ); } /** diff --git a/packages/ckeditor5-utils/tests/keyboard.js b/packages/ckeditor5-utils/tests/keyboard.js index d497a167c61..f7c6c7db2f3 100644 --- a/packages/ckeditor5-utils/tests/keyboard.js +++ b/packages/ckeditor5-utils/tests/keyboard.js @@ -30,7 +30,7 @@ describe( 'Keyboard', () => { it( 'modifiers and other keys', () => { expect( keyCodes.delete ).to.equal( 46 ); expect( keyCodes.ctrl ).to.equal( 0x110000 ); - expect( keyCodes.cmd ).to.equal( 0x110000 ); + expect( keyCodes.cmd ).to.equal( 0x880000 ); expect( keyCodes.f1 ).to.equal( 112 ); expect( keyCodes.f12 ).to.equal( 123 ); @@ -73,40 +73,88 @@ describe( 'Keyboard', () => { } ); it( 'adds modifiers to the keystroke code', () => { - expect( getCode( { keyCode: 48, altKey: true, ctrlKey: true, shiftKey: true } ) ) - .to.equal( 48 + 0x110000 + 0x220000 + 0x440000 ); + expect( getCode( { keyCode: 48, altKey: true, ctrlKey: true, shiftKey: true, metaKey: true } ) ) + .to.equal( 48 + 0x110000 + 0x220000 + 0x440000 + 0x880000 ); } ); } ); describe( 'parseKeystroke', () => { - it( 'parses string', () => { - expect( parseKeystroke( 'ctrl+a' ) ).to.equal( 0x110000 + 65 ); - } ); + const initialEnvMac = env.isMac; - it( 'allows spacing', () => { - expect( parseKeystroke( 'ctrl + a' ) ).to.equal( 0x110000 + 65 ); + afterEach( () => { + env.isMac = initialEnvMac; } ); - it( 'is case-insensitive', () => { - expect( parseKeystroke( 'Ctrl+A' ) ).to.equal( 0x110000 + 65 ); - } ); + describe( 'on Macintosh', () => { + beforeEach( () => { + env.isMac = true; + } ); - it( 'works with an array', () => { - expect( parseKeystroke( [ 'ctrl', 'a' ] ) ).to.equal( 0x110000 + 65 ); - } ); + it( 'parses string', () => { + expect( parseKeystroke( 'ctrl+a' ) ).to.equal( 0x880000 + 65 ); + } ); - it( 'works with an array which contains numbers', () => { - expect( parseKeystroke( [ 'shift', 33 ] ) ).to.equal( 0x220000 + 33 ); - } ); + it( 'allows spacing', () => { + expect( parseKeystroke( 'ctrl + a' ) ).to.equal( 0x880000 + 65 ); + } ); + + it( 'is case-insensitive', () => { + expect( parseKeystroke( 'Ctrl+A' ) ).to.equal( 0x880000 + 65 ); + } ); + + it( 'works with an array', () => { + expect( parseKeystroke( [ 'ctrl', 'a' ] ) ).to.equal( 0x880000 + 65 ); + } ); + + it( 'works with an array which contains numbers', () => { + expect( parseKeystroke( [ 'shift', 33 ] ) ).to.equal( 0x220000 + 33 ); + } ); + + it( 'works with two modifiers', () => { + expect( parseKeystroke( 'ctrl+shift+a' ) ).to.equal( 0x880000 + 0x220000 + 65 ); + } ); - it( 'works with two modifiers', () => { - expect( parseKeystroke( 'ctrl+shift+a' ) ).to.equal( 0x110000 + 0x220000 + 65 ); + it( 'throws on unknown name', () => { + expectToThrowCKEditorError( () => { + parseKeystroke( 'foo' ); + }, 'keyboard-unknown-key', null ); + } ); } ); - it( 'throws on unknown name', () => { - expectToThrowCKEditorError( () => { - parseKeystroke( 'foo' ); - }, 'keyboard-unknown-key', null ); + describe( 'on non–Macintosh', () => { + beforeEach( () => { + env.isMac = false; + } ); + + it( 'parses string', () => { + expect( parseKeystroke( 'ctrl+a' ) ).to.equal( 0x110000 + 65 ); + } ); + + it( 'allows spacing', () => { + expect( parseKeystroke( 'ctrl + a' ) ).to.equal( 0x110000 + 65 ); + } ); + + it( 'is case-insensitive', () => { + expect( parseKeystroke( 'Ctrl+A' ) ).to.equal( 0x110000 + 65 ); + } ); + + it( 'works with an array', () => { + expect( parseKeystroke( [ 'ctrl', 'a' ] ) ).to.equal( 0x110000 + 65 ); + } ); + + it( 'works with an array which contains numbers', () => { + expect( parseKeystroke( [ 'shift', 33 ] ) ).to.equal( 0x220000 + 33 ); + } ); + + it( 'works with two modifiers', () => { + expect( parseKeystroke( 'ctrl+shift+a' ) ).to.equal( 0x110000 + 0x220000 + 65 ); + } ); + + it( 'throws on unknown name', () => { + expectToThrowCKEditorError( () => { + parseKeystroke( 'foo' ); + }, 'keyboard-unknown-key', null ); + } ); } ); } ); @@ -145,11 +193,13 @@ describe( 'Keyboard', () => { expect( getEnvKeystrokeText( 'ALT+SHIFT+X' ) ).to.equal( '⌥⇧X' ); } ); - it( 'does not touch other keys', () => { - expect( getEnvKeystrokeText( 'ESC+A' ) ).to.equal( 'ESC+A' ); - expect( getEnvKeystrokeText( 'TAB' ) ).to.equal( 'TAB' ); + it( 'normalizes value', () => { + expect( getEnvKeystrokeText( 'ESC' ) ).to.equal( 'Esc' ); + expect( getEnvKeystrokeText( 'TAB' ) ).to.equal( 'Tab' ); expect( getEnvKeystrokeText( 'A' ) ).to.equal( 'A' ); - expect( getEnvKeystrokeText( 'A+CTRL+B' ) ).to.equal( 'A+⌘B' ); + expect( getEnvKeystrokeText( 'a' ) ).to.equal( 'A' ); + expect( getEnvKeystrokeText( 'CTRL+a' ) ).to.equal( '⌘A' ); + expect( getEnvKeystrokeText( 'ctrl+b' ) ).to.equal( '⌘B' ); } ); } ); @@ -158,13 +208,16 @@ describe( 'Keyboard', () => { env.isMac = false; } ); - it( 'does not touch anything', () => { - expect( getEnvKeystrokeText( 'CTRL+A' ) ).to.equal( 'CTRL+A' ); - expect( getEnvKeystrokeText( 'ctrl+A' ) ).to.equal( 'ctrl+A' ); - expect( getEnvKeystrokeText( 'SHIFT+A' ) ).to.equal( 'SHIFT+A' ); - expect( getEnvKeystrokeText( 'alt+A' ) ).to.equal( 'alt+A' ); - expect( getEnvKeystrokeText( 'CTRL+SHIFT+A' ) ).to.equal( 'CTRL+SHIFT+A' ); + it( 'normalizes value', () => { + expect( getEnvKeystrokeText( 'ESC' ) ).to.equal( 'Esc' ); + expect( getEnvKeystrokeText( 'TAB' ) ).to.equal( 'Tab' ); expect( getEnvKeystrokeText( 'A' ) ).to.equal( 'A' ); + expect( getEnvKeystrokeText( 'a' ) ).to.equal( 'A' ); + expect( getEnvKeystrokeText( 'CTRL+a' ) ).to.equal( 'Ctrl+A' ); + expect( getEnvKeystrokeText( 'ctrl+b' ) ).to.equal( 'Ctrl+B' ); + expect( getEnvKeystrokeText( 'SHIFT+A' ) ).to.equal( 'Shift+A' ); + expect( getEnvKeystrokeText( 'alt+A' ) ).to.equal( 'Alt+A' ); + expect( getEnvKeystrokeText( 'CTRL+SHIFT+A' ) ).to.equal( 'Ctrl+Shift+A' ); } ); } ); } ); diff --git a/packages/ckeditor5-utils/tests/keystrokehandler.js b/packages/ckeditor5-utils/tests/keystrokehandler.js index 45bb5613043..7f77abfbcd9 100644 --- a/packages/ckeditor5-utils/tests/keystrokehandler.js +++ b/packages/ckeditor5-utils/tests/keystrokehandler.js @@ -6,17 +6,25 @@ import EmitterMixin from '../src/emittermixin'; import KeystrokeHandler from '../src/keystrokehandler'; import { keyCodes } from '../src/keyboard'; +import env from '../src/env'; describe( 'KeystrokeHandler', () => { + const initialEnvMac = env.isMac; let emitter, keystrokes; beforeEach( () => { + env.isMac = false; + emitter = Object.create( EmitterMixin ); keystrokes = new KeystrokeHandler(); keystrokes.listenTo( emitter ); } ); + afterEach( () => { + env.isMac = initialEnvMac; + } ); + describe( 'listenTo()', () => { it( 'activates the listening on the emitter', () => { const spy = sinon.spy(); From 8b9aa75afc277a2cf757c7ebe34c1ba6fed4f58a Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Sat, 20 Feb 2021 17:38:06 +0100 Subject: [PATCH 04/16] Added tests. --- .../tests/view/observer/keyobserver.js | 5 ++-- .../ckeditor5-enter/tests/enterobserver.js | 28 ++++++++++++++++++- .../docs/features/todo-lists.md | 2 +- .../ckeditor5-list/tests/todolistediting.js | 11 ++++++-- .../utils/injectunsafekeystrokeshandling.js | 5 ++++ 5 files changed, 45 insertions(+), 6 deletions(-) diff --git a/packages/ckeditor5-engine/tests/view/observer/keyobserver.js b/packages/ckeditor5-engine/tests/view/observer/keyobserver.js index 89345a84a86..7882b949fd5 100644 --- a/packages/ckeditor5-engine/tests/view/observer/keyobserver.js +++ b/packages/ckeditor5-engine/tests/view/observer/keyobserver.js @@ -84,7 +84,7 @@ describe( 'KeyObserver', () => { expect( getCode( data ) ).to.be.greaterThan( 111 ); } ); - it( 'should fire keydown ctrlKey set to true one meta (cmd) was pressed', () => { + it( 'should fire keydown metaKey set to true once meta (cmd) was pressed', () => { const spy = sinon.spy(); viewDocument.on( 'keydown', spy ); @@ -92,7 +92,8 @@ describe( 'KeyObserver', () => { observer.onDomEvent( { type: 'keydown', target: document.body, keyCode: 111, metaKey: true } ); const data = spy.args[ 0 ][ 1 ]; - expect( data ).to.have.property( 'ctrlKey', true ); + expect( data ).to.have.property( 'metaKey', true ); + expect( data ).to.have.property( 'ctrlKey', false ); } ); it( 'should fire keyup with the target and key info', () => { diff --git a/packages/ckeditor5-enter/tests/enterobserver.js b/packages/ckeditor5-enter/tests/enterobserver.js index b123c0e71cf..5e8cb24769d 100644 --- a/packages/ckeditor5-enter/tests/enterobserver.js +++ b/packages/ckeditor5-enter/tests/enterobserver.js @@ -57,6 +57,32 @@ describe( 'EnterObserver', () => { expect( spy.firstCall.args[ 1 ].isSoft ).to.be.true; } ); + it( 'is not fired on keydown when ctrl key was pressed along with the "enter" key', () => { + const spy = sinon.spy(); + + viewDocument.on( 'enter', spy ); + + viewDocument.fire( 'keydown', new DomEventData( viewDocument, getDomEvent(), { + keyCode: getCode( 'enter' ), + ctrlKey: true + } ) ); + + expect( spy.notCalled ).to.be.true; + } ); + + it( 'is not fired on keydown when meta key was pressed along with the "enter" key', () => { + const spy = sinon.spy(); + + viewDocument.on( 'enter', spy ); + + viewDocument.fire( 'keydown', new DomEventData( viewDocument, getDomEvent(), { + keyCode: getCode( 'enter' ), + metaKey: true + } ) ); + + expect( spy.notCalled ).to.be.true; + } ); + it( 'is not fired on keydown when keyCode does not match enter', () => { const spy = sinon.spy(); @@ -66,7 +92,7 @@ describe( 'EnterObserver', () => { keyCode: 1 } ) ); - expect( spy.calledOnce ).to.be.false; + expect( spy.notCalled ).to.be.true; } ); it( 'should stop keydown event when enter event is stopped', () => { diff --git a/packages/ckeditor5-list/docs/features/todo-lists.md b/packages/ckeditor5-list/docs/features/todo-lists.md index 6a7cceae5ce..ad27a15170b 100644 --- a/packages/ckeditor5-list/docs/features/todo-lists.md +++ b/packages/ckeditor5-list/docs/features/todo-lists.md @@ -16,7 +16,7 @@ To-do lists can be introduced using the dedicated toolbar button. Thanks to the ## Keyboard support -You can check and uncheck a list item by using the Ctrl + Space shortcut when the selection is in that item. +You can check and uncheck a list item by using the Ctrl + Enter shortcut when the selection is in that item. ## Installation diff --git a/packages/ckeditor5-list/tests/todolistediting.js b/packages/ckeditor5-list/tests/todolistediting.js index e56df30fa23..37c0791d5e1 100644 --- a/packages/ckeditor5-list/tests/todolistediting.js +++ b/packages/ckeditor5-list/tests/todolistediting.js @@ -23,13 +23,20 @@ import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils import { getCode } from '@ckeditor/ckeditor5-utils/src/keyboard'; import { assertEqualMarkup } from '@ckeditor/ckeditor5-utils/tests/_utils/utils'; import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; +import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; +import { env } from '@ckeditor/ckeditor5-utils'; /* global Event, document */ describe( 'TodoListEditing', () => { let editor, model, modelDoc, modelRoot, view, viewDoc; + testUtils.createSinonSandbox(); + beforeEach( () => { + // For the KeystrokeHandler. + sinon.stub( env, 'isMac' ).value( false ); + return VirtualTestEditor .create( { plugins: [ Paragraph, TodoListEditing, Typing, BoldEditing, BlockQuoteEditing, LinkEditing, Enter, ShiftEnter ] @@ -1157,12 +1164,12 @@ describe( 'TodoListEditing', () => { } } ); - describe( 'Ctrl+space keystroke handling', () => { + describe( 'Ctrl+enter keystroke handling', () => { let domEvtDataStub; beforeEach( () => { domEvtDataStub = { - keyCode: getCode( 'space' ), + keyCode: getCode( 'enter' ), ctrlKey: true, preventDefault: sinon.spy(), stopPropagation: sinon.spy() diff --git a/packages/ckeditor5-typing/tests/utils/injectunsafekeystrokeshandling.js b/packages/ckeditor5-typing/tests/utils/injectunsafekeystrokeshandling.js index 01d5c8dcd10..849d1e65c43 100644 --- a/packages/ckeditor5-typing/tests/utils/injectunsafekeystrokeshandling.js +++ b/packages/ckeditor5-typing/tests/utils/injectunsafekeystrokeshandling.js @@ -17,6 +17,11 @@ describe( 'unsafe keystroke handling utils', () => { expect( isNonTypingKeystroke( { keyCode: keyCodes[ 0 ], ctrlKey: true } ), 'Ctrl+0' ).to.be.true; } ); + it( 'should return "true" for any keystroke with the Cmd key', () => { + expect( isNonTypingKeystroke( { keyCode: keyCodes.a, metaKey: true } ), 'Ctrl+a' ).to.be.true; + expect( isNonTypingKeystroke( { keyCode: keyCodes[ 0 ], metaKey: true } ), 'Ctrl+0' ).to.be.true; + } ); + it( 'should return "true" for all arrow keys', () => { expect( isNonTypingKeystroke( { keyCode: keyCodes.arrowup } ), 'arrow up' ).to.be.true; expect( isNonTypingKeystroke( { keyCode: keyCodes.arrowdown } ), 'arrow down' ).to.be.true; From feaa295790dcf9df13e2d404fbfb8587cb35c263 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Sat, 20 Feb 2021 18:12:33 +0100 Subject: [PATCH 05/16] Fixed test. --- packages/ckeditor5-engine/tests/view/observer/keyobserver.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/ckeditor5-engine/tests/view/observer/keyobserver.js b/packages/ckeditor5-engine/tests/view/observer/keyobserver.js index 7882b949fd5..77c579ef0fa 100644 --- a/packages/ckeditor5-engine/tests/view/observer/keyobserver.js +++ b/packages/ckeditor5-engine/tests/view/observer/keyobserver.js @@ -93,7 +93,6 @@ describe( 'KeyObserver', () => { const data = spy.args[ 0 ][ 1 ]; expect( data ).to.have.property( 'metaKey', true ); - expect( data ).to.have.property( 'ctrlKey', false ); } ); it( 'should fire keyup with the target and key info', () => { From f1ea04f74aaeb0b4cbf8e3a3c3b6d78b427b7d51 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Mon, 22 Feb 2021 12:57:57 +0100 Subject: [PATCH 06/16] Added code comment. --- packages/ckeditor5-utils/src/keyboard.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/ckeditor5-utils/src/keyboard.js b/packages/ckeditor5-utils/src/keyboard.js index dfc85216f6b..50289235ab4 100644 --- a/packages/ckeditor5-utils/src/keyboard.js +++ b/packages/ckeditor5-utils/src/keyboard.js @@ -119,6 +119,7 @@ export function getEnvKeystrokeText( keystroke ) { const modifiersToGlyphs = Object.entries( env.isMac ? modifiersToGlyphsMac : modifiersToGlyphsNonMac ); const modifiers = modifiersToGlyphs.reduce( ( modifiers, [ name, glyph ] ) => { + // Modifier keys are stored as a bit mask so extract those from the keystroke code. if ( ( keystrokeCode & keyCodes[ name ] ) != 0 ) { keystrokeCode &= ~keyCodes[ name ]; modifiers += glyph; From 7453d25a1a761f8f465c9a196a29a9d037c72fff Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Mon, 22 Feb 2021 16:29:09 +0100 Subject: [PATCH 07/16] Reverted changes in EnterObserver. --- packages/ckeditor5-enter/src/enterobserver.js | 2 +- .../ckeditor5-enter/tests/enterobserver.js | 26 ------------------- 2 files changed, 1 insertion(+), 27 deletions(-) diff --git a/packages/ckeditor5-enter/src/enterobserver.js b/packages/ckeditor5-enter/src/enterobserver.js index be907f84242..c705a141b6c 100644 --- a/packages/ckeditor5-enter/src/enterobserver.js +++ b/packages/ckeditor5-enter/src/enterobserver.js @@ -23,7 +23,7 @@ export default class EnterObserver extends Observer { const doc = this.document; doc.on( 'keydown', ( evt, data ) => { - if ( this.isEnabled && data.keyCode == keyCodes.enter && !data.ctrlKey && !data.metaKey ) { + if ( this.isEnabled && data.keyCode == keyCodes.enter ) { // Save the event object to check later if it was stopped or not. let event; doc.once( 'enter', evt => ( event = evt ), { priority: 'highest' } ); diff --git a/packages/ckeditor5-enter/tests/enterobserver.js b/packages/ckeditor5-enter/tests/enterobserver.js index 5e8cb24769d..3eab6171e4a 100644 --- a/packages/ckeditor5-enter/tests/enterobserver.js +++ b/packages/ckeditor5-enter/tests/enterobserver.js @@ -57,32 +57,6 @@ describe( 'EnterObserver', () => { expect( spy.firstCall.args[ 1 ].isSoft ).to.be.true; } ); - it( 'is not fired on keydown when ctrl key was pressed along with the "enter" key', () => { - const spy = sinon.spy(); - - viewDocument.on( 'enter', spy ); - - viewDocument.fire( 'keydown', new DomEventData( viewDocument, getDomEvent(), { - keyCode: getCode( 'enter' ), - ctrlKey: true - } ) ); - - expect( spy.notCalled ).to.be.true; - } ); - - it( 'is not fired on keydown when meta key was pressed along with the "enter" key', () => { - const spy = sinon.spy(); - - viewDocument.on( 'enter', spy ); - - viewDocument.fire( 'keydown', new DomEventData( viewDocument, getDomEvent(), { - keyCode: getCode( 'enter' ), - metaKey: true - } ) ); - - expect( spy.notCalled ).to.be.true; - } ); - it( 'is not fired on keydown when keyCode does not match enter', () => { const spy = sinon.spy(); From 74a20ae8cd645e675cfee7b63af0628816d1ac8a Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Mon, 22 Feb 2021 16:32:01 +0100 Subject: [PATCH 08/16] Updated to-do list to use 'keydown' event instead of KeystrokeHandler. --- .../ckeditor5-list/src/todolistediting.js | 9 ++- .../ckeditor5-list/tests/todolistediting.js | 64 +++++++++++++++++-- 2 files changed, 64 insertions(+), 9 deletions(-) diff --git a/packages/ckeditor5-list/src/todolistediting.js b/packages/ckeditor5-list/src/todolistediting.js index 3cff250bc3d..6b9f5a94c2b 100644 --- a/packages/ckeditor5-list/src/todolistediting.js +++ b/packages/ckeditor5-list/src/todolistediting.js @@ -8,7 +8,7 @@ */ import { Plugin } from 'ckeditor5/src/core'; -import { getLocalizedArrowKeyCodeDirection } from 'ckeditor5/src/utils'; +import { env, keyCodes, getLocalizedArrowKeyCodeDirection } from 'ckeditor5/src/utils'; import ListCommand from './listcommand'; import ListEditing from './listediting'; @@ -113,7 +113,12 @@ export default class TodoListEditing extends Plugin { this.listenTo( editing.view.document, 'keydown', jumpOverCheckmarkOnSideArrowKeyPress( model, editor.locale ) ); // Toggle check state of selected to-do list items on keystroke. - editor.keystrokes.set( 'Ctrl+Enter', () => editor.execute( 'checkTodoList' ) ); + this.listenTo( editing.view.document, 'keydown', ( evt, data ) => { + if ( data.keyCode == keyCodes.enter && ( env.isMac ? data.metaKey : data.ctrlKey ) ) { + editor.execute( 'checkTodoList' ); + evt.stop(); + } + }, { priority: 'high' } ); // Remove `todoListChecked` attribute when a host element is no longer a to-do list item. const listItemsToFix = new Set(); diff --git a/packages/ckeditor5-list/tests/todolistediting.js b/packages/ckeditor5-list/tests/todolistediting.js index 37c0791d5e1..a8942809633 100644 --- a/packages/ckeditor5-list/tests/todolistediting.js +++ b/packages/ckeditor5-list/tests/todolistediting.js @@ -34,9 +34,6 @@ describe( 'TodoListEditing', () => { testUtils.createSinonSandbox(); beforeEach( () => { - // For the KeystrokeHandler. - sinon.stub( env, 'isMac' ).value( false ); - return VirtualTestEditor .create( { plugins: [ Paragraph, TodoListEditing, Typing, BoldEditing, BlockQuoteEditing, LinkEditing, Enter, ShiftEnter ] @@ -1161,30 +1158,83 @@ describe( 'TodoListEditing', () => { sinon.assert.notCalled( domEvtDataStub.preventDefault ); sinon.assert.notCalled( domEvtDataStub.stopPropagation ); } ); + + it( 'should do nothing when other arrow key was pressed', () => { + setModelData( model, '[]bar' ); + + domEvtDataStub = { + keyCode: getCode( 'arrowDown' ), + preventDefault: sinon.spy(), + stopPropagation: sinon.spy(), + domTarget: { + ownerDocument: { + defaultView: { + getSelection: () => ( { rangeCount: 0 } ) + } + } + } + }; + + viewDoc.fire( 'keydown', domEvtDataStub ); + + sinon.assert.notCalled( domEvtDataStub.preventDefault ); + sinon.assert.notCalled( domEvtDataStub.stopPropagation ); + } ); } } ); describe( 'Ctrl+enter keystroke handling', () => { - let domEvtDataStub; + const initialEnvMac = env.isMac; + + afterEach( () => { + env.isMac = initialEnvMac; + } ); + + it( 'should execute CheckTodoListCommand (non-macOS)', () => { + env.isMac = false; + + const command = editor.commands.get( 'checkTodoList' ); - beforeEach( () => { - domEvtDataStub = { + sinon.spy( command, 'execute' ); + + const domEvtDataStub = { keyCode: getCode( 'enter' ), ctrlKey: true, preventDefault: sinon.spy(), stopPropagation: sinon.spy() }; + + // First call. + viewDoc.fire( 'keydown', domEvtDataStub ); + + sinon.assert.calledOnce( command.execute ); + + // Second call. + viewDoc.fire( 'keydown', domEvtDataStub ); + + sinon.assert.calledTwice( command.execute ); } ); - it( 'should execute CheckTodoListCommand', () => { + it( 'should execute CheckTodoListCommand (macOS)', () => { + env.isMac = true; + const command = editor.commands.get( 'checkTodoList' ); sinon.spy( command, 'execute' ); + const domEvtDataStub = { + keyCode: getCode( 'enter' ), + metaKey: true, + preventDefault: sinon.spy(), + stopPropagation: sinon.spy() + }; + + // First call. viewDoc.fire( 'keydown', domEvtDataStub ); sinon.assert.calledOnce( command.execute ); + // Second call. viewDoc.fire( 'keydown', domEvtDataStub ); sinon.assert.calledTwice( command.execute ); From c75c053df426a9323020fca195fd93b907e3f9ef Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Mon, 22 Feb 2021 16:33:25 +0100 Subject: [PATCH 09/16] Reverted changes in EnterObserver tests. --- packages/ckeditor5-enter/tests/enterobserver.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ckeditor5-enter/tests/enterobserver.js b/packages/ckeditor5-enter/tests/enterobserver.js index 3eab6171e4a..b123c0e71cf 100644 --- a/packages/ckeditor5-enter/tests/enterobserver.js +++ b/packages/ckeditor5-enter/tests/enterobserver.js @@ -66,7 +66,7 @@ describe( 'EnterObserver', () => { keyCode: 1 } ) ); - expect( spy.notCalled ).to.be.true; + expect( spy.calledOnce ).to.be.false; } ); it( 'should stop keydown event when enter event is stopped', () => { From 74ab1a495bb5c16e8abd17464401600e05f4d467 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Mon, 22 Feb 2021 17:38:11 +0100 Subject: [PATCH 10/16] Added support for forced keystroke modifiers. --- packages/ckeditor5-utils/src/keyboard.js | 21 +++++++++++++++++++-- packages/ckeditor5-utils/tests/keyboard.js | 16 ++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/packages/ckeditor5-utils/src/keyboard.js b/packages/ckeditor5-utils/src/keyboard.js index 50289235ab4..44a7351d02a 100644 --- a/packages/ckeditor5-utils/src/keyboard.js +++ b/packages/ckeditor5-utils/src/keyboard.js @@ -13,6 +13,7 @@ import CKEditorError from './ckeditorerror'; import env from './env'; const modifiersToGlyphsMac = { + 'ctrl': '⌃', 'cmd': '⌘', 'alt': '⌥', 'shift': '⇧' @@ -101,8 +102,7 @@ export function parseKeystroke( keystroke ) { } return keystroke - .map( key => ( typeof key == 'string' ) ? getCode( key ) : key ) - .map( key => env.isMac && key == keyCodes.ctrl ? keyCodes.cmd : key ) + .map( key => ( typeof key == 'string' ) ? getEnvKeyCode( key ) : key ) .reduce( ( key, sum ) => sum + key, 0 ); } @@ -174,6 +174,23 @@ export function getLocalizedArrowKeyCodeDirection( keyCode, contentLanguageDirec } } +// Converts a key name to the key code with mapping based on the env. +// +// See: {@link module:utils/keyboard~getCode}. +// +// @param {String} key The key name (see {@link module:utils/keyboard~keyCodes}). +// @returns {Number} Key code. +function getEnvKeyCode( key ) { + // Don't remap modifier key for forced modifiers. + if ( key.endsWith( '!' ) ) { + return getCode( key.slice( 0, -1 ) ); + } + + const code = getCode( key ); + + return env.isMac && code == keyCodes.ctrl ? keyCodes.cmd : code; +} + /** * Determines if the provided key code moves the {@link module:engine/model/documentselection~DocumentSelection selection} * forward or backward considering the language direction of the editor content. diff --git a/packages/ckeditor5-utils/tests/keyboard.js b/packages/ckeditor5-utils/tests/keyboard.js index f7c6c7db2f3..e7114906938 100644 --- a/packages/ckeditor5-utils/tests/keyboard.js +++ b/packages/ckeditor5-utils/tests/keyboard.js @@ -114,6 +114,10 @@ describe( 'Keyboard', () => { expect( parseKeystroke( 'ctrl+shift+a' ) ).to.equal( 0x880000 + 0x220000 + 65 ); } ); + it( 'supports forced modifier', () => { + expect( parseKeystroke( 'ctrl!+a' ) ).to.equal( 0x110000 + 65 ); + } ); + it( 'throws on unknown name', () => { expectToThrowCKEditorError( () => { parseKeystroke( 'foo' ); @@ -150,6 +154,10 @@ describe( 'Keyboard', () => { expect( parseKeystroke( 'ctrl+shift+a' ) ).to.equal( 0x110000 + 0x220000 + 65 ); } ); + it( 'supports forced modifier', () => { + expect( parseKeystroke( 'ctrl!+a' ) ).to.equal( 0x110000 + 65 ); + } ); + it( 'throws on unknown name', () => { expectToThrowCKEditorError( () => { parseKeystroke( 'foo' ); @@ -176,6 +184,12 @@ describe( 'Keyboard', () => { expect( getEnvKeystrokeText( 'ctrl+A' ) ).to.equal( '⌘A' ); } ); + it( 'replaces CTRL! with ⌃', () => { + expect( getEnvKeystrokeText( 'CTRL!' ) ).to.equal( '⌃' ); + expect( getEnvKeystrokeText( 'CTRL!+A' ) ).to.equal( '⌃A' ); + expect( getEnvKeystrokeText( 'ctrl!+A' ) ).to.equal( '⌃A' ); + } ); + it( 'replaces SHIFT with ⇧', () => { expect( getEnvKeystrokeText( 'SHIFT' ) ).to.equal( '⇧' ); expect( getEnvKeystrokeText( 'SHIFT+A' ) ).to.equal( '⇧A' ); @@ -214,7 +228,9 @@ describe( 'Keyboard', () => { expect( getEnvKeystrokeText( 'A' ) ).to.equal( 'A' ); expect( getEnvKeystrokeText( 'a' ) ).to.equal( 'A' ); expect( getEnvKeystrokeText( 'CTRL+a' ) ).to.equal( 'Ctrl+A' ); + expect( getEnvKeystrokeText( 'CTRL!+a' ) ).to.equal( 'Ctrl+A' ); expect( getEnvKeystrokeText( 'ctrl+b' ) ).to.equal( 'Ctrl+B' ); + expect( getEnvKeystrokeText( 'ctrl!+b' ) ).to.equal( 'Ctrl+B' ); expect( getEnvKeystrokeText( 'SHIFT+A' ) ).to.equal( 'Shift+A' ); expect( getEnvKeystrokeText( 'alt+A' ) ).to.equal( 'Alt+A' ); expect( getEnvKeystrokeText( 'CTRL+SHIFT+A' ) ).to.equal( 'Ctrl+Shift+A' ); From a50dbfcf06dffbde8dabd2bb3db96c61ef59d69b Mon Sep 17 00:00:00 2001 From: Kuba Niegowski <1232187+niegowski@users.noreply.github.com> Date: Tue, 23 Feb 2021 12:10:25 +0100 Subject: [PATCH 11/16] Fixed test comment. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Maksymilian Barnaś --- .../tests/utils/injectunsafekeystrokeshandling.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/ckeditor5-typing/tests/utils/injectunsafekeystrokeshandling.js b/packages/ckeditor5-typing/tests/utils/injectunsafekeystrokeshandling.js index 849d1e65c43..4852162abd6 100644 --- a/packages/ckeditor5-typing/tests/utils/injectunsafekeystrokeshandling.js +++ b/packages/ckeditor5-typing/tests/utils/injectunsafekeystrokeshandling.js @@ -18,8 +18,8 @@ describe( 'unsafe keystroke handling utils', () => { } ); it( 'should return "true" for any keystroke with the Cmd key', () => { - expect( isNonTypingKeystroke( { keyCode: keyCodes.a, metaKey: true } ), 'Ctrl+a' ).to.be.true; - expect( isNonTypingKeystroke( { keyCode: keyCodes[ 0 ], metaKey: true } ), 'Ctrl+0' ).to.be.true; + expect( isNonTypingKeystroke( { keyCode: keyCodes.a, metaKey: true } ), '⌘a' ).to.be.true; + expect( isNonTypingKeystroke( { keyCode: keyCodes[ 0 ], metaKey: true } ), '⌘0' ).to.be.true; } ); it( 'should return "true" for all arrow keys', () => { From 7d86b0c90a2f9a603cfc32ca61c2780af53c3c5d Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 23 Feb 2021 14:19:42 +0100 Subject: [PATCH 12/16] Docs. --- packages/ckeditor5-utils/src/keyboard.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/ckeditor5-utils/src/keyboard.js b/packages/ckeditor5-utils/src/keyboard.js index 44a7351d02a..ba50995e906 100644 --- a/packages/ckeditor5-utils/src/keyboard.js +++ b/packages/ckeditor5-utils/src/keyboard.js @@ -273,7 +273,7 @@ function splitKeystrokeText( keystroke ) { */ /** - * Whether the Ctrl or Cmd modifier was pressed. + * Whether the Ctrl modifier was pressed. * * @member {Boolean} module:utils/keyboard~KeystrokeInfo#ctrlKey */ @@ -283,3 +283,9 @@ function splitKeystrokeText( keystroke ) { * * @member {Boolean} module:utils/keyboard~KeystrokeInfo#shiftKey */ + +/** + * Whether the Cmd modifier was pressed. + * + * @member {Boolean} module:utils/keyboard~KeystrokeInfo#metaKey + */ From 0daff3141aa91c532e2038dd770761ef3bb5f20c Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 23 Feb 2021 14:25:23 +0100 Subject: [PATCH 13/16] Tests: Added an extra test for ctrlKey property in the KeystrokeInfo data. --- .../tests/view/observer/keyobserver.js | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/packages/ckeditor5-engine/tests/view/observer/keyobserver.js b/packages/ckeditor5-engine/tests/view/observer/keyobserver.js index 77c579ef0fa..8c3923f4d1f 100644 --- a/packages/ckeditor5-engine/tests/view/observer/keyobserver.js +++ b/packages/ckeditor5-engine/tests/view/observer/keyobserver.js @@ -84,7 +84,18 @@ describe( 'KeyObserver', () => { expect( getCode( data ) ).to.be.greaterThan( 111 ); } ); - it( 'should fire keydown metaKey set to true once meta (cmd) was pressed', () => { + it( 'should fire keydown with ctrlKey set to true once ctrl was pressed', () => { + const spy = sinon.spy(); + + viewDocument.on( 'keydown', spy ); + + observer.onDomEvent( { type: 'keydown', target: document.body, keyCode: 111, ctrlKey: true } ); + + const data = spy.args[ 0 ][ 1 ]; + expect( data ).to.have.property( 'ctrlKey', true ); + } ); + + it( 'should fire keydown with metaKey set to true once meta (cmd) was pressed', () => { const spy = sinon.spy(); viewDocument.on( 'keydown', spy ); From 9c0574e7a6fba6293140c47d8b92e8ae76936674 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 23 Feb 2021 14:27:13 +0100 Subject: [PATCH 14/16] Docs: Updated the todo lists guide with the item toggle keystroke on Mac. --- packages/ckeditor5-list/docs/features/todo-lists.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ckeditor5-list/docs/features/todo-lists.md b/packages/ckeditor5-list/docs/features/todo-lists.md index ad27a15170b..92a0b45c499 100644 --- a/packages/ckeditor5-list/docs/features/todo-lists.md +++ b/packages/ckeditor5-list/docs/features/todo-lists.md @@ -16,7 +16,7 @@ To-do lists can be introduced using the dedicated toolbar button. Thanks to the ## Keyboard support -You can check and uncheck a list item by using the Ctrl + Enter shortcut when the selection is in that item. +You can check and uncheck a list item by using the Ctrl + Enter (⌘ + Enter on Mac) shortcut when the selection is in that item. ## Installation From 61ce0b114c7fb69bcbe063e586ddabc656ce2260 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 23 Feb 2021 14:42:56 +0100 Subject: [PATCH 15/16] Simplified the list item toggle keystroke discovery in the TodoListEditing. --- .../ckeditor5-list/src/todolistediting.js | 10 ++++- .../ckeditor5-list/tests/todolistediting.js | 40 +++---------------- 2 files changed, 14 insertions(+), 36 deletions(-) diff --git a/packages/ckeditor5-list/src/todolistediting.js b/packages/ckeditor5-list/src/todolistediting.js index 6b9f5a94c2b..a0a6d568cf8 100644 --- a/packages/ckeditor5-list/src/todolistediting.js +++ b/packages/ckeditor5-list/src/todolistediting.js @@ -8,7 +8,11 @@ */ import { Plugin } from 'ckeditor5/src/core'; -import { env, keyCodes, getLocalizedArrowKeyCodeDirection } from 'ckeditor5/src/utils'; +import { + getCode, + parseKeystroke, + getLocalizedArrowKeyCodeDirection +} from 'ckeditor5/src/utils'; import ListCommand from './listcommand'; import ListEditing from './listediting'; @@ -22,6 +26,8 @@ import { modelViewInsertion } from './todolistconverters'; +const ITEM_TOGGLE_KEYSTROKE = parseKeystroke( 'Ctrl+Enter' ); + /** * The engine of the to-do list feature. It handles creating, editing and removing to-do lists and their items. * @@ -114,7 +120,7 @@ export default class TodoListEditing extends Plugin { // Toggle check state of selected to-do list items on keystroke. this.listenTo( editing.view.document, 'keydown', ( evt, data ) => { - if ( data.keyCode == keyCodes.enter && ( env.isMac ? data.metaKey : data.ctrlKey ) ) { + if ( getCode( data ) === ITEM_TOGGLE_KEYSTROKE ) { editor.execute( 'checkTodoList' ); evt.stop(); } diff --git a/packages/ckeditor5-list/tests/todolistediting.js b/packages/ckeditor5-list/tests/todolistediting.js index a8942809633..0147faf5396 100644 --- a/packages/ckeditor5-list/tests/todolistediting.js +++ b/packages/ckeditor5-list/tests/todolistediting.js @@ -1184,50 +1184,22 @@ describe( 'TodoListEditing', () => { } ); describe( 'Ctrl+enter keystroke handling', () => { - const initialEnvMac = env.isMac; - - afterEach( () => { - env.isMac = initialEnvMac; - } ); - - it( 'should execute CheckTodoListCommand (non-macOS)', () => { - env.isMac = false; - + it( 'should execute CheckTodoListCommand', () => { const command = editor.commands.get( 'checkTodoList' ); sinon.spy( command, 'execute' ); const domEvtDataStub = { keyCode: getCode( 'enter' ), - ctrlKey: true, preventDefault: sinon.spy(), stopPropagation: sinon.spy() }; - // First call. - viewDoc.fire( 'keydown', domEvtDataStub ); - - sinon.assert.calledOnce( command.execute ); - - // Second call. - viewDoc.fire( 'keydown', domEvtDataStub ); - - sinon.assert.calledTwice( command.execute ); - } ); - - it( 'should execute CheckTodoListCommand (macOS)', () => { - env.isMac = true; - - const command = editor.commands.get( 'checkTodoList' ); - - sinon.spy( command, 'execute' ); - - const domEvtDataStub = { - keyCode: getCode( 'enter' ), - metaKey: true, - preventDefault: sinon.spy(), - stopPropagation: sinon.spy() - }; + if ( env.isMac ) { + domEvtDataStub.metaKey = true; + } else { + domEvtDataStub.ctrlKey = true; + } // First call. viewDoc.fire( 'keydown', domEvtDataStub ); From 344fc54ccbc2181431c23be6f996a03f62809c67 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 23 Feb 2021 14:50:00 +0100 Subject: [PATCH 16/16] Code refactoring. --- packages/ckeditor5-utils/src/keyboard.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/ckeditor5-utils/src/keyboard.js b/packages/ckeditor5-utils/src/keyboard.js index ba50995e906..fdd2d41febd 100644 --- a/packages/ckeditor5-utils/src/keyboard.js +++ b/packages/ckeditor5-utils/src/keyboard.js @@ -13,16 +13,16 @@ import CKEditorError from './ckeditorerror'; import env from './env'; const modifiersToGlyphsMac = { - 'ctrl': '⌃', - 'cmd': '⌘', - 'alt': '⌥', - 'shift': '⇧' + ctrl: '⌃', + cmd: '⌘', + alt: '⌥', + shift: '⇧' }; const modifiersToGlyphsNonMac = { - 'ctrl': 'Ctrl+', - 'alt': 'Alt+', - 'shift': 'Shift+' + ctrl: 'Ctrl+', + alt: 'Alt+', + shift: 'Shift+' }; /**