fix: port implicit keyboard binding configuration to decision editors

Closes #920
bpmn-io · Nov 15, 2024 · 1d2bc3e · 1d2bc3e
1 parent 734f9d2
commit 1d2bc3e
Show file tree

Hide file tree

Showing 5 changed files with 84 additions and 31 deletions.
diff --git a/packages/dmn-js-decision-table/src/features/keyboard/Keyboard.js b/packages/dmn-js-decision-table/src/features/keyboard/Keyboard.js
@@ -11,6 +11,10 @@ import {
   isShift
 } from './KeyboardUtil';
 
+var compatMessage =
+  'Keyboard binding is now implicit; explicit binding to an element got removed. ' +
+  'For more information, see https://github.com/bpmn-io/diagram-js/pull/662';
+
 
 /**
  * A keyboard abstraction that may be activated and
@@ -27,17 +31,18 @@ import {
  *
  * All events contain the fields (node, listeners).
  *
- * A default binding for the keyboard may be specified via the
- * `keyboard.bindTo` configuration option.
+ * Specify the initial keyboard binding state via the
+ * `keyboard.bind=true|false` configuration option.
  *
  * @param {Config} config
  * @param {EventBus} eventBus
  * @param {EditorActions} editorActions
  * @param {CellSelection} cellSelection
+ * @param {Renderer} renderer
  */
 export default class Keyboard {
 
-  constructor(config, eventBus, editorActions, cellSelection) {
+  constructor(config, eventBus, editorActions, cellSelection, renderer) {
 
     this._config = config || {};
     this._editorActions = editorActions;
@@ -52,7 +57,16 @@ export default class Keyboard {
     eventBus.on('attach', () => {
 
       if (this._config.bindTo) {
-        this.bind(config.bindTo);
+        console.error('unsupported configuration <keyboard.bindTo>',
+          new Error(compatMessage));
+      }
+
+      this._target = renderer.getContainer();
+
+      var bind = this._config && this._config.bind !== false;
+
+      if (bind) {
+        this.bind();
       }
     });
 
@@ -96,10 +110,14 @@ export default class Keyboard {
 
   bind(node) {
 
+    if (node) {
+      console.error('unsupported argument <node>', new Error(compatMessage));
+    }
+
     // make sure that the keyboard is only bound once to the DOM
     this.unbind();
 
-    this._node = node;
+    node = this._node = this._target;
 
     // bind key events
     domEvent.bind(node, 'keydown', this._keyHandler);
@@ -241,7 +259,8 @@ Keyboard.$inject = [
   'config.keyboard',
   'eventBus',
   'editorActions',
-  'cellSelection'
+  'cellSelection',
+  'renderer'
 ];
 
 

diff --git a/...es/dmn-js-decision-table/test/spec/features/copy-cut-paste/CopyCutPasteKeyBindingsSpec.js b/...es/dmn-js-decision-table/test/spec/features/copy-cut-paste/CopyCutPasteKeyBindingsSpec.js
@@ -30,7 +30,6 @@ import diagramXML from './copy-cut-paste-key-bindings.dmn';
 
 describe('features/copy-cut-paste/key-bindings', function() {
 
-  const keyboardTarget = document.createElement('div');
 
   beforeEach(bootstrapModeler(diagramXML, {
     modules: [
@@ -41,17 +40,16 @@ describe('features/copy-cut-paste/key-bindings', function() {
       DecisionRulesEditorModule,
       CopyCutPasteKeyBindingsModule,
       SelectionModule
-    ],
-    keyboard: {
-      bindTo: keyboardTarget
-    }
+    ]
   }));
 
 
   let testContainer;
+  let keyboardTarget;
 
   beforeEach(function() {
     testContainer = TestContainer.get(this);
+    keyboardTarget = testContainer.querySelector('.dmn-decision-table-container');
   });
 
 

diff --git a/packages/dmn-js-decision-table/test/spec/features/keyboard/KeyboardSpec.js b/packages/dmn-js-decision-table/test/spec/features/keyboard/KeyboardSpec.js
@@ -1,3 +1,5 @@
+/* global sinon */
+
 import {
   bootstrapModeler,
   inject
@@ -26,7 +28,6 @@ import {
 
 describe('features/keyboard', function() {
 
-  const keyboardTarget = document.createElement('div');
 
   beforeEach(bootstrapModeler(diagramXML, {
     modules: [
@@ -37,27 +38,42 @@ describe('features/keyboard', function() {
       DecisionRulesEditorModule,
       TablePropertiesEditorModule,
       SelectionModule
-    ],
-    keyboard: {
-      bindTo: keyboardTarget
-    }
+    ]
   }));
 
   describe('keyboard binding', function() {
 
+    let testContainer;
+
+    beforeEach(function() {
+      testContainer = TestContainer.get(this);
+    });
+
     it('should integrate with <attach> + <detach> events', inject(
-      function(keyboard, eventBus) {
+      function(eventBus) {
+
+        // given
+        const keyboardTarget =
+          testContainer.querySelector('.dmn-decision-table-container');
 
-        // assume
-        expect(keyboard._node).to.eql(keyboardTarget);
+        const bindSpy = sinon.spy();
+        eventBus.on('keyboard.bind', ({ node }) => bindSpy(node));
+
+        // when
+        eventBus.fire('attach');
+
+        // then
+        expect(bindSpy).to.have.been.calledOnceWith(keyboardTarget);
+
+        // and given
+        const unbindSpy = sinon.spy();
+        eventBus.on('keyboard.unbind', ({ node }) => unbindSpy(node));
 
         // when
         eventBus.fire('detach');
-        expect(keyboard._node).not.to.exist;
 
-        // but when
-        eventBus.fire('attach');
-        expect(keyboard._node).to.eql(keyboardTarget);
+        // then
+        expect(unbindSpy).to.have.been.calledOnceWith(keyboardTarget);
       }
     ));
 
@@ -73,7 +89,7 @@ describe('features/keyboard', function() {
     });
 
     beforeEach(inject(function(keyboard) {
-      keyboard.bind(testContainer);
+      keyboard.bind();
     }));
 
     it('should select cell below on <ENTER>', inject(function(cellSelection) {

diff --git a/packages/dmn-js-literal-expression/src/features/keyboard/Keyboard.js b/packages/dmn-js-literal-expression/src/features/keyboard/Keyboard.js
@@ -7,6 +7,11 @@ import {
   isShift
 } from './KeyboardUtil';
 
+var compatMessage =
+  'Keyboard binding is now implicit; explicit binding to an element got removed. ' +
+  'For more information, see https://github.com/bpmn-io/diagram-js/pull/662';
+
+
 /**
  * A keyboard abstraction that may be activated and
  * deactivated by users at will, consuming key events
@@ -22,16 +27,17 @@ import {
  *
  * All events contain the fields (node, listeners).
  *
- * A default binding for the keyboard may be specified via the
- * `keyboard.bindTo` configuration option.
+ * Specify the initial keyboard binding state via the
+ * `keyboard.bind=true|false` configuration option.
  *
  * @param {Config} config
  * @param {EventBus} eventBus
  * @param {EditorActions} editorActions
+ * @param {Renderer} renderer
  */
 export default class Keyboard {
 
-  constructor(config, eventBus, editorActions) {
+  constructor(config, eventBus, editorActions, renderer) {
 
     this._config = config || {};
     this._eventBus = eventBus;
@@ -45,7 +51,16 @@ export default class Keyboard {
     eventBus.on('attach', () => {
 
       if (this._config.bindTo) {
-        this.bind(config.bindTo);
+        console.error('unsupported configuration <keyboard.bindTo>',
+          new Error(compatMessage));
+      }
+
+      this._target = renderer.getContainer();
+
+      var bind = this._config && this._config.bind !== false;
+
+      if (bind) {
+        this.bind();
       }
     });
 
@@ -89,10 +104,14 @@ export default class Keyboard {
 
   bind(node) {
 
+    if (node) {
+      console.error('unsupported argument <node>', new Error(compatMessage));
+    }
+
     // make sure that the keyboard is only bound once to the DOM
     this.unbind();
 
-    this._node = node;
+    node = this._node = this._target;
 
     // bind key events
     domEvent.bind(node, 'keydown', this._keyHandler, true);
@@ -185,5 +204,6 @@ export default class Keyboard {
 Keyboard.$inject = [
   'config.keyboard',
   'eventBus',
-  'editorActions'
+  'editorActions',
+  'renderer'
 ];
diff --git a/packages/dmn-js/CHANGELOG.md b/packages/dmn-js/CHANGELOG.md
@@ -22,7 +22,7 @@ ___Note:__ Yet to be released changes appear here._
 
 ### Breaking Changes
 
-* Keyboard (DRD) is now implicit, and canvas is focusable, cf. [bpmn-io/diagram-js#662](https://github.com/bpmn-io/diagram-js/pull/662)
+* Keyboard (DRD) is now implicit, and canvas is focusable, cf. ([bpmn-io/diagram-js#662](https://github.com/bpmn-io/diagram-js/pull/662))
 
 ## 16.8.2