Skip to content

fix(keyboard): use key codes for undo/redo to support all keyboard la… #980

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

fix(keyboard): use key codes for undo/redo to support all keyboard la… #980

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions lib/features/keyboard/KeyboardBindings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import {
KEYS_COPY,
KEYS_PASTE,
KEYS_UNDO,
KEYS_REDO
KEYS_REDO,
KEY_CODES
} from './KeyboardUtil';

/**
Expand All @@ -21,7 +22,8 @@ export {
KEYS_COPY,
KEYS_PASTE,
KEYS_UNDO,
KEYS_REDO
KEYS_REDO,
KEY_CODES
};


Expand Down Expand Up @@ -79,7 +81,7 @@ KeyboardBindings.prototype.registerBindings = function(keyboard, editorActions)


// undo
// (CTRL|CMD) + Z
// (CTRL|CMD) + Z (any keyboard layout)
addListener('undo', function(context) {

var event = context.keyEvent;
Expand All @@ -92,8 +94,8 @@ KeyboardBindings.prototype.registerBindings = function(keyboard, editorActions)
});

// redo
// CTRL + Y
// CMD + SHIFT + Z
// CTRL + Y (any keyboard layout)
// CMD + SHIFT + Z (any keyboard layout)
addListener('redo', function(context) {

var event = context.keyEvent;
Expand Down
18 changes: 16 additions & 2 deletions lib/features/keyboard/KeyboardUtil.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ export var KEYS_PASTE = [ 'v', 'V' ];
export var KEYS_REDO = [ 'y', 'Y' ];
export var KEYS_UNDO = [ 'z', 'Z' ];

// Use key codes for consistent behavior across keyboard layouts
export var KEY_CODES = {
Z: 'KeyZ',
Y: 'KeyY'
};

/**
* Returns true if event was triggered with any modifier
* @param {KeyboardEvent} event
Expand Down Expand Up @@ -66,16 +72,24 @@ export function isPaste(event) {
* @param {KeyboardEvent} event
*/
export function isUndo(event) {
return isCmd(event) && !isShift(event) && isKey(KEYS_UNDO, event);
return isCmd(event) && !isShift(event) && (
isKey(KEYS_UNDO, event) ||
event.code === KEY_CODES.Z
);
}

