feat: multi-column form layout with column stacking#119
Merged
Conversation
Add row_index and column_index Int fields to FormField schema to support side-by-side field placement. Both fields are layout-only and excluded from DocType sync via to_frappe_field. Includes backfill patch (direct SQL, idempotent) and tests covering sync regression and patch correctness.
Add row_index/column_index to FormField type. Add moveField, insertNewRow, and compact helpers to editForm store; update addField, addFieldFromDoctype, and removeField to maintain layout invariants. Extract useGroupedRows as a shared composable for use in both the builder and submission renderer.
…lpers Add null guard to addFieldFromDoctype matching addField's pattern. Add fs.includes() guard to moveField and insertNewRow to prevent corrupting layout when a stale field ref is passed. Extract lastRowIndex helper using reduce to avoid spread-on-large-array.
Replace flat vuedraggable list with row/column grid derived from useGroupedRows. Fields with the same row_index render side-by-side as flex items. Extract FieldCard.vue from FormBuilderContent. Drag-and-drop temporarily removed; restored in next chunk.
Key rows by row_index value instead of array index to prevent wrong-row DOM patching on delete. Key FieldCard by row_index+column_index instead of idx which is undefined on new fields before save. Restore gap-3 between rows to match previous my-3 spacing.
Replace flat draggable with per-row inner draggables using group="fields" for cross-row drag. onFieldChange handles evt.moved (within-row reorder via direct column_index renumber) and evt.added (cross-row via moveField). Source row evt.removed is a no-op; target evt.added owns the move.
- RowDropZone: thin drop target between rows; expands on drag-start so fields can be inserted between existing rows instead of merging as a new column. Uses an empty vuedraggable list with put=true/pull=false and clears its buffer on nextTick after emitting. - Eject to own row: SquareSplitVertical button appears on FieldActions when a field shares its row with at least one other field. Clicking calls insertNewRow(field, row_index + 1), pushing the field to a new row immediately below. - Code-review fixes: extract rowIndexOf() helper (eliminates repeated row[0]?.row_index ?? rIdx), remove outer wrapper div (key + classes moved to draggableComponent via <template v-for>), oldIdx guard now returns early instead of silently skipping the splice. Note: cross-row visual swap requires the SortableJS Swap plugin which is not bundled in vuedraggable 4.x — within-row reorder already behaves correctly as a swap.
…en dragging last row RowDropZone: MutationObserver watches for SortableJS placeholder insertion to detect hover state (more reliable than pointer events during drag). Zone expands and shows dashed border while dragging; turns blue when hovered. FormBuilderContent: track draggingFromRow per @start event. Bottom drop zone is hidden when the field being dragged is already from the last row (dropping it there would be a no-op).
Group submission fields by row_index/column_index via useGroupedRows. Rows render flex-col on mobile, flex-row on md+. Use v-if on row + field so conditionally hidden fields are unmounted (clears stale v-model state and skips reqd computation). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add cell_index to FormField so multiple fields can stack vertically inside a single column. Sort key becomes (row_index, column_index, cell_index). Drag a field onto another column's cell list to drop above/below existing cells; ColumnDropZone (vertical strip between columns) handles new-column creation. - Backend: cell_index Int + regenerated types + extended layout-not- synced test - Composable: useGroupedRows returns FormField[][][] (rows -> cols -> cells) - Store: 3-axis compact preserves multi-cell column grouping by remapping distinct column_index values per row; new insertCell; moveField/insertNewRow/addField set cell_index = 0 - FormBuilderContent: row -> column -> cell render with ColumnDropZone between columns; per-column draggable cell list (group "fields") - FormRenderer: 3D render with v-if visibility per row/column/cell CellDropZone deemed redundant — vuedraggable's native cell-list insertion already shows the indicator and computes newIndex. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds row/column/cell indices to FormField, a post-model-sync backfill patch, backend tests, frontend types/store/composable, builder drop-zone/components, FormBuilder/FormRenderer integration, and E2E helpers/specs to enable grouped row/column/cell layouts and drag-and-drop editing. ChangesMulti-Column Form Layout Support
Sequence DiagramsequenceDiagram
participant User as User / FormBuilder
participant Component as FormBuilderContent
participant Zone as RowDropZone / ColumnDropZone
participant Store as editForm Store
participant Compact as compact()
participant Render as Template Re-render
User->>Component: Drag field over row/column drop zone
activate Component
Component->>Zone: Dragging signal (isDragging=true)
activate Zone
Zone->>Zone: MutationObserver detects placeholder
Zone->>Zone: isHighlighted = true
deactivate Zone
User->>Zone: Drop field into ColumnDropZone
activate Zone
Zone->>Component: Emit drop(field, atRow, atCol)
deactivate Zone
Component->>Store: insertCell(field, targetRow, targetCol, atCell)
activate Store
Note over Store: Shift existing cells at targetCol down by 1
Store->>Store: Update cell_index for affected fields
Store->>Compact: compact()
activate Compact
Note over Compact: Renormalize indices: row_index, column_index, cell_index
Compact->>Store: Return updated fields
deactivate Compact
Store->>Render: Emit store update
deactivate Store
Render->>Component: groupedRows recalculated
Component->>Render: useGroupedRows restructures fields into rows/columns
Render->>Render: Re-render row/column hierarchy
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@forms_pro/patches/v0_x/backfill_field_layout.py`:
- Around line 11-15: The UPDATE currently skips rows with NULLs because "!=
NULL" doesn't match; modify the WHERE clause in the SQL inside
backfill_field_layout.py so it also selects NULL layout indices — e.g., change
the WHERE to include "row_index IS NULL OR column_index IS NULL" (or
alternatively use "NOT (row_index = idx - 1 AND column_index = 0)") so every
`tabForm Field` row with NULL or wrong values gets updated (keep the SET as
"row_index = idx - 1, column_index = 0").
In `@frontend/src/components/builder/FieldCard.vue`:
- Around line 15-20: The computed isMultiColumn currently checks the total
number of fields in the same row, which misclassifies stacked cells as
multi-column; change the computation in isMultiColumn to collect fields from
editFormStore.fields whose (row_index ?? 0) matches (props.field.row_index ??
0), extract their column_index (use 0 default if needed), deduplicate those
values (e.g. via a Set) and return true only if the number of unique
column_index values is > 1; update any variable names (isMultiColumn,
editFormStore.fields, FormField, props.field.row_index, column_index)
accordingly so the eject action triggers only when there are multiple distinct
columns in that row.
In `@frontend/src/components/FormBuilderContent.vue`:
- Around line 248-252: The trailing RowDropZone is hidden when draggingFromRow
equals the last row index, preventing dragging a field from the last row into a
new row; update the RowDropZone v-if so the last-row drop zone is always
rendered regardless of draggingFromRow (i.e., remove the draggingFromRow !==
rowIndexOf(row, rIdx) check) so the component (RowDropZone) at rIdx ===
groupedRows.length - 1 remains available while dragging from the last row;
locate the conditional using rIdx, groupedRows, draggingFromRow, and rowIndexOf
to apply this change.
In `@frontend/src/types/FormsPro/form_field.types.ts`:
- Around line 37-40: The exported Form Field type contract is missing the
cell_index property; update the exported interface (the DocType/Form Field
interface that currently declares row_index and column_index) to include an
optional numeric cell_index field (e.g., add cell_index?: number;) so consumers
can represent stacked-cell positions alongside row_index and column_index.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 47c5e587-7bd9-4847-bf03-dc072acff5dd
📒 Files selected for processing (17)
forms_pro/forms_pro/doctype/form/test_form.pyforms_pro/forms_pro/doctype/form_field/form_field.jsonforms_pro/forms_pro/doctype/form_field/form_field.pyforms_pro/patches.txtforms_pro/patches/v0_x/__init__.pyforms_pro/patches/v0_x/backfill_field_layout.pyforms_pro/tests/test_form_field.pyfrontend/src/components/FormBuilderContent.vuefrontend/src/components/builder/ColumnDropZone.vuefrontend/src/components/builder/FieldActions.vuefrontend/src/components/builder/FieldCard.vuefrontend/src/components/builder/RowDropZone.vuefrontend/src/components/submission/FormRenderer.vuefrontend/src/composables/useGroupedRows.tsfrontend/src/stores/editForm.tsfrontend/src/types/FormsPro/form_field.types.tsfrontend/src/types/formfield.ts
- v-show (was v-if) on trailing RowDropZone — keeps SortableJS instance alive across drag start, fixes last-row reorder - global pointerup/dragend reset drag state when drop misses a target - drop zones: subtle blue tint (was dashed border), gap-4 between cells
- outer drop zones reserve constant size (h-6 row, w-4 col); visual hairline rendered via ::before pseudo only - removed parent flex gaps; zones now provide all inter-row/col spacing - no flow shift when drag starts — form stays still - highlighted state: 4px tinted bar, dragging: 1px hairline, idle: hidden
- before:transition-all → before:transition-[height,background-color] (row) / [width,background-color] (col): avoid animating unrelated props - add before:ease-out for entrance feel - motion-reduce:before:transition-none honors prefers-reduced-motion
Add :animation="150" to column draggable. SortableJS default 0ms made sibling swaps instant, producing jittery snap-snap-snap during drag. 150ms shuffle animation smooths sibling reorder.
…back - FieldCard, RowDropZone, ColumnDropZone, FormBuilderContent, FormRenderer now expose stable data-form-builder-component and data-form-renderer-component selectors plus row/column/cell index attributes for layout introspection. - Eject button on FieldActions tagged with a stable selector. - Set forceFallback: true on the cell column and drop zone draggables so SortableJS uses pointer events instead of native HTML5 drag, making drag behavior deterministic in headless browsers (and unblocks Playwright).
Adds frontend/e2e/specs/form-layout.spec.ts with 8 scenarios: 1. Drag-stack a cell into an existing column. 2. Drop on a column drop zone creates a new column. 3. Drop on a row drop zone creates a new row. 4. Eject button moves a stacked cell to its own row. 5. Cross-row drag collapses the source column. 6. Within-column reorder renumbers cell_index. 7. Mobile viewport (375x800) stacks columns vertically (flex-col). 8. Conditional hide unmounts the column entirely (no empty gap). Extends FormBuilderPage with layout introspection (rowCount, columnCount, cellCount, fieldCard, waitForFields) and drag helpers (dragFieldOntoCell, dragFieldToColumnZone, dragFieldToRowZone, ejectField). Drag uses a real mouse path with intermediate steps; cross-row drags route through an L-shaped waypoint via the row drop zone gap so SortableJS doesn't accidentally swap into a neighbor column en route.
Frappe's schema sync materializes new Int columns as NOT NULL DEFAULT 0, so legacy FormField rows pre-dating row_index/column_index land on 0 after migration, never NULL. The existing != predicate is sufficient; adding an IS NULL branch (per CodeRabbit suggestion) would be unreachable. Comment captures the reasoning so future maintainers don't re-litigate it.
Three changes from CodeRabbit pass: - FormBuilderContent: trailing RowDropZone no longer hides when the drag starts in the last row. The previous guard (`draggingFromRow !== rowIndexOf(row, rIdx)`) blocked the only path to a new row below when dragging the only field in the last row. Removed the guard and the now-unused `draggingFromRow` ref. - FieldCard / FieldActions: rename `isMultiColumn` -> `canEject`. The variable already returned true for stacked single-column rows (which is correct -- eject means "move to own row" and is valid for stacked cells), so the name was misleading. Behavior unchanged. - FormField type contract: add `cell_index?: number`. The exported DocType interface was lagging behind the schema; consumers couldn't represent stacked-cell positions.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
frontend/e2e/specs/form-layout.spec.ts (1)
18-31: ⚡ Quick winFail fast when a form setup
PUTis rejected.These setup writes ignore the response. If the backend rejects one of these layout payloads, the spec keeps running and the later UI assertions hide the real cause. Add an explicit response check here.
Suggested fix
- await apiContext.put(`/api/resource/Form/${formId}`, { + const response = await apiContext.put(`/api/resource/Form/${formId}`, { data: { fields: fields.map((f, i) => ({ idx: i + 1, fieldtype: f.fieldtype ?? "Data", label: f.label, fieldname: f.label.toLowerCase(), reqd: 0, row_index: f.row_index, column_index: f.column_index ?? 0, cell_index: f.cell_index ?? 0, })), }, }); + if (!response.ok()) { + throw new Error( + `Failed to seed form ${formId}: ${response.status()} ${await response.text()}` + ); + }Apply the same guard to the two inline
apiContext.put()calls below.Also applies to: 214-237, 274-298
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/e2e/specs/form-layout.spec.ts` around lines 18 - 31, The form-setup PUTs using apiContext.put (the call that sends { data: { fields: fields.map(...) } } with formId/fields) currently ignore the response; change each inline apiContext.put to capture the response (e.g., const res = await apiContext.put(...)) and immediately assert it succeeded (check res.ok or res.status === 200) and throw or fail the test including response body/details on rejection so the spec fails fast; apply the same guard to the two other inline apiContext.put calls in this file that set up form layout.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/e2e/helpers/form-builder.ts`:
- Around line 8-11: The selector in fieldCard(label: string) interpolates raw
user text into a CSS attribute selector which breaks for labels containing
quotes or special characters; update fieldCard to escape the label before
interpolation (use CSS.escape(label) or a small utility that
percent-escapes/escapes CSS selector metacharacters) and then pass the escaped
value into
this.page.locator(`[data-form-builder-component="field-card"][data-field-label="${escaped}"]`)
so labels with quotes, brackets, backslashes, etc. won’t break the locator.
In `@frontend/src/components/FormBuilderContent.vue`:
- Around line 32-34: The fieldKey function is producing unstable keys from
positional indexes; update it so it returns a stable identifier (use
field.fieldname) and ensure any place using fieldKey (e.g., the item-key on
FieldCard instances) switches to the new stable key; in function fieldKey(field:
FormField) return field.fieldname (with a fallback like `${field.fieldname ??
''}` or `${field.fieldname ?? field.id ?? ''}` if needed) and replace usages at
the FieldCard item-key locations (including the occurrences around the previous
224-227 area) to use this stable key.
---
Nitpick comments:
In `@frontend/e2e/specs/form-layout.spec.ts`:
- Around line 18-31: The form-setup PUTs using apiContext.put (the call that
sends { data: { fields: fields.map(...) } } with formId/fields) currently ignore
the response; change each inline apiContext.put to capture the response (e.g.,
const res = await apiContext.put(...)) and immediately assert it succeeded
(check res.ok or res.status === 200) and throw or fail the test including
response body/details on rejection so the spec fails fast; apply the same guard
to the two other inline apiContext.put calls in this file that set up form
layout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: eeeadc48-16fe-4573-ae82-43954151a417
📒 Files selected for processing (10)
forms_pro/patches/v0_x/backfill_field_layout.pyfrontend/e2e/helpers/form-builder.tsfrontend/e2e/specs/form-layout.spec.tsfrontend/src/components/FormBuilderContent.vuefrontend/src/components/builder/ColumnDropZone.vuefrontend/src/components/builder/FieldActions.vuefrontend/src/components/builder/FieldCard.vuefrontend/src/components/builder/RowDropZone.vuefrontend/src/components/submission/FormRenderer.vuefrontend/src/types/FormsPro/form_field.types.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- frontend/src/components/builder/RowDropZone.vue
- frontend/src/components/builder/ColumnDropZone.vue
- frontend/src/components/builder/FieldCard.vue
- forms_pro/patches/v0_x/backfill_field_layout.py
- frontend/src/components/submission/FormRenderer.vue
Comment on lines
+8
to
+11
| fieldCard(label: string): Locator { | ||
| return this.page.locator( | ||
| `[data-form-builder-component="field-card"][data-field-label="${label}"]` | ||
| ); |
There was a problem hiding this comment.
Escape label before interpolating it into the selector.
fieldCard() builds a CSS attribute selector from raw user text. Labels with quotes or other CSS-string characters will break this locator and make the new drag helpers fail on realistic field names.
Suggested fix
+ private escapeSelectorValue(value: string): string {
+ return value.replace(/\\/g, "\\\\").replace(/"/g, '\\"');
+ }
+
fieldCard(label: string): Locator {
+ const escapedLabel = this.escapeSelectorValue(label);
return this.page.locator(
- `[data-form-builder-component="field-card"][data-field-label="${label}"]`
+ `[data-form-builder-component="field-card"][data-field-label="${escapedLabel}"]`
);
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/e2e/helpers/form-builder.ts` around lines 8 - 11, The selector in
fieldCard(label: string) interpolates raw user text into a CSS attribute
selector which breaks for labels containing quotes or special characters; update
fieldCard to escape the label before interpolation (use CSS.escape(label) or a
small utility that percent-escapes/escapes CSS selector metacharacters) and then
pass the escaped value into
this.page.locator(`[data-form-builder-component="field-card"][data-field-label="${escaped}"]`)
so labels with quotes, brackets, backslashes, etc. won’t break the locator.
Comment on lines
+32
to
+34
| function fieldKey(field: FormField): string { | ||
| return `${field.row_index ?? 0}-${field.column_index ?? 0}-${field.cell_index ?? 0}`; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg "FormField" -t ts -t vue --no-heading | head -20Repository: BuildWithHussain/forms_pro
Length of output: 98
🏁 Script executed:
rg "row_index.*column_index.*cell_index" -A 1 -B 1Repository: BuildWithHussain/forms_pro
Length of output: 1323
🏁 Script executed:
git ls-files | grep -E "\.(vue|ts|tsx)$" | head -30Repository: BuildWithHussain/forms_pro
Length of output: 1282
🏁 Script executed:
rg "vuedraggable" -t vue -A 3 -B 3Repository: BuildWithHussain/forms_pro
Length of output: 98
🏁 Script executed:
rg "class FormField" -A 10Repository: BuildWithHussain/forms_pro
Length of output: 941
🏁 Script executed:
rg "FormField\s*=" -A 5Repository: BuildWithHussain/forms_pro
Length of output: 1113
🏁 Script executed:
rg "interface FormField" -A 10Repository: BuildWithHussain/forms_pro
Length of output: 829
🏁 Script executed:
cat -n frontend/src/components/FormBuilderContent.vue | sed -n '25,45p'Repository: BuildWithHussain/forms_pro
Length of output: 842
🏁 Script executed:
cat -n frontend/src/components/FormBuilderContent.vue | sed -n '220,230p'Repository: BuildWithHussain/forms_pro
Length of output: 681
🏁 Script executed:
head -50 frontend/src/components/FormBuilderContent.vueRepository: BuildWithHussain/forms_pro
Length of output: 1821
🏁 Script executed:
rg "item-key" frontend/src/components/FormBuilderContent.vue -B 3 -A 3Repository: BuildWithHussain/forms_pro
Length of output: 407
🏁 Script executed:
rg "fieldKey" -A 5 -B 5Repository: BuildWithHussain/forms_pro
Length of output: 1988
🏁 Script executed:
cat frontend/src/components/builder/FieldCard.vue | head -50Repository: BuildWithHussain/forms_pro
Length of output: 1871
🏁 Script executed:
rg "field\.name" frontend/src/ --type ts --type tsx | head -15Repository: BuildWithHussain/forms_pro
Length of output: 98
🏁 Script executed:
wc -l frontend/src/components/FormBuilderContent.vueRepository: BuildWithHussain/forms_pro
Length of output: 121
🏁 Script executed:
sed -n '1,100p' frontend/src/components/FormBuilderContent.vueRepository: BuildWithHussain/forms_pro
Length of output: 3694
🏁 Script executed:
sed -n '180,250p' frontend/src/stores/editForm.tsRepository: BuildWithHussain/forms_pro
Length of output: 2371
🏁 Script executed:
cat frontend/src/types/formfield.tsRepository: BuildWithHussain/forms_pro
Length of output: 506
🏁 Script executed:
rg "name\s*:" frontend/src/types/formfield.ts -A 1Repository: BuildWithHussain/forms_pro
Length of output: 115
🏁 Script executed:
head -100 frontend/src/stores/editForm.tsRepository: BuildWithHussain/forms_pro
Length of output: 3196
🏁 Script executed:
sed -n '1,60p' frontend/src/stores/editForm.tsRepository: BuildWithHussain/forms_pro
Length of output: 2200
🏁 Script executed:
sed -n '130,200p' frontend/src/stores/editForm.tsRepository: BuildWithHussain/forms_pro
Length of output: 2095
🏁 Script executed:
sed -n '240,280p' frontend/src/stores/editForm.tsRepository: BuildWithHussain/forms_pro
Length of output: 1084
🏁 Script executed:
python3 << 'EOF'
# Analyze whether position-based keying causes actual problems
# When a field is dragged, does Vue reuse the FieldCard instance incorrectly?
# Key question: After drag, does the key change?
# Before drag: field A at [0-0-0] has key "0-0-0"
# After drag: field A at [0-0-1] has key "0-0-1"
# Meanwhile field B that was at [0-0-1] is now at [0-0-0] with key "0-0-0"
# If FieldCard had local state, it would be reused with old state
# Let's check if FieldCard has any reactive local state
import subprocess
result = subprocess.run(
['rg', 'ref\\(|reactive\\(', 'frontend/src/components/builder/FieldCard.vue'],
capture_output=True,
text=True
)
print("FieldCard reactive state:")
print(result.stdout)
EOFRepository: BuildWithHussain/forms_pro
Length of output: 97
🏁 Script executed:
cat frontend/src/components/builder/FieldActions.vueRepository: BuildWithHussain/forms_pro
Length of output: 1597
🏁 Script executed:
cat frontend/src/components/builder/FieldRenderer.vueRepository: BuildWithHussain/forms_pro
Length of output: 5052
🏁 Script executed:
rg "selectedField === \|selectedField ==" frontend/src/components/builder/ -B 2 -A 2Repository: BuildWithHussain/forms_pro
Length of output: 52
Use a stable field identifier for item-key.
fieldKey() derives its value from row/column/cell position, which changes every time a field is moved during drag operations. This makes the key unstable—Vue may reuse a FieldCard instance for a different field after reordering. Use field.fieldname instead, which is the stable per-field identifier.
Suggested fix
function fieldKey(field: FormField): string {
- return `${field.row_index ?? 0}-${field.column_index ?? 0}-${field.cell_index ?? 0}`;
+ return field.fieldname;
}Also applies to: 224-227
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/FormBuilderContent.vue` around lines 32 - 34, The
fieldKey function is producing unstable keys from positional indexes; update it
so it returns a stable identifier (use field.fieldname) and ensure any place
using fieldKey (e.g., the item-key on FieldCard instances) switches to the new
stable key; in function fieldKey(field: FormField) return field.fieldname (with
a fallback like `${field.fieldname ?? ''}` or `${field.fieldname ?? field.id ??
''}` if needed) and replace usages at the FieldCard item-key locations
(including the occurrences around the previous 224-227 area) to use this stable
key.
Collaborator
|
Demo video? 👀 |
Collaborator
Author
|
@NagariaHussain added |
|
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin version-15
git worktree add -d .worktree/backport-119-to-version-15 origin/version-15
cd .worktree/backport-119-to-version-15
git switch --create backport-119-to-version-15
git cherry-pick -x 2c18e310a28d1747cf4a75d46989f3919bf3c762 |
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds spatial drag-and-drop multi-column layout to the form builder, with vertical cell stacking inside columns.
row_index+column_index+cell_indexinteger columns onFormField. Sort key:(row_index, column_index, cell_index). Layout is purely presentational, never serialized to the linked DocType'sCustomField.ColumnDropZoneto add a new column. Drop onto another column's cell list to stack as a new cell.RowDropZonebetween rows still creates a fresh full-width row. Eject button on multi-column fields restores them to their own row.flex flex-col md:flex-row(mobile stack, desktop side-by-side).v-ifper row/column/cell so hidden fields fully unmount.compact,moveField,insertCell,insertNewRow) keep the three axes contiguous after every mutation.to_frappe_fieldfor every fieldtype.column-demo.mp4
Tests
Playwright e2e covers all builder + renderer flows (
frontend/e2e/specs/form-layout.spec.ts):cell_indexbench run-tests --app forms_propassesSummary by CodeRabbit
Release Notes
New Features
Tests