From 93549604837ae6e0f02c8ad3a47df1a6b883490d Mon Sep 17 00:00:00 2001 From: Niklas Kiefer Date: Tue, 24 Oct 2023 15:17:23 +0100 Subject: [PATCH] fix(moveFormField): assign new row when no row specified Closes #861 --- .../features/modeling/FormLayoutUpdater.js | 20 +++++++-- .../modeling/cmd/MoveFormFieldHandler.js | 14 ++++--- .../test/spec/FormEditor.spec.js | 41 +++++++++++++++++++ .../spec/features/modeling/Modeling.spec.js | 19 ++++++++- .../form-js-editor/test/spec/form-group.json | 39 ++++++++++++++++++ .../form-js-playground/test/spec/form.json | 27 +++++++++++- 6 files changed, 148 insertions(+), 12 deletions(-) create mode 100644 packages/form-js-editor/test/spec/form-group.json diff --git a/packages/form-js-editor/src/features/modeling/FormLayoutUpdater.js b/packages/form-js-editor/src/features/modeling/FormLayoutUpdater.js index 1ff2beb7b..75c53092f 100644 --- a/packages/form-js-editor/src/features/modeling/FormLayoutUpdater.js +++ b/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); } } diff --git a/packages/form-js-editor/src/features/modeling/cmd/MoveFormFieldHandler.js b/packages/form-js-editor/src/features/modeling/cmd/MoveFormFieldHandler.js index 1534bde2b..8110a5a44 100644 --- a/packages/form-js-editor/src/features/modeling/cmd/MoveFormFieldHandler.js +++ b/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' ]; \ No newline at end of file +MoveFormFieldHandler.$inject = [ 'formEditor', 'formFieldRegistry', 'pathRegistry', 'formLayouter' ]; \ No newline at end of file diff --git a/packages/form-js-editor/test/spec/FormEditor.spec.js b/packages/form-js-editor/test/spec/FormEditor.spec.js index 7ea3f2013..cc8114476 100644 --- a/packages/form-js-editor/test/spec/FormEditor.spec.js +++ b/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 diff --git a/packages/form-js-editor/test/spec/features/modeling/Modeling.spec.js b/packages/form-js-editor/test/spec/features/modeling/Modeling.spec.js index 13abdf859..1bb26673b 100644 --- a/packages/form-js-editor/test/spec/features/modeling/Modeling.spec.js +++ b/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('', inject(function(commandStack, formFieldRegistry) { // when @@ -735,7 +744,6 @@ describe('features/modeling', function() { it('', 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('', inject(function(commandStack, formFieldRegistry) { // when diff --git a/packages/form-js-editor/test/spec/form-group.json b/packages/form-js-editor/test/spec/form-group.json new file mode 100644 index 000000000..620012250 --- /dev/null +++ b/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 +} \ No newline at end of file diff --git a/packages/form-js-playground/test/spec/form.json b/packages/form-js-playground/test/spec/form.json index 2181435c5..d6ea64ee9 100644 --- a/packages/form-js-playground/test/spec/form.json +++ b/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",