fix: input group clones with any id format, and unset row Settings checkboxes

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: {

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, {})
+  })
+})

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
     })
   }

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'] } },

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'] } },

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' })
+    })
+  })
 })

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('/')

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)

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([])
+  })
+})

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()
+  })
+})