Skip to content

Commit

Permalink
feat: drop KeyboardEvent.keyCode in favor of code
Browse files Browse the repository at this point in the history
This fixes the issue of not detecting key strokes on non-latin
keyboards.

BREAKING CHANGE:

* Keyboard-related features no longer use KeyboardEvent#keyCode.
  Use a polyfill (e.g. [keyboardevent-key-polyfill](https://www.npmjs.com/package/keyboardevent-key-polyfill) if you need to support old browsers.

Co-authored-by: Maciej Barelkowski <[email protected]>
  • Loading branch information
kerlon5 and barmac committed Nov 15, 2022
1 parent 1346a8a commit ff861b0
Show file tree
Hide file tree
Showing 14 changed files with 62 additions and 88 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,13 @@ All notable changes to [diagram-js](https://github.com/bpmn-io/diagram-js) are d

_**Note:** Yet to be released changes appear here._

* `FEAT`: drop KeyboardEvent.keyCode in favor of `code` ([#681](https://github.com/bpmn-io/diagram-js/pull/681))

### Breaking Changes
* `FEAT`: new popupMenu UI featuring menu and group titles, search, entry descriptions and documentation urls ([#687](https://github.com/bpmn-io/diagram-js/pull/687))
* `FEAT`: introduction of `.djs-parent` class to canvas and popup menu root ([#687](https://github.com/bpmn-io/diagram-js/pull/687))
* Keyboard-related features no longer use KeyboardEvent#keyCode.
Use a polyfill (e.g. [keyboardevent-key-polyfill](https://www.npmjs.com/package/keyboardevent-key-polyfill) if you need to support old browsers.


## 10.0.0
Expand Down
4 changes: 3 additions & 1 deletion lib/features/dragging/Dragging.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import {
delta as deltaPos
} from '../../util/PositionUtil';

import { isKey } from '../keyboard/KeyboardUtil';

var DRAG_ACTIVE_CLS = 'djs-drag-active';


Expand Down Expand Up @@ -282,7 +284,7 @@ export default function Dragging(eventBus, canvas, selection, elementRegistry) {

function checkCancel(event) {

if (event.which === 27) {
if (isKey('Escape')) {
preventDefault(event);

cancel();
Expand Down
2 changes: 1 addition & 1 deletion lib/features/hand-tool/HandTool.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,5 +155,5 @@ HandTool.prototype.isActive = function() {
// helpers //////////

function isSpace(keyEvent) {
return isKey(' ', keyEvent);
return isKey('Space', keyEvent);
}
13 changes: 4 additions & 9 deletions lib/features/keyboard/KeyboardBindings.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,10 @@ import {

var LOW_PRIORITY = 500;

export var KEYCODE_C = 67;
export var KEYCODE_V = 86;
export var KEYCODE_Y = 89;
export var KEYCODE_Z = 90;

export var KEYS_COPY = [ 'c', 'C', KEYCODE_C ];
export var KEYS_PASTE = [ 'v', 'V', KEYCODE_V ];
export var KEYS_REDO = [ 'y', 'Y', KEYCODE_Y ];
export var KEYS_UNDO = [ 'z', 'Z', KEYCODE_Z ];
export var KEYS_COPY = [ 'c', 'C', 'KeyC' ];
export var KEYS_PASTE = [ 'v', 'V', 'KeyV' ];
export var KEYS_REDO = [ 'y', 'Y', 'KeyY' ];

This comment has been minimized.

Copy link
@nikku

nikku Feb 14, 2023

Member

This broke German keyboard layouts, where z and y are swapped.

In order to keep the original behavior it should have been:

export var KEYS_REDO = [ 'y', 'Y', 'KeyZ' ];
export var KEYS_UNDO = [ 'z', 'Z', 'KeyZ' ];


/**
Expand Down
15 changes: 5 additions & 10 deletions lib/features/keyboard/KeyboardUtil.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,9 @@
import { isArray } from 'min-dash';

var KEYCODE_C = 67;
var KEYCODE_V = 86;
var KEYCODE_Y = 89;
var KEYCODE_Z = 90;

var KEYS_COPY = [ 'c', 'C', KEYCODE_C ];
var KEYS_PASTE = [ 'v', 'V', KEYCODE_V ];
var KEYS_REDO = [ 'y', 'Y', KEYCODE_Y ];
var KEYS_UNDO = [ 'z', 'Z', KEYCODE_Z ];
var KEYS_COPY = [ 'c', 'C', 'KeyC' ];
var KEYS_PASTE = [ 'v', 'V', 'KeyV' ];
var KEYS_REDO = [ 'y', 'Y', 'KeyY' ];
var KEYS_UNDO = [ 'z', 'Z', 'KeyZ' ];

/**
* Returns true if event was triggered with any modifier
Expand Down Expand Up @@ -41,7 +36,7 @@ export function isCmd(event) {
export function isKey(keys, event) {
keys = isArray(keys) ? keys : [ keys ];

return keys.indexOf(event.key) !== -1 || keys.indexOf(event.keyCode) !== -1;
return keys.indexOf(event.key) !== -1 || keys.indexOf(event.code) !== -1;
}

/**
Expand Down
23 changes: 9 additions & 14 deletions lib/features/search-pad/SearchPad.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import {
escapeHTML
} from '../../util/EscapeUtil';

import { isKey } from '../keyboard/KeyboardUtil';

/**
* Provides searching infrastructure
*/
Expand Down Expand Up @@ -93,45 +95,38 @@ SearchPad.prototype._bindEvents = function() {
// navigate results
listen(this._container, SearchPad.INPUT_SELECTOR, 'keydown', function(e) {

// up
if (e.keyCode === 38) {
if (isKey('ArrowUp', e)) {
e.preventDefault();
}

// down
if (e.keyCode === 40) {
if (isKey('ArrowDown', e)) {
e.preventDefault();
}
});

// handle keyboard input
listen(this._container, SearchPad.INPUT_SELECTOR, 'keyup', function(e) {

// escape
if (e.keyCode === 27) {
if (isKey('Escape', e)) {
return self.close();
}

// enter
if (e.keyCode === 13) {
if (isKey('Enter', e)) {
var selected = self._getCurrentResult();

return selected ? self._select(selected) : self.close();
}

// up
if (e.keyCode === 38) {
if (isKey('ArrowUp', e)) {
return self._scrollToDirection(true);
}

// down
if (e.keyCode === 40) {
if (isKey('ArrowDown', e)) {
return self._scrollToDirection();
}

// left && right
// do not search while navigating text input
if (e.keyCode === 37 || e.keyCode === 39) {
if (isKey([ 'ArrowLeft', 'ArrowRight' ], e)) {
return;
}

Expand Down
8 changes: 4 additions & 4 deletions test/spec/features/hand-tool/HandToolSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ describe('features/hand-tool', function() {

// when
eventBus.fire('keyboard.keydown', {
keyEvent: createKeyEvent(' ')
keyEvent: createKeyEvent('Space')
});

// then
Expand All @@ -152,12 +152,12 @@ describe('features/hand-tool', function() {

// given
eventBus.fire('keyboard.keydown', {
keyEvent: createKeyEvent(' ')
keyEvent: createKeyEvent('Space')
});

// when
eventBus.fire('keyboard.keyup', {
keyEvent: createKeyEvent(' ')
keyEvent: createKeyEvent('Space')
});

// then
Expand All @@ -169,7 +169,7 @@ describe('features/hand-tool', function() {

// given
eventBus.fire('keyboard.keydown', {
keyEvent: createKeyEvent(' ')
keyEvent: createKeyEvent('Space')
});

// when
Expand Down
2 changes: 1 addition & 1 deletion test/spec/features/keyboard/CopySpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import editorActionsModule from 'lib/features/editor-actions';

import { createKeyEvent } from 'test/util/KeyEvents';

var KEYS_COPY = [ 'c', 'C', 67 ];
var KEYS_COPY = [ 'c', 'C', 'KeyC' ];


describe('features/keyboard - copy', function() {
Expand Down
15 changes: 3 additions & 12 deletions test/spec/features/keyboard/KeyboardSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { createKeyEvent } from 'test/util/KeyEvents';

describe('features/keyboard', function() {

var TEST_KEY = 99;
var TEST_KEY = 'Numpad3';

var defaultDiagramConfig = {
modules: [
Expand Down Expand Up @@ -442,15 +442,6 @@ describe('features/keyboard', function() {
// helpers //////////

function dispatchKeyboardEvent(target, type) {
var event;

try {
event = new KeyboardEvent(type);
} catch (e) {
event = document.createEvent('KeyboardEvent');

event.initEvent(type, true, false);
}

var event = createKeyEvent('Any', { type: type });
target.dispatchEvent(event);
}
}
2 changes: 1 addition & 1 deletion test/spec/features/keyboard/PasteSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import editorActionsModule from 'lib/features/editor-actions';

import { createKeyEvent } from 'test/util/KeyEvents';

var KEYS_PASTE = [ 'v', 'V', 86 ];
var KEYS_PASTE = [ 'v', 'V', 'KeyV' ];


describe('features/keyboard - paste', function() {
Expand Down
4 changes: 2 additions & 2 deletions test/spec/features/keyboard/RedoSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import keyboardModule from 'lib/features/keyboard';

import { createKeyEvent } from 'test/util/KeyEvents';

var KEYS_REDO = [ 'y', 'Y', 89 ];
var KEYS_UNDO = [ 'z', 'Z', 90 ];
var KEYS_REDO = [ 'y', 'Y', 'KeyY' ];
var KEYS_UNDO = [ 'z', 'Z', 'KeyZ' ];


describe('features/keyboard - redo', function() {
Expand Down
2 changes: 1 addition & 1 deletion test/spec/features/keyboard/UndoSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import keyboardModule from 'lib/features/keyboard';

import { createKeyEvent } from 'test/util/KeyEvents';

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


describe('features/keyboard - undo', function() {
Expand Down
42 changes: 20 additions & 22 deletions test/spec/features/search-pad/SearchPadSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ import {
inject
} from 'test/TestHelper';

import {
createKeyEvent
} from 'test/util/KeyEvents';

import searchPadModule from 'lib/features/search-pad';
import SearchPad from 'lib/features/search-pad/SearchPad';

Expand Down Expand Up @@ -267,7 +271,7 @@ describe('features/searchPad', function() {
typeText(input_node, 'two');

// when
triggerKeyEvent(input_node, 'keyup', 13);
triggerKeyEvent(input_node, 'keyup', 'Enter');

// then
expect(capturedEvents).to.eql([
Expand Down Expand Up @@ -296,7 +300,7 @@ describe('features/searchPad', function() {
typeText(input_node, 'one');

// when
triggerKeyEvent(input_node, 'keyup', 13);
triggerKeyEvent(input_node, 'keyup', 'Enter');

// then
var overlay = overlays.get({ element: element });
Expand All @@ -321,7 +325,7 @@ describe('features/searchPad', function() {
});

// when
triggerKeyEvent(input_node, 'keyup', 13);
triggerKeyEvent(input_node, 'keyup', 'Enter');

// then
var newViewbox = canvas.viewbox();
Expand All @@ -338,7 +342,7 @@ describe('features/searchPad', function() {
typeText(input_node, 'one');

// when
triggerKeyEvent(input_node, 'keyup', 13);
triggerKeyEvent(input_node, 'keyup', 'Enter');

// then
var newViewbox = canvas.viewbox();
Expand All @@ -352,7 +356,7 @@ describe('features/searchPad', function() {
typeText(input_node, 'one');

// when
triggerKeyEvent(input_node, 'keyup', 13);
triggerKeyEvent(input_node, 'keyup', 'Enter');

// then
expect(selection.isSelected(element)).to.be.true;
Expand All @@ -362,7 +366,7 @@ describe('features/searchPad', function() {
it('should close on escape', inject(function(canvas, eventBus, searchPad) {

// when
triggerKeyEvent(input_node, 'keyup', 27);
triggerKeyEvent(input_node, 'keyup', 'Escape');

// then
expect(searchPad.isOpen()).to.equal(false);
Expand All @@ -377,14 +381,14 @@ describe('features/searchPad', function() {
var result_nodes = domQueryAll(SearchPad.RESULT_SELECTOR, canvas.getContainer());

// when press 'down'
triggerKeyEvent(input_node, 'keyup', 40);
triggerKeyEvent(input_node, 'keyup', 'ArrowDown');

// then
expect(domClasses(result_nodes[0]).has(SearchPad.RESULT_SELECTED_CLASS)).to.be.false;
expect(domClasses(result_nodes[1]).has(SearchPad.RESULT_SELECTED_CLASS)).to.be.true;

// when press 'up'
triggerKeyEvent(input_node, 'keyup', 38);
triggerKeyEvent(input_node, 'keyup', 'ArrowUp');

// then
expect(domClasses(result_nodes[0]).has(SearchPad.RESULT_SELECTED_CLASS)).to.be.true;
Expand All @@ -405,7 +409,7 @@ describe('features/searchPad', function() {
typeText(input_node, 'two');

// when press 'down'
var e = triggerKeyEvent(input_node, 'keydown', 40);
var e = triggerKeyEvent(input_node, 'keydown', 'ArrowDown');
expect(e.defaultPrevented).to.be.true;
}));

Expand All @@ -416,7 +420,7 @@ describe('features/searchPad', function() {
typeText(input_node, 'two');

// when press 'up'
var e = triggerKeyEvent(input_node, 'keydown', 38);
var e = triggerKeyEvent(input_node, 'keydown', 'ArrowUp');
expect(e.defaultPrevented).to.be.true;
}));

Expand All @@ -428,7 +432,7 @@ describe('features/searchPad', function() {
typeText(input_node, 'two');

// when press 'left'
triggerKeyEvent(input_node, 'keyup', 37);
triggerKeyEvent(input_node, 'keyup', 'ArrowLeft');

// then
expect(find).callCount('two'.length);
Expand All @@ -442,7 +446,7 @@ describe('features/searchPad', function() {
typeText(input_node, 'two');

// when press 'right'
triggerKeyEvent(input_node, 'keyup', 39);
triggerKeyEvent(input_node, 'keyup', 'ArrowRight');

// then
expect(find).callCount('two'.length);
Expand All @@ -453,18 +457,12 @@ describe('features/searchPad', function() {
});


function triggerKeyEvent(element, event, code) {
var e = document.createEvent('Events');

if (e.initEvent) {
e.initEvent(event, true, true);
}
function triggerKeyEvent(element, eventType, code) {
var event = createKeyEvent(code, { type: eventType });

e.keyCode = code;
e.which = code;
element.dispatchEvent(e);
element.dispatchEvent(event);

return e;
return event;
}


Expand Down
Loading

0 comments on commit ff861b0

Please sign in to comment.