fix(moveFormField): assign new row when no row specified

Closes #861

packages/form-js-editor/src/features/modeling/FormLayoutUpdater.js

@@ -41,11 +41,23 @@ export default class FormLayoutUpdater extends CommandInterceptor {
 
     const { schema } = this._formEditor._getState();
 
+    const setRowIds = parent => {
+      if (!parent.components || !parent.components.length) {
+        return;
+      }
+
+      parent.components.forEach(formField => {
+        const row = this._formLayouter.getRowForField(formField);
+
+        updateRow(formField, row.id);
+
+        // handle children recursively
+        setRowIds(formField);
+      });
+    };
+
     // make sure rows are persisted in schema (e.g. for migration case)
-    schema.components.forEach(formField => {
-      const row = this._formLayouter.getRowForField(formField);
-      updateRow(formField, row.id);
-    });
+    setRowIds(schema);
   }
 
 }

packages/form-js-editor/src/features/modeling/cmd/MoveFormFieldHandler.js

@@ -15,11 +15,13 @@ export default class MoveFormFieldHandler {
    * @param { import('../../../FormEditor').default } formEditor
    * @param { import('../../../core/FormFieldRegistry').default } formFieldRegistry
    * @param { import('@bpmn-io/form-js-viewer').PathRegistry } pathRegistry
+   * @param { import('@bpmn-io/form-js-viewer').FormLayouter } formLayouter
    */
-  constructor(formEditor, formFieldRegistry, pathRegistry) {
+  constructor(formEditor, formFieldRegistry, pathRegistry, formLayouter) {
     this._formEditor = formEditor;
     this._formFieldRegistry = formFieldRegistry;
     this._pathRegistry = pathRegistry;
+    this._formLayouter = formLayouter;
   }
 
   execute(context) {
@@ -73,8 +75,8 @@ export default class MoveFormFieldHandler {
 
       const formField = get(schema, [ ...sourcePath, sourceIndex ]);
 
-      // (1) Add to row
-      updateRow(formField, targetRow ? targetRow.id : null);
+      // (1) Add to row or create new one
+      updateRow(formField, targetRow ? targetRow.id : this._formLayouter.nextRowId());
 
       // (2) Move form field
       arrayMove(get(schema, sourcePath), sourceIndex, targetIndex);
@@ -100,8 +102,8 @@ export default class MoveFormFieldHandler {
 
       const targetPath = [ ...targetFormField._path, 'components' ];
 
-      // (4) Add to row
-      updateRow(formField, targetRow ? targetRow.id : null);
+      // (4) Add to row or create new one
+      updateRow(formField, targetRow ? targetRow.id : this._formLayouter.nextRowId());
 
       // (5) Add form field
       arrayAdd(get(schema, targetPath), targetIndex, formField);
@@ -120,4 +122,4 @@ export default class MoveFormFieldHandler {
   }
 }
 
-MoveFormFieldHandler.$inject = [ 'formEditor', 'formFieldRegistry', 'pathRegistry' ];
+MoveFormFieldHandler.$inject = [ 'formEditor', 'formFieldRegistry', 'pathRegistry', 'formLayouter' ];

packages/form-js-editor/test/spec/FormEditor.spec.js

@@ -25,6 +25,7 @@ import {
 import schema from './form.json';
 import schemaNoIds from './form-no-ids.json';
 import schemaRows from './form-rows.json';
+import schemaGroup from './form-group.json';
 
 insertStyles();
 
@@ -1049,6 +1050,46 @@ describe('FormEditor', function() {
     });
 
 
+    it('should move form field into group', async function() {
+
+      // given
+      formEditor = await createFormEditor({
+        schema: schemaGroup,
+        container
+      });
+
+      const formFieldRegistry = formEditor.get('formFieldRegistry');
+
+      await expectDragulaCreated(formEditor);
+
+      // assume
+      expectLayout(formFieldRegistry.get('Textfield_1'), {
+        columns: 16,
+        row: 'Row_1'
+      });
+
+      const formField = container.querySelector('[data-id=Textfield_1]').parentNode;
+
+      const row = container.querySelector('[data-row-id=Row_3]');
+      const bounds = row.getBoundingClientRect();
+
+      // when
+      startDragging(container, formField);
+      moveDragging(container, {
+        clientX: bounds.x + 10,
+        clientY: bounds.y + bounds.height
+      });
+
+      endDragging(container);
+
+      // then
+      const textfield = formFieldRegistry.get('Textfield_1');
+
+      expect(textfield.layout.row).to.not.be.null;
+      expect(textfield.layout.columns).to.eql(16);
+    });
+
+
     it('should NOT move form field - invalid', async function() {
 
       // given

packages/form-js-editor/test/spec/features/modeling/Modeling.spec.js

@@ -659,6 +659,15 @@ describe('features/modeling', function() {
         }));
 
 
+        it('should NOT set empty row', inject(function(formFieldRegistry) {
+
+          // then
+          const formField = formFieldRegistry.get('Text_1');
+
+          expect(formField.layout.row).to.exist;
+        }));
+
+
         it('<undo>', inject(function(commandStack, formFieldRegistry) {
 
           // when
@@ -735,7 +744,6 @@ describe('features/modeling', function() {
 
         it('<do>', inject(function(formFieldRegistry) {
 
-
           // then
           expect(formFieldRegistry.getAll()).to.have.length(formFieldsLength);
 
@@ -750,6 +758,15 @@ describe('features/modeling', function() {
         }));
 
 
+        it('should NOT set empty row', inject(function(formFieldRegistry) {
+
+          // then
+          const formField = formFieldRegistry.get('GroupTextfield_1');
+
+          expect(formField.layout.row).to.exist;
+        }));
+
+
         it('<undo>', inject(function(commandStack, formFieldRegistry) {
 
           // when

packages/form-js-editor/test/spec/form-group.json

@@ -0,0 +1,39 @@
+{
+  "components": [
+    {
+      "id": "Textfield_1",
+      "key": "invoiceNumber",
+      "label": "Invoice Number",
+      "type": "textfield",
+      "layout": {
+        "row": "Row_1",
+        "columns": 16
+      }
+    },
+    {
+      "components": [
+        {
+          "label": "Text field",
+          "type": "textfield",
+          "layout": {
+            "row": "Row_3",
+            "columns": null
+          },
+          "id": "Textfield_2",
+          "key": "textfield_2"
+        }
+      ],
+      "showOutline": true,
+      "label": "Group",
+      "type": "group",
+      "layout": {
+        "row": "Row_2",
+        "columns": null
+      },
+      "id": "Group_1"
+    }
+  ],
+  "type": "default",
+  "id": "Form_1",
+  "schemaVersion": 11
+}

packages/form-js-playground/test/spec/form.json

@@ -2,7 +2,32 @@
   "components": [
     {
       "type": "text",
-      "text": "# Invoice\nLorem _ipsum_ __dolor__ `sit`.\n  \n  \nA list of BPMN symbols:\n* Start Event\n* Task\nLearn more about [forms](https://bpmn.io).\n  \n  \nThis [malicious link](javascript:throw onerror=alert,'some string',123,'haha') __should not work__."
+      "text": "# Invoice\nLorem _ipsum_ __dolor__ `sit`.\n  \n  \nA list of BPMN symbols:\n* Start Event\n* Task\nLearn more about [forms](https://bpmn.io).\n  \n  \nThis [malicious link](javascript:throw onerror=alert,'some string',123,'haha') __should not work__.",
+      "layout": {
+        "row": "Row_1",
+        "columns": 10
+      }
+    },
+    {
+      "id": "Group_1",
+      "type": "group",
+      "label": "Supplementary Information",
+      "path": "invoiceDetails",
+      "showOutline": true,
+      "components": [
+        {
+          "id": "GroupTextfield_1",
+          "type": "textfield",
+          "key": "supplementaryInfo1",
+          "label": "Field 1"
+        },
+        {
+          "id": "GroupTextfield_2",
+          "type": "textfield",
+          "key": "supplementaryInfo2",
+          "label": "Field 2"
+        }
+      ]
     },
     {
       "key": "creditor",