Skip to content

Commit

Permalink
Updated keystroke normalization. Added tests.
Browse files Browse the repository at this point in the history
  • Loading branch information
@niegowski
niegowski committed Feb 20, 2021
1 parent 7c17d12 commit 5c2a69a
Show file tree
Hide file tree
Showing 5 changed files with 124 additions and 89 deletions.
2 changes: 1 addition & 1 deletion packages/ckeditor5-enter/src/enterobserver.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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' } );
Expand Down
2 changes: 1 addition & 1 deletion packages/ckeditor5-list/src/todolistediting.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
82 changes: 28 additions & 54 deletions packages/ckeditor5-utils/src/keyboard.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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+'
};

/**
Expand All @@ -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.
*
Expand Down Expand Up @@ -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.
Expand All @@ -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 ] : '' );
}

/**
Expand Down
119 changes: 86 additions & 33 deletions packages/ckeditor5-utils/tests/keyboard.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 );

Expand Down Expand Up @@ -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 );
} );
} );
} );

Expand Down Expand Up @@ -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' );
} );
} );

Expand All @@ -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' );
} );
} );
} );
Expand Down
8 changes: 8 additions & 0 deletions packages/ckeditor5-utils/tests/keystrokehandler.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit 5c2a69a

Please sign in to comment.