/**
* @param {KeyboardEvent} event
*/
export function isRedo(event) {
return isCmd(event) && (
isKey(KEYS_REDO, event) || (
isKey(KEYS_REDO, event) ||
event.code === KEY_CODES.Y ||
(
isKey(KEYS_UNDO, event) && isShift(event)
) ||
(
event.code === KEY_CODES.Z && isShift(event)
)
);
}
51 changes: 41 additions & 10 deletions test/spec/features/keyboard/RedoSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import { createKeyEvent } from 'test/util/KeyEvents';

var KEYS_REDO = [ 'y', 'Y' ];
var KEYS_UNDO = [ 'z', 'Z' ];
var KEY_CODE_Y = 'KeyY';
var KEY_CODE_Z = 'KeyZ';


describe('features/keyboard - redo', function() {
Expand All @@ -31,20 +33,26 @@ describe('features/keyboard - redo', function() {
};

var decisionTable = [ {
desc: 'should call redo',
keys: KEYS_UNDO,
desc: 'should call redo (Y key)',
keys: KEYS_REDO,
ctrlKey: true,
shiftKey: true,
shiftKey: false,
called: true
}, {
desc: 'should call redo',
keys: KEYS_REDO,
desc: 'should call redo (KeyY code)',
keyCode: KEY_CODE_Y,
ctrlKey: true,
shiftKey: false,
called: true
}, {
desc: 'should call redo',
keys: KEYS_REDO,
desc: 'should call redo (Z + Shift)',
keys: KEYS_UNDO,
ctrlKey: true,
shiftKey: true,
called: true
}, {
desc: 'should call redo (KeyZ + Shift)',
keyCode: KEY_CODE_Z,
ctrlKey: true,
shiftKey: true,
called: true
Expand Down Expand Up @@ -79,16 +87,39 @@ describe('features/keyboard - redo', function() {

forEach(decisionTable, function(testCase) {

forEach(testCase.keys, function(key) {
if (testCase.keys) {
forEach(testCase.keys, function(key) {

it(testCase.desc, inject(function(keyboard, editorActions) {

// given
var redoSpy = sinon.spy(editorActions, 'trigger');

var event = createKeyEvent(key, {
ctrlKey: testCase.ctrlKey,
shiftKey: testCase.shiftKey
});

// when
keyboard._keyHandler(event);

// then
expect(redoSpy.calledWith('redo')).to.be.equal(testCase.called);
}));

});
} else if (testCase.keyCode) {

it(testCase.desc, inject(function(keyboard, editorActions) {

// given
var redoSpy = sinon.spy(editorActions, 'trigger');

var key = testCase.keyCode === KEY_CODE_Y ? 'y' : 'z';
var event = createKeyEvent(key, {
ctrlKey: testCase.ctrlKey,
shiftKey: testCase.shiftKey
shiftKey: testCase.shiftKey,
code: testCase.keyCode
});

// when
Expand All @@ -98,7 +129,7 @@ describe('features/keyboard - redo', function() {
expect(redoSpy.calledWith('redo')).to.be.equal(testCase.called);
}));

});
}

});

Expand Down
47 changes: 35 additions & 12 deletions test/spec/features/keyboard/UndoSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import keyboardModule from 'lib/features/keyboard';
import { createKeyEvent } from 'test/util/KeyEvents';

var KEYS_UNDO = [ 'z', 'Z' ];
var KEY_CODE_Z = 'KeyZ';


describe('features/keyboard - undo', function() {
Expand All @@ -30,28 +31,28 @@ describe('features/keyboard - undo', function() {
};

var decisionTable = [ {
desc: 'should call undo',
desc: 'should call undo (Z key)',
keys: KEYS_UNDO,
ctrlKey: true,
shiftKey: false,
called: true
}, {
desc: 'should not call undo',
keys: KEYS_UNDO,
desc: 'should call undo (KeyZ code)',
keyCode: KEY_CODE_Z,
ctrlKey: true,
shiftKey: true,
called: false
shiftKey: false,
called: true
}, {
desc: 'should not call undo',
keys: KEYS_UNDO,
ctrlKey: false,
shiftKey: true,
shiftKey: false,
called: false
}, {
desc: 'should not call undo',
keys: KEYS_UNDO,
ctrlKey: false,
shiftKey: false,
ctrlKey: true,
shiftKey: true,
called: false
} ];

Expand All @@ -60,16 +61,38 @@ describe('features/keyboard - undo', function() {

forEach(decisionTable, function(testCase) {

forEach(testCase.keys, function(key) {
if (testCase.keys) {
forEach(testCase.keys, function(key) {

it(testCase.desc, inject(function(keyboard, editorActions) {

// given
var undoSpy = sinon.spy(editorActions, 'trigger');

var event = createKeyEvent(key, {
ctrlKey: testCase.ctrlKey,
shiftKey: testCase.shiftKey
});

// when
keyboard._keyHandler(event);

// then
expect(undoSpy.calledWith('undo')).to.be.equal(testCase.called);
}));

});
} else if (testCase.keyCode) {

it(testCase.desc, inject(function(keyboard, editorActions) {

// given
var undoSpy = sinon.spy(editorActions, 'trigger');

var event = createKeyEvent(key, {
var event = createKeyEvent('z', {
ctrlKey: testCase.ctrlKey,
shiftKey: testCase.shiftKey
shiftKey: testCase.shiftKey,
code: testCase.keyCode
});

// when
Expand All @@ -79,7 +102,7 @@ describe('features/keyboard - undo', function() {
expect(undoSpy.calledWith('undo')).to.be.equal(testCase.called);
}));

});
}

});

Expand Down