Skip to content

fix: stop rows and columns rendering their editor label - #484

Merged
kevinchappell merged 2 commits into
Draggable:mainfrom
Webotvurci-s-r-o:fix/layout-container-labels
Aug 8, 2026
Merged

fix: stop rows and columns rendering their editor label#484
kevinchappell merged 2 commits into
Draggable:mainfrom
Webotvurci-s-r-o:fix/layout-container-labels

Conversation

@tomeus

@tomeus tomeus commented Aug 8, 2026

Copy link
Copy Markdown

What

Dragging a row or column control onto the stage stores the caption of the control's button as the layout container's own label. The rendered form then prints it:

<div class="formeo-row-wrap">
  <div class="">
    <label for="f-bcd2ebd9">row</label>
    <div class="formeo-row" id="f-bcd2ebd9"></div>
  </div>
</div>

Every such row renders <label>row</label> and every column <label>column</label>, visible to whoever fills the form in.

Why it happens

config.label on a layout control is the caption of its button in the controls panel (Control.dom renders it as .control-label), not data for the component the control creates. The drop handler in Component.onAdd strips only meta off the control payload and hands the rest to addChild:

const { controlData: { meta: { id: metaId }, ...elementData } } = Controls.get(item.id)
set(elementData, 'config.controlId', metaId)
...
const component = action(elementData, newIndex)   // elementData = { config: { label: 'row', controlId } }

So the caption lands in the row's/column's stored config, and dom.create renders a label whenever config.label is set and config.hideLabel is not.

That is exactly how it reproduces:

Action Result
drag row onto the stage the row stores label: 'row'
drag column onto the stage its wrapping row is built from defaults and stays clean, the column stores label: 'column'
drag a field onto the stage row and column are both built from defaults → no label ✓
add a field to an already dragged-in row the row's label stays ❌

Clicking a layout control instead of dragging it already produced clean data — Controls.addElement discards the payload for the layout group — so the two paths disagreed with each other.

Two further consequences beyond the stray label:

  • a dragged-in row lost its default config (fieldset, legend, inputGroup): the control's config replaces the defaults wholesale in the shallow spread the Row constructor does
  • the condition builder resolves a component's name from config.label before config.legend, so such a row is offered as "row" when picking a condition target

Fix

Layout controls now contribute no data to the row or column they create — the container starts from its own defaults, the same result the click path already produced. Field controls keep passing their data through unchanged.

The renderer is deliberately left alone. Suppressing the label at render time would have hidden the symptom while every newly built form kept storing it, and would have overridden hideLabel for layout containers for good. Forms already saved with the bad data need a one-off cleanup of config.label on their rows and columns.

Tests

src/lib/js/components/layout-controls.test.js drives the drop handler the way Sortable does:

  • a dropped row keeps its own defaults and stores no label
  • a dropped column keeps its own defaults and stores no label
  • a dropped field control still brings its own config.label and config.controlId

The first two fail without the change.

A row and a column carry a config.label of "row" and "column" that the editor
puts there for the stage. The renderer passed it straight to dom.create, which
renders a label whenever one is set and hideLabel is not, so every rendered form
grew a stray <label>row</label> above each row and <label>column</label> above
each column.
@kevinchappell

Copy link
Copy Markdown
Collaborator

Can you give steps to reproduce this? is it possibly a data issue? I tried to replicate the problem in the demo but I am not seeing the rendered labels:
image

@tomeus

tomeus commented Aug 8, 2026

Copy link
Copy Markdown
Author

Can you give steps to reproduce this? is it possibly a data issue? I tried to replicate the problem in the demo but I am not seeing the rendered labels: image

Try it out at https://draggable.github.io/formeo/ where I simply add rows or columns and then click the “Render form” button. But if the row is created by adding an element, that element doesn't have it. If you start with a row and add an element to it, the label remains.

Screenshot 2026-08-08 at 21 39 45

@kevinchappell

Copy link
Copy Markdown
Collaborator

Try it out at https://draggable.github.io/formeo/ where I simply add rows or columns and then click the “Render form” button. But if the row is created by adding an element, that element doesn't have it. If you start with a row and add an element to it, the label remains.

Screenshot 2026-08-08 at 21 39 45

I think I am missing something.

Screencast.From.2026-08-08.15-51-49.mp4

@tomeus

tomeus commented Aug 8, 2026

Copy link
Copy Markdown
Author

Try it out at https://draggable.github.io/formeo/ where I simply add rows or columns and then click the “Render form” button. But if the row is created by adding an element, that element doesn't have it. If you start with a row and add an element to it, the label remains.
Screenshot 2026-08-08 at 21 39 45

I think I am missing something.

Screencast.From.2026-08-08.15-51-49.mp4

I really don't get it. I guess the only difference is dark mode.

https://github.com/user-attachments/assets/3b4f64e5-b9e9-4f2e-82bf-9d6d5bcae635
https://github.com/user-attachments/assets/2c46edff-125c-49b1-8f76-136bbf098b71

Comment thread src/lib/js/renderer/index.js Outdated
* @param {Object} config
* @return {Object} config that renders no label of its own
*/
const layoutConfig = (config = {}) => ({ ...config, hideLabel: true })

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this override the label for input groups? There are cases where we want to row and column to have a label but if this default to hide then they may not appear. Say for example you have an input group for contacts, without the label it might not be clear. The editor could add another row just for the label but then would lose the fieldset capability.

Image

@kevinchappell

kevinchappell commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

I really don't get it. I guess the only difference is dark mode.

can you try clearing the page storage? maybe something carried over from a previous config? I'm not able to replicate this bug. This is unique for sure.

edit: just saw your videos. are you on safari? it may be safari related. it should still be fixed, just need to pin down what safari is doing differently.

@tomeus

tomeus commented Aug 8, 2026

Copy link
Copy Markdown
Author

I really don't get it. I guess the only difference is dark mode.

can you try clearing the page storage? maybe something carried over from a previous config? I'm not able to replicate this bug. This is unique for sure.

edit: just saw your videos. are you on safari? it may be safari related. it should still be fixed, just need to pin down what safari is doing differently.

I really don't understand this situation. I'm on Chrome Version 151.0.7922.108 (Official Build) (arm64), MacBook M2 Pro with macOs 26.5.1. I opened it in an incognito window, but the behavior remained the same. If I fill in the row title, I see both.
Screenshot 2026-08-08 at 22 19 53

edit:
When I use edit data button

{
  "id": "9f4c660b",
  "stages": {
    "6c764dd2": {
      "conditions": [
        {
          "if": [
            {
              "source": "",
              "sourceProperty": "",
              "comparison": "",
              "target": "",
              "targetProperty": ""
            }
          ],
          "then": [
            {
              "target": "",
              "targetProperty": "",
              "assignment": "",
              "value": ""
            }
          ]
        }
      ],
      "children": [
        "6ff85dd0",
        "19f05290",
        "f968b582"
      ],
      "id": "6c764dd2"
    }
  },
  "rows": {
    "6ff85dd0": {
      "config": {
        "label": "row",
        "controlId": "layout-row"
      },
      "children": [],
      "className": [
        "formeo-row"
      ],
      "id": "6ff85dd0"
    },
    "f968b582": {
      "config": {
        "label": "row",
        "controlId": "layout-row"
      },
      "children": [
        "25b9f416"
      ],
      "className": [
        "formeo-row"
      ],
      "id": "f968b582"
    },
    "19f05290": {
      "config": {
        "label": "row",
        "controlId": "layout-row",
        "legend": "User wants to have title"
      },
      "children": [],
      "className": [
        "formeo-row"
      ],
      "id": "19f05290"
    }
  },
  "columns": {
    "25b9f416": {
      "config": {
        "label": "column",
        "controlId": "layout-column",
        "width": "100%"
      },
      "children": [
        "16b01f70"
      ],
      "className": [
        "formeo-column"
      ],
      "id": "25b9f416"
    }
  },
  "fields": {
    "16b01f70": {
      "tag": "input",
      "attrs": {
        "required": false,
        "type": "text",
        "className": ""
      },
      "config": {
        "label": "Text Input",
        "hideLabel": false,
        "controlId": "text-input"
      },
      "id": "16b01f70"
    }
  }
}

@tomeus

tomeus commented Aug 8, 2026

Copy link
Copy Markdown
Author

Claude identified it D&D vs. click

Verdict

The fix works (the symptom disappears), but it treats the effect, not the cause. The reproduction description matches the actual root cause exactly — and that cause lives in the editor, not the renderer.

Where the problem actually originates

The config.label: 'row' / 'column' in src/lib/js/components/controls/layout/row.js:3 and column.js:3 is the caption of the button in the controls panel (control.js:34const controlLabel = this.i18n(config.label) || config.label). It is not component data.

The drag & drop handler copies it into the data of the created component — src/lib/js/components/component.js:560-575:

const { controlData: { meta: { id: metaId }, ...elementData } } = Controls.get(item.id)
set(elementData, 'config.controlId', metaId)
...
const component = action(elementData, newIndex)   // ← elementData = { config: { label: 'row', controlId } }

That explains all three branches of the report:

Action Path Result
drag row onto stage depth 0stage.addChild(elementData) row gets config.label: 'row'
drag column onto stage depth -1 → row from addChild({}), column from elementData row clean, column has label: 'column'
drag field onto stage depth -2addChild({}).addChild() row and column created from {}no label
add a field to an existing row row data unchanged label stays ✓

Extra evidence that this is a bug: clicking a control (instead of dragging) goes through controls/index.js:352-374, where elementData for layout controls is discarded (this.layoutTypes[...]() is called with no arguments) → no label. Two paths, two different outcomes.

What the renderer fix does not solve

  1. Polluted formData — exported JSON still contains config.label: "row". Consumers who render the definition themselves still hit the problem.
  2. Lost row config defaults — the Row constructor does a shallow spread (row.js:31, { ...DEFAULT_DATA(), ...rowData }), so the control's config replaces the whole default config → a drag-dropped row has no fieldset, legend, or inputGroup.
  3. Condition autocompleteautocomplete/helpers.mjs:23,34 puts config.label before config.legend and name in the resolution order, so such a row is offered as "row" as a condition target instead of its legend / "Row".

Recommendation

Keep the renderer guard (it is defensive and repairs already-saved forms that contain the bad data), but add a fix for the root cause in component.js:

const isLayout = metaId.startsWith('layout-')
const controlType = isLayout ? metaId.replace(/^layout-/, '') : 'field'
// layout controls carry only the panel button caption, no component data
const componentData = isLayout ? {} : elementData
...
const component = action(componentData, newIndex)

This aligns the drag path with the click path and eliminates points 1–3. Worth adding an editor-level test asserting that a dropped row/column has no config.label in its data — the current test only covers the renderer output.

Minor note on the fix itself: layoutConfig(config = {}) handles undefined, but processColumn right below it still reads columnData.config.width without a guard (renderer/index.js:183) — pre-existing behavior, just inconsistent.

A layout control's config.label is the caption of its button in the controls
panel — "row", "column". The drag-drop handler passed the whole control payload
to the component it created, so a dropped row or column stored that caption as
its own label: the renderer printed <label>row</label> above every row, and the
condition builder offered "row" as the component's name. A dropped row also lost
its default config, because the control's config replaced it wholesale.

Layout controls now contribute no data to the row or column they create, which
starts from its own defaults — the same result the click path already produced.
bad label are cleaned up on the consumer side.

@kevinchappell kevinchappell left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good find and fix.

@kevinchappell
kevinchappell merged commit 5d3eef3 into Draggable:main Aug 8, 2026
2 checks passed
@kevinchappell

Copy link
Copy Markdown
Collaborator

🎉 This PR is included in version 5.1.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants