From 93bbe50ed1b9a42b4346af9de5063991722c496a Mon Sep 17 00:00:00 2001 From: Kevin Chappell Date: Thu, 1 Oct 2026 09:58:08 +0100 Subject: [PATCH 1/3] fix(renderer): clone input groups whatever the component ids look like The renderer cached each component under baseId() of its rendered id, which only strips the f- prefix when the id holds an 8-hex or UUID run, while "Add +" looked the row up by its raw id. Readable ids such as row-1 (the renderer docs' own example) missed the cache and the click threw "Cannot read properties of undefined (reading 'children')". Cache components under their rendered id and look rows and their children up by that same id. Name lookups for userFormData keep resolving both readable and generated names. Refs #520 --- src/lib/js/renderer/index.js | 17 +++++++--- src/lib/js/renderer/layout-attrs.test.js | 1 - src/lib/js/renderer/option-groups.test.js | 3 -- src/lib/js/renderer/renderer.test.js | 41 +++++++++++++++++++++++ tests/renderer-input-group-ids.spec.js | 37 ++++++++++++++++++++ 5 files changed, 90 insertions(+), 9 deletions(-) create mode 100644 tests/renderer-input-group-ids.spec.js diff --git a/src/lib/js/renderer/index.js b/src/lib/js/renderer/index.js index 9c0d023d..c3d1dbea 100644 --- a/src/lib/js/renderer/index.js +++ b/src/lib/js/renderer/index.js @@ -185,7 +185,9 @@ export default class FormeoRenderer { */ componentByName(name) { return ( - this.components[baseId(name)] || + // a generated name starts with the rendered id: `f-{id}` or `f-{id}-{label}` + this.components[name] || + this.components[this.prefixId(baseId(name))] || Object.values(this.components).find( component => component.attrs?.name === name || component.attrs?.name === `${name}[]` ) || @@ -490,8 +492,13 @@ export default class FormeoRenderer { return acc }, []) + /** + * Caches a component under its rendered id (`f-{id}`), so input group clones can find it whatever the id looks like + * @param {Object} data processed component data + * @return {Object} data + */ cacheComponent = data => { - this.components[baseId(data.id)] = data + this.components[data.id] = data return data } @@ -516,7 +523,7 @@ export default class FormeoRenderer { const configConditions = [ { condition: config.legend, result: () => ({ tag: config.fieldset ? 'legend' : 'h3', children: config.legend }) }, { condition: true, result: () => rowData }, - { condition: config.inputGroup, result: () => this.addButton(id) }, + { condition: config.inputGroup, result: () => this.addButton(rowData.id) }, ] const children = configConditions.reduce((acc, { condition, result }) => { @@ -551,7 +558,7 @@ export default class FormeoRenderer { return { ...rest, id: RENDER_PREFIX + uuid(id), - children: children?.length && children.map(({ id }) => this.cloneComponentData(baseId(id))), + children: children?.length && children.map(({ id }) => this.cloneComponentData(id)), attrs: updatedAttrs, } } @@ -600,7 +607,7 @@ export default class FormeoRenderer { stage.children = this.processRows(stage.id) stage.className = STAGE_CLASSNAME - this.components[baseId(stage.id)] = stage + this.components[this.prefixId(stage.id)] = stage return stage }) } diff --git a/src/lib/js/renderer/layout-attrs.test.js b/src/lib/js/renderer/layout-attrs.test.js index 15d64de3..5e9e5316 100644 --- a/src/lib/js/renderer/layout-attrs.test.js +++ b/src/lib/js/renderer/layout-attrs.test.js @@ -167,7 +167,6 @@ describe('row and column attributes in the renderer (#112)', () => { }) test('an input-group clone keeps the attributes and gets its own ids', () => { - // clone lookup goes through baseId(), which only recognises editor-style hex ids render({ id: 'form-la', stages: { '0a0a0a0a': { id: '0a0a0a0a', children: ['1b1b1b1b'] } }, diff --git a/src/lib/js/renderer/option-groups.test.js b/src/lib/js/renderer/option-groups.test.js index 978a5522..389fe2a1 100644 --- a/src/lib/js/renderer/option-groups.test.js +++ b/src/lib/js/renderer/option-groups.test.js @@ -339,7 +339,6 @@ describe('checkbox and radio groups', () => { describe('input group clones', () => { test('a cloned radio group gets its own name so it does not share a selection with the original', () => { - // clone lookup goes through baseId(), which only recognises editor-style hex ids const formData = { id: 'clone-form', stages: { '0a0a0a0a': { id: '0a0a0a0a', children: ['1b1b1b1b'] } }, @@ -358,7 +357,6 @@ describe('checkbox and radio groups', () => { }) test('a cloned select keeps its configured name (selects also carry top-level options)', () => { - // clone lookup goes through baseId(), which only recognises editor-style hex ids const selectField = (id, attrs = {}) => ({ id, tag: 'select', @@ -591,7 +589,6 @@ describe('checkbox and radio groups', () => { }) test('a repeated input group gets its own Other text box', () => { - // clone lookup goes through baseId(), which only recognises editor-style hex ids const formData = { id: 'clone-form', stages: { '0a0a0a0a': { id: '0a0a0a0a', children: ['1b1b1b1b'] } }, diff --git a/src/lib/js/renderer/renderer.test.js b/src/lib/js/renderer/renderer.test.js index 1a5a2fd3..3581f90b 100644 --- a/src/lib/js/renderer/renderer.test.js +++ b/src/lib/js/renderer/renderer.test.js @@ -1254,4 +1254,45 @@ describe('FormeoRenderer', () => { assert.ok(renderForm(data).elements['hidden-1532560563828']) }) }) + + describe('input group clones with any id format (#520)', () => { + const inputGroupForm = ({ rowId, columnId, fieldId }) => ({ + id: 'form-1', + stages: { s1: { id: 's1', children: [rowId] } }, + rows: { [rowId]: { id: rowId, config: { inputGroup: true }, children: [columnId] } }, + columns: { [columnId]: { id: columnId, config: { width: '100%' }, children: [fieldId] } }, + fields: { + [fieldId]: { id: fieldId, tag: 'input', attrs: { type: 'text', name: 'n' }, config: { label: 'Name' } }, + }, + }) + + const assertAddsCopy = ids => { + new FormeoRenderer({ renderContainer: container }).render(inputGroupForm(ids)) + assert.doesNotThrow(() => container.querySelector('.add-input-group').click()) + + const copy = container.querySelector(`[data-clone-of="f-${ids.rowId}"]`) + assert.ok(copy, 'the row was copied') + assert.ok(copy.querySelector(`[data-clone-of="f-${ids.columnId}"]`), 'the copy has its column') + assert.ok(copy.querySelector(`input[data-clone-of="f-${ids.fieldId}"]`), 'the copy has its field') + assert.equal(container.querySelectorAll('input[name="n"]').length, 2) + const domIds = [...container.querySelectorAll('[id]')].map(elem => elem.id) + assert.equal(new Set(domIds).size, domIds.length, 'no duplicate ids') + } + + test('clones a row whose ids are readable, as in the renderer docs', () => { + assertAddsCopy({ rowId: 'row-1', columnId: 'col-1', fieldId: 'field-1' }) + }) + + test('clones a row whose readable ids share an 8-character hex segment', () => { + assertAddsCopy({ rowId: 'row-deadbeef', columnId: 'col-deadbeef', fieldId: 'field-deadbeef' }) + }) + + test('clones a row whose ids start with the render prefix', () => { + assertAddsCopy({ rowId: 'f-row', columnId: 'f-col', fieldId: 'f-field' }) + }) + + test('still clones a row with editor-style hex ids', () => { + assertAddsCopy({ rowId: '1b1b1b1b', columnId: '2c2c2c2c', fieldId: '3d3d3d3d' }) + }) + }) }) diff --git a/tests/renderer-input-group-ids.spec.js b/tests/renderer-input-group-ids.spec.js new file mode 100644 index 00000000..34549989 --- /dev/null +++ b/tests/renderer-input-group-ids.spec.js @@ -0,0 +1,37 @@ +import { expect, test } from '@playwright/test' + +test.describe('renderer input group with readable ids (#520)', () => { + test('"Add +" copies the row when ids are not hex', async ({ page }) => { + const errors = [] + page.on('pageerror', error => errors.push(error.message)) + + await page.goto('/') + await expect(page.locator('.formeo-editor')).toBeVisible() + + await page.evaluate(() => { + const container = Object.assign(document.createElement('div'), { id: 'ig-container' }) + document.body.prepend(container) + new window.FormeoRenderer({ renderContainer: '#ig-container' }).render({ + id: 'form-1', + stages: { s1: { id: 's1', children: ['row-1'] } }, + rows: { 'row-1': { id: 'row-1', config: { inputGroup: true }, children: ['col-1'] } }, + columns: { 'col-1': { id: 'col-1', config: { width: '100%' }, children: ['field-1'] } }, + fields: { + 'field-1': { + id: 'field-1', + tag: 'input', + attrs: { type: 'text', name: 'n' }, + config: { label: 'Name' }, + }, + }, + }) + }) + + const container = page.locator('#ig-container') + await container.getByRole('button', { name: 'Add +' }).click() + + await expect(container.locator('[data-clone-of="f-row-1"]')).toHaveCount(1) + await expect(container.locator('input[name="n"]')).toHaveCount(2) + expect(errors).toEqual([]) + }) +}) From 3ff1536816bee5ddbedc80ef583e54ae6e4cb2af Mon Sep 17 00:00:00 2001 From: Kevin Chappell Date: Thu, 1 Oct 2026 10:02:56 +0100 Subject: [PATCH 2/3] fix(editor): show unset row Fieldset and Input group settings as unchecked A row loaded with a config that leaves fieldset or inputGroup unset (config: {}, or a partial config) replaces the row's default config, so the Settings checkboxes got checked: undefined. dom renders an undefined attribute value as an empty string, so the box showed as checked and the first click saved false. Coerce both values to booleans. Saved formData is unchanged: the missing keys are still not added on load. Refs #521 --- src/lib/js/components/rows/row.js | 5 +- src/lib/js/components/rows/row.test.js | 50 +++++++++++++++++++ tests/row-settings-checkboxes.spec.js | 68 ++++++++++++++++++++++++++ 3 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 tests/row-settings-checkboxes.spec.js diff --git a/src/lib/js/components/rows/row.js b/src/lib/js/components/rows/row.js index c9e75060..ec72897a 100644 --- a/src/lib/js/components/rows/row.js +++ b/src/lib/js/components/rows/row.js @@ -103,7 +103,8 @@ export default class Row extends Component { id: `${this.id}-fieldset`, attrs: { type: 'checkbox', - checked: this.get('config.fieldset'), + // a loaded `config` may leave this unset, and an undefined `checked` renders as checked (#521) + checked: Boolean(this.get('config.fieldset')), ariaLabel: i18n.get('row.settings.fieldsetWrap.aria'), }, action: { @@ -121,7 +122,7 @@ export default class Row extends Component { id: `${this.id}-inputGroup`, attrs: { type: 'checkbox', - checked: this.get('config.inputGroup'), + checked: Boolean(this.get('config.inputGroup')), ariaLabel: i18n.get('row.settings.inputGroup.aria'), }, action: { diff --git a/src/lib/js/components/rows/row.test.js b/src/lib/js/components/rows/row.test.js index 54b06d8f..10bae7d1 100644 --- a/src/lib/js/components/rows/row.test.js +++ b/src/lib/js/components/rows/row.test.js @@ -202,3 +202,53 @@ describe('Row edit panels (#112)', () => { assert.equal(editorComponents.columns.get('c-1').dom.querySelector('.panel-nav'), columnNav) }) }) + +describe('Row Settings checkboxes (#521)', () => { + afterEach(() => { + for (const node of mounted.splice(0)) { + node.remove() + } + }) + + // the Settings panel renders on the next frame + const checkboxes = async row => { + await nextFrames() + return { + fieldset: row.dom.querySelector('#r-1-fieldset'), + inputGroup: row.dom.querySelector('#r-1-inputGroup'), + } + } + + it('an unset fieldset or input group shows unchecked', async () => { + const { fieldset, inputGroup } = await checkboxes(rowOf(editorWith({ row: { config: {} } }))) + assert.equal(fieldset.checked, false) + assert.equal(inputGroup.checked, false) + }) + + it('a config with only fieldset leaves Input group unchecked', async () => { + const { fieldset, inputGroup } = await checkboxes(rowOf(editorWith({ row: { config: { fieldset: true } } }))) + assert.equal(fieldset.checked, true) + assert.equal(inputGroup.checked, false) + }) + + it('a config with only inputGroup leaves Fieldset unchecked', async () => { + const { fieldset, inputGroup } = await checkboxes(rowOf(editorWith({ row: { config: { inputGroup: true } } }))) + assert.equal(fieldset.checked, false) + assert.equal(inputGroup.checked, true) + }) + + it('the first click on an unset checkbox turns the setting on', async () => { + const editorComponents = editorWith({ row: { config: {} } }) + const row = rowOf(editorComponents) + const { fieldset, inputGroup } = await checkboxes(row) + fieldset.click() + inputGroup.click() + assert.equal(row.get('config.fieldset'), true) + assert.equal(row.get('config.inputGroup'), true) + }) + + it('loading an unset config saves it unchanged', () => { + const editorComponents = editorWith({ row: { config: {} } }) + assert.deepEqual(editorComponents.formData.rows['r-1'].config, {}) + }) +}) diff --git a/tests/row-settings-checkboxes.spec.js b/tests/row-settings-checkboxes.spec.js new file mode 100644 index 00000000..85da657d --- /dev/null +++ b/tests/row-settings-checkboxes.spec.js @@ -0,0 +1,68 @@ +// @ts-check +import { expect, test } from '@playwright/test' + +/** + * The issue's form: row `r1` with the given config, one empty column + * @param {Object} config + */ +const formWithRowConfig = config => ({ + id: 'form-1', + stages: { s1: { id: 's1', children: ['r1'] } }, + rows: { r1: { id: 'r1', config, children: ['c1'] } }, + columns: { c1: { id: 'c1', config: { width: '100%' }, children: [] } }, + fields: {}, +}) + +/** + * Mounts an editor in #e2e-rs (above the demo editor) and opens row r1's edit window. The editor is window.__editor. + * @param {import('@playwright/test').Page} page + * @param {Object} config row r1's config + */ +const openRowSettings = async (page, config) => { + await page.goto('/') + await expect(page.locator('.formeo-editor').first()).toBeVisible() + await page.evaluate(async formData => { + const container = Object.assign(document.createElement('div'), { id: 'e2e-rs' }) + document.body.prepend(container) + window.__editor = new window.FormeoEditor( + { editorContainer: container, sessionStorage: false, style: null }, + formData + ) + await window.__editor.whenReady() + }, formWithRowConfig(config)) + + const row = page.locator('#e2e-rs .formeo-row').first() + // the empty column fills the row, so hover the row's own edge + await row.hover({ position: { x: 2, y: 2 } }) + await row.locator('.row-actions').first().hover() + await row.locator('.row-actions .edit-toggle').first().click() + const editWindow = row.locator('.row-edit').first() + await expect(editWindow).toBeVisible() + return { fieldset: editWindow.locator('#r1-fieldset'), inputGroup: editWindow.locator('#r1-inputGroup') } +} + +const rowConfigOf = page => page.evaluate(() => window.__editor.formData.rows.r1.config) + +test.describe('row Settings checkboxes with an unset config (#521)', () => { + test('config: {} shows Fieldset and Input group unchecked', async ({ page }) => { + const { fieldset, inputGroup } = await openRowSettings(page, {}) + await expect(fieldset).not.toBeChecked() + await expect(inputGroup).not.toBeChecked() + expect(await rowConfigOf(page)).toEqual({}) + }) + + test('the first click turns an unset setting on', async ({ page }) => { + const { fieldset, inputGroup } = await openRowSettings(page, {}) + await fieldset.click() + await inputGroup.click() + await expect(fieldset).toBeChecked() + await expect(inputGroup).toBeChecked() + await expect.poll(() => rowConfigOf(page)).toEqual({ fieldset: true, inputGroup: true }) + }) + + test('a partial config checks only the setting it holds', async ({ page }) => { + const { fieldset, inputGroup } = await openRowSettings(page, { fieldset: true }) + await expect(fieldset).toBeChecked() + await expect(inputGroup).not.toBeChecked() + }) +}) From dbf1de4a39fc5b804ee43f1aa5d4720934a924d9 Mon Sep 17 00:00:00 2001 From: Kevin Chappell Date: Thu, 1 Oct 2026 10:04:11 +0100 Subject: [PATCH 3/3] style(tests): drop unused variables from two e2e specs --- tests/editor-initialization.spec.js | 5 ----- tests/events.spec.js | 5 +---- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/tests/editor-initialization.spec.js b/tests/editor-initialization.spec.js index 043c2c85..66d1c70b 100644 --- a/tests/editor-initialization.spec.js +++ b/tests/editor-initialization.spec.js @@ -1,11 +1,6 @@ // @ts-check import { expect, test } from '@playwright/test' -/** - * Helper to get the editor instance from the demo page - */ -const getEditor = () => window.frameworkLoader?.currentDemo?.editor - test.describe('Editor Initialization', () => { test.beforeEach(async ({ page }) => { await page.goto('/') diff --git a/tests/events.spec.js b/tests/events.spec.js index 4da415ba..eb70e3c8 100644 --- a/tests/events.spec.js +++ b/tests/events.spec.js @@ -92,10 +92,7 @@ test.describe('Formeo Events System', () => { // Verify we can add a listener for this event type const listenerAdded = await page.evaluate(() => { - let called = false - const handler = () => { - called = true - } + const handler = () => {} document.addEventListener('formeoUpdatedField', handler) // Clean up document.removeEventListener('formeoUpdatedField', handler)