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