Skip to content

Prepare for diagram-js@9 / browser native focus handling #311

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.

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

Draft
wants to merge 8 commits into
base: develop
Choose a base branch
from
Draft
Changes from all commits
Commits
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
33 changes: 24 additions & 9 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -67,7 +67,7 @@
"chai": "^4.3.5",
"cross-env": "^7.0.3",
"del-cli": "^4.0.0",
"diagram-js": "^7.5.0",
"diagram-js": "^9.0.0-alpha.1",
"eslint": "^7.32.0",
"eslint-plugin-bpmn-io": "^0.14.0",
"eslint-plugin-import": "^2.23.4",
2 changes: 1 addition & 1 deletion packages/form-js-editor/src/FormEditor.js
Original file line number Diff line number Diff line change
@@ -59,7 +59,7 @@ export default class FormEditor {
*/
this._container = createFormContainer();

this._container.setAttribute('input-handle-modified-keys', 'z,y');
this._container.setAttribute('tabindex', '0');

const {
container,
Original file line number Diff line number Diff line change
@@ -1,14 +1,8 @@
import {
isCmd,
isKey,
isShift
isUndo,
isRedo
} from 'diagram-js/lib/features/keyboard/KeyboardUtil';

import {
KEYS_REDO,
KEYS_UNDO
} from 'diagram-js/lib/features/keyboard/KeyboardBindings';

const LOW_PRIORITY = 500;

export default class FormEditorKeyboardBindings {
@@ -33,7 +27,7 @@ export default class FormEditorKeyboardBindings {
addListener('undo', (context) => {
const { keyEvent } = context;

if (isCmd(keyEvent) && !isShift(keyEvent) && isKey(KEYS_UNDO, keyEvent)) {
if (isUndo(keyEvent)) {
editorActions.trigger('undo');

return true;
@@ -46,7 +40,7 @@ export default class FormEditorKeyboardBindings {
addListener('redo', (context) => {
const { keyEvent } = context;

if (isCmd(keyEvent) && (isKey(KEYS_REDO, keyEvent) || (isKey(KEYS_UNDO, keyEvent) && isShift(keyEvent)))) {
if (isRedo(keyEvent)) {
editorActions.trigger('redo');

return true;
27 changes: 27 additions & 0 deletions packages/form-js-editor/src/render/Renderer.js
Original file line number Diff line number Diff line change
@@ -26,6 +26,33 @@ export default class Renderer {
compact = false
} = renderConfig;

eventBus.on('form.init', function() {

// emit <canvas.init> so dependent components can hook in
// this is required to register keyboard bindings
eventBus.fire('canvas.init', {
svg: container,
viewport: null
});
});

// focus container on over if no selection
container.addEventListener('mouseover', function() {
if (document.activeElement === document.body) {
container.focus({ preventScroll: true });
}
});

// ensure we focus the container if the users clicks
// inside; this follows input focus handling closely
container.addEventListener('click', function(event) {

// force focus when clicking container
if (!container.contains(document.activeElement)) {
container.focus({ preventScroll: true });
}
});

const App = () => {
const [ state, setState ] = useState(formEditor._getState());

19 changes: 15 additions & 4 deletions packages/form-js-editor/src/render/components/FormEditor.js
Original file line number Diff line number Diff line change
@@ -78,11 +78,22 @@ function Element(props) {
return () => eventBus.off('selection.changed', scrollIntoView);
}, [ id ]);

function onClick(event) {
event.stopPropagation();
const onClick = useCallback((event) => {

// TODO(nikku): refactor this to use proper DOM delegation
const fieldEl = event.target.closest('[data-id]');

if (!fieldEl) {
return;
}

const id = fieldEl.dataset.id;

if (id === field.id) {
selection.toggle(field);
}
}, [ field ]);

selection.toggle(field);
}

const classes = [ 'fjs-element' ];

8 changes: 1 addition & 7 deletions packages/form-js-editor/test/spec/FormEditor.spec.js
Original file line number Diff line number Diff line change
@@ -56,10 +56,7 @@ describe('FormEditor', function() {
// when
formEditor = await createFormEditor({
container,
schema,
keyboard: {
bindTo: document
}
schema
});

formEditor.on('changed', event => {
@@ -80,9 +77,6 @@ describe('FormEditor', function() {
debounce: true,
renderer: {
compact: true
},
keyboard: {
bindTo: document
}
});

Original file line number Diff line number Diff line change
@@ -10,13 +10,11 @@ import modelingModule from 'src/features/modeling';

import { createKeyEvent } from 'diagram-js/test/util/KeyEvents';

import {
KEYS_REDO,
KEYS_UNDO
} from 'diagram-js/lib/features/keyboard/KeyboardBindings';

import schema from '../../form.json';

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


describe('features/editor-actions', function() {

@@ -32,22 +30,27 @@ describe('features/editor-actions', function() {
getFormEditor().destroy();
});

function triggerEvent(key, attrs, target) {
return getFormEditor().invoke(function(config) {
target = target || config.renderer.container;

return target.dispatchEvent(createKeyEvent(key, attrs));
});
}

KEYS_UNDO.forEach((key) => {

it('should undo', inject(function(keyboard, editorActions) {
it('should undo', inject(function(config, keyboard, editorActions) {

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

const event = createKeyEvent(key, {
// when
triggerEvent(key, {
ctrlKey: true,
shiftKey: false
});

// when
keyboard._keyHandler(event);

// then
expect(undoSpy).to.have.been.calledWith('undo');
}));
@@ -62,45 +65,45 @@ describe('features/editor-actions', function() {
// given
const redoSpy = sinon.spy(editorActions, 'trigger');

const event = createKeyEvent(key, {
// when
triggerEvent(key, {
ctrlKey: true,
shiftKey: false
});

// when
keyboard._keyHandler(event);

// then
expect(redoSpy).to.have.been.calledWith('redo');
}));

});


it('should undo/redo when focused on input', inject(function(formEditor, keyboard, editorActions) {
it('should undo/redo when focused on input', inject(
function(formEditor, keyboard, editorActions) {

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

const container = formEditor._container;
const inputField = document.createElement('input');
const container = formEditor._container;
const inputEl = document.createElement('input');

container.appendChild(inputField);
container.appendChild(inputEl);

// when
// select all
keyboard._keyHandler({ key: 'a', ctrlKey: true, target: inputField });
// when
// select all
triggerEvent('a', { ctrlKey: true }, inputEl);

// then
expect(spy).to.not.have.been.called;
// then
expect(spy).to.not.have.been.called;

// when
// undo/redo
keyboard._keyHandler({ key: 'z', ctrlKey: true, target: inputField, preventDefault: () => {} });
keyboard._keyHandler({ key: 'y', ctrlKey: true, target: inputField, preventDefault: () => {} });
// when
// undo/redo
triggerEvent('z', { ctrlKey: true }, inputEl);
triggerEvent('y', { ctrlKey: true }, inputEl);

// then
expect(spy).to.have.been.called.calledTwice;
}));
// then
expect(spy).to.have.been.called.calledTwice;
}
));

});
94 changes: 94 additions & 0 deletions packages/form-js-editor/test/spec/render/RendererSpec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
import {
bootstrapFormEditor,
getFormEditor,
inject
} from 'test/TestHelper';


describe('render', function() {

beforeEach(bootstrapFormEditor());

afterEach(function() {
getFormEditor().destroy();
});


function getContainer() {
return getFormEditor()._container;
}

describe('focus handling', function() {

it('should be focusable', inject(function(formEditor) {

// given
var container = getContainer();

// when
container.focus();

// then
expect(document.activeElement).to.equal(container);
}));


describe('<hover>', function() {

beforeEach(function() {
document.body.focus();
});


it('should focus if body is focused', inject(function(formEditor, eventBus) {

// given
var container = getContainer();

// when
container.emit(document.createEvent('mouseover'));

// then
expect(document.activeElement).to.equal(container);
}));


it('should not scroll on focus', inject(function(canvas, eventBus) {

// given
var container = getContainer();

var clientRect = container.getBoundingClientRect();

// when
container.emit(document.createEvent('mouseover'));

// then
expect(clientRect).to.eql(container.getBoundingClientRect());
}));


it('should not focus on existing document focus', inject(function(canvas, eventBus) {

// given
var container = getContainer();
var inputEl = document.createElement('input');

document.body.appendChild(inputEl);
inputEl.focus();

// assume
expect(document.activeElement).to.equal(inputEl);

// when
container.emit(document.createEvent('mouseover'));

// then
expect(document.activeElement).to.eql(inputEl);
}));

});

});

});