From 51996e9e02e4da7b2c882a68beff24cba9a799c8 Mon Sep 17 00:00:00 2001 From: Nico Rehwaldt Date: Wed, 17 Aug 2022 21:32:59 +0200 Subject: [PATCH 1/8] deps: update to diagram-js@9.0.0-alpha.1 --- package-lock.json | 33 ++++++++++++++++++++++++--------- package.json | 2 +- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/package-lock.json b/package-lock.json index 22c4b3079..29b895a09 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6620,26 +6620,41 @@ "dev": true }, "diagram-js": { - "version": "7.5.0", - "resolved": "https://registry.npmjs.org/diagram-js/-/diagram-js-7.5.0.tgz", - "integrity": "sha512-BAs3PJhTQYd+PjehBlf+ofGwEhSjsK1ku0B3MZKWQ4c2g1graa8CXHOdSHtxWY3KTcBVfARqx0DJdFga66Lfeg==", + "version": "9.0.0-alpha.1", + "resolved": "https://registry.npmjs.org/diagram-js/-/diagram-js-9.0.0-alpha.1.tgz", + "integrity": "sha512-F8QasxlU04PQbs7GSQ+OWnHjc6wLa053hK4yBnmYtDnYNTTy41f3HsvUG03JvepBSzd8ysd37vFqJB3h/JFDmg==", "dev": true, "requires": { "css.escape": "^1.5.1", - "didi": "^5.2.1", + "didi": "^8.0.1", "hammerjs": "^2.0.1", - "inherits": "^2.0.4", + "inherits-browser": "0.0.1", "min-dash": "^3.5.2", - "min-dom": "^3.1.3", + "min-dom": "^3.2.0", "object-refs": "^0.3.0", "path-intersection": "^2.2.1", "tiny-svg": "^2.2.2" + }, + "dependencies": { + "min-dom": { + "version": "3.2.1", + "resolved": "https://registry.npmjs.org/min-dom/-/min-dom-3.2.1.tgz", + "integrity": "sha512-v6YCmnDzxk4rRJntWTUiwggLupPw/8ZSRqUq0PDaBwVZEO/wYzCH4SKVBV+KkEvf3u0XaWHly5JEosPtqRATZA==", + "dev": true, + "requires": { + "component-event": "^0.1.4", + "domify": "^1.3.1", + "indexof": "0.0.1", + "matches-selector": "^1.2.0", + "min-dash": "^3.8.1" + } + } } }, "didi": { - "version": "5.2.1", - "resolved": "https://registry.npmjs.org/didi/-/didi-5.2.1.tgz", - "integrity": "sha512-IKNnajUlD4lWMy/Q9Emkk7H1qnzREgY4UyE3IhmOi/9IKua0JYtYldk928bOdt1yNxN8EiOy1sqtSozEYsmjCg==", + "version": "8.0.1", + "resolved": "https://registry.npmjs.org/didi/-/didi-8.0.1.tgz", + "integrity": "sha512-7oXiXbp8DHE3FfQsVBkc2pwePo3Jy2uyGS9trAeBmfxiZAP4WV23LWokRpMmyl3hlu8OEAsyMxx19i5P6TVaJQ==", "dev": true }, "diff": { diff --git a/package.json b/package.json index bebcadb57..485112e02 100644 --- a/package.json +++ b/package.json @@ -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", From 10a469b3ed573b9890416f5ffaba5b5e0517eba8 Mon Sep 17 00:00:00 2001 From: Nico Rehwaldt Date: Wed, 17 Aug 2022 21:33:38 +0200 Subject: [PATCH 2/8] chore: make container keyboard focusable --- packages/form-js-editor/src/FormEditor.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/form-js-editor/src/FormEditor.js b/packages/form-js-editor/src/FormEditor.js index 5f5affb34..c52d9d73c 100644 --- a/packages/form-js-editor/src/FormEditor.js +++ b/packages/form-js-editor/src/FormEditor.js @@ -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, From 7d172ff67a29157cde5baf250cbf7e621b749e66 Mon Sep 17 00:00:00 2001 From: Nico Rehwaldt Date: Wed, 17 Aug 2022 21:50:48 +0200 Subject: [PATCH 3/8] chore: emit `canvas.init` event to initialize keyboard --- packages/form-js-editor/src/render/Renderer.js | 10 ++++++++++ packages/form-js-editor/test/spec/FormEditor.spec.js | 8 +------- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/packages/form-js-editor/src/render/Renderer.js b/packages/form-js-editor/src/render/Renderer.js index 5182484e6..ea7f2a312 100644 --- a/packages/form-js-editor/src/render/Renderer.js +++ b/packages/form-js-editor/src/render/Renderer.js @@ -26,6 +26,16 @@ export default class Renderer { compact = false } = renderConfig; + eventBus.on('form.init', function() { + + // emit so dependent components can hook in + // this is required to register keyboard bindings + eventBus.fire('canvas.init', { + svg: container, + viewport: null + }); + }); + const App = () => { const [ state, setState ] = useState(formEditor._getState()); diff --git a/packages/form-js-editor/test/spec/FormEditor.spec.js b/packages/form-js-editor/test/spec/FormEditor.spec.js index 8aa9e878e..2840c3add 100644 --- a/packages/form-js-editor/test/spec/FormEditor.spec.js +++ b/packages/form-js-editor/test/spec/FormEditor.spec.js @@ -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 } }); From 8a3a7d4a92b0947f4bbc68259fe8bdd41523dca1 Mon Sep 17 00:00:00 2001 From: Nico Rehwaldt Date: Wed, 17 Aug 2022 21:50:12 +0200 Subject: [PATCH 4/8] chore: use feature detection utilities --- .../keyboard/FormEditorKeyboardBindings.js | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/packages/form-js-editor/src/features/keyboard/FormEditorKeyboardBindings.js b/packages/form-js-editor/src/features/keyboard/FormEditorKeyboardBindings.js index 4825b79b1..81717b379 100644 --- a/packages/form-js-editor/src/features/keyboard/FormEditorKeyboardBindings.js +++ b/packages/form-js-editor/src/features/keyboard/FormEditorKeyboardBindings.js @@ -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; From d16c2b4509d3d3e71895ce6d3423489dde2688c4 Mon Sep 17 00:00:00 2001 From: Nico Rehwaldt Date: Wed, 17 Aug 2022 22:14:55 +0200 Subject: [PATCH 5/8] feat(editor): implement diagram-js style hover handling --- .../form-js-editor/src/render/Renderer.js | 6 ++ .../test/spec/renderer/RendererSpec.js | 93 +++++++++++++++++++ 2 files changed, 99 insertions(+) create mode 100644 packages/form-js-editor/test/spec/renderer/RendererSpec.js diff --git a/packages/form-js-editor/src/render/Renderer.js b/packages/form-js-editor/src/render/Renderer.js index ea7f2a312..4075d0b50 100644 --- a/packages/form-js-editor/src/render/Renderer.js +++ b/packages/form-js-editor/src/render/Renderer.js @@ -36,6 +36,12 @@ export default class Renderer { }); }); + eventBus.on('element.hover', function() { + if (document.activeElement === document.body) { + container.focus({ preventScroll: true }); + } + }); + const App = () => { const [ state, setState ] = useState(formEditor._getState()); diff --git a/packages/form-js-editor/test/spec/renderer/RendererSpec.js b/packages/form-js-editor/test/spec/renderer/RendererSpec.js new file mode 100644 index 000000000..a3cc194af --- /dev/null +++ b/packages/form-js-editor/test/spec/renderer/RendererSpec.js @@ -0,0 +1,93 @@ +import { + bootstrapFormEditor, + getFormEditor, + inject +} from 'test/TestHelper'; + + +describe('renderer', 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('', function() { + + beforeEach(function() { + document.body.focus(); + }); + + + it('should focus if body is focused', inject(function(formEditor, eventBus) { + + // given + var container = getContainer(); + + // when + eventBus.fire('element.hover'); + + // 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 + eventBus.fire('element.hover'); + + // then + expect(clientRect).to.eql(container.getBoundingClientRect()); + })); + + + it('should not focus on existing document focus', inject(function(canvas, eventBus) { + + // given + var inputEl = document.createElement('input'); + + document.body.appendChild(inputEl); + inputEl.focus(); + + // assume + expect(document.activeElement).to.equal(inputEl); + + // when + eventBus.fire('element.hover'); + + // then + expect(document.activeElement).to.eql(inputEl); + })); + + }); + + }); + +}); \ No newline at end of file From bff07e172f57df028661d479c153b292717535e2 Mon Sep 17 00:00:00 2001 From: Nico Rehwaldt Date: Wed, 17 Aug 2022 22:15:30 +0200 Subject: [PATCH 6/8] test(editor): test keyboard using actual DOM events --- .../FormEditorKeyboardBindings.spec.js | 67 ++++++++++--------- 1 file changed, 35 insertions(+), 32 deletions(-) diff --git a/packages/form-js-editor/test/spec/features/keyboard/FormEditorKeyboardBindings.spec.js b/packages/form-js-editor/test/spec/features/keyboard/FormEditorKeyboardBindings.spec.js index 81c762870..1035d1541 100644 --- a/packages/form-js-editor/test/spec/features/keyboard/FormEditorKeyboardBindings.spec.js +++ b/packages/form-js-editor/test/spec/features/keyboard/FormEditorKeyboardBindings.spec.js @@ -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,14 +65,12 @@ 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'); })); @@ -77,30 +78,32 @@ describe('features/editor-actions', function() { }); - 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; + } + )); }); From 359101b11d3fdf715edcfee03056f5da2fdc175c Mon Sep 17 00:00:00 2001 From: Nico Rehwaldt Date: Wed, 17 Aug 2022 23:04:37 +0200 Subject: [PATCH 7/8] feat(editor): focus container on click --- packages/form-js-editor/src/render/Renderer.js | 13 ++++++++++++- .../test/spec/{renderer => render}/RendererSpec.js | 9 +++++---- 2 files changed, 17 insertions(+), 5 deletions(-) rename packages/form-js-editor/test/spec/{renderer => render}/RendererSpec.js (86%) diff --git a/packages/form-js-editor/src/render/Renderer.js b/packages/form-js-editor/src/render/Renderer.js index 4075d0b50..5f84de874 100644 --- a/packages/form-js-editor/src/render/Renderer.js +++ b/packages/form-js-editor/src/render/Renderer.js @@ -36,12 +36,23 @@ export default class Renderer { }); }); - eventBus.on('element.hover', function() { + // 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()); diff --git a/packages/form-js-editor/test/spec/renderer/RendererSpec.js b/packages/form-js-editor/test/spec/render/RendererSpec.js similarity index 86% rename from packages/form-js-editor/test/spec/renderer/RendererSpec.js rename to packages/form-js-editor/test/spec/render/RendererSpec.js index a3cc194af..524f0c5e1 100644 --- a/packages/form-js-editor/test/spec/renderer/RendererSpec.js +++ b/packages/form-js-editor/test/spec/render/RendererSpec.js @@ -5,7 +5,7 @@ import { } from 'test/TestHelper'; -describe('renderer', function() { +describe('render', function() { beforeEach(bootstrapFormEditor()); @@ -46,7 +46,7 @@ describe('renderer', function() { var container = getContainer(); // when - eventBus.fire('element.hover'); + container.emit(document.createEvent('mouseover')); // then expect(document.activeElement).to.equal(container); @@ -61,7 +61,7 @@ describe('renderer', function() { var clientRect = container.getBoundingClientRect(); // when - eventBus.fire('element.hover'); + container.emit(document.createEvent('mouseover')); // then expect(clientRect).to.eql(container.getBoundingClientRect()); @@ -71,6 +71,7 @@ describe('renderer', function() { 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); @@ -80,7 +81,7 @@ describe('renderer', function() { expect(document.activeElement).to.equal(inputEl); // when - eventBus.fire('element.hover'); + container.emit(document.createEvent('mouseover')); // then expect(document.activeElement).to.eql(inputEl); From 03ff6865d1b8afe3bbbcc4f2e232f9a06586d952 Mon Sep 17 00:00:00 2001 From: Nico Rehwaldt Date: Wed, 17 Aug 2022 23:05:17 +0200 Subject: [PATCH 8/8] fix(editor): don't prevent `click` bubbling (A better fix would be using proper event delegation, but OK...) --- .../src/render/components/FormEditor.js | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/packages/form-js-editor/src/render/components/FormEditor.js b/packages/form-js-editor/src/render/components/FormEditor.js index eb04299df..db7d8452d 100644 --- a/packages/form-js-editor/src/render/components/FormEditor.js +++ b/packages/form-js-editor/src/render/components/FormEditor.js @@ -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' ];