Repository navigation
Conversation
Adding a field renumbered the field order inputs by builder position, then copied the submit row orders sent by the server over them. The server bases those orders on the highest saved field_order, which can be lower than the builder positions (for example on forms from templates, imports or the API). The submit row then saved with a lower order than the fields above it and showed above them after reload. Click-to-add never renumbered at all. afterAddField now copies the server orders first and then renumbers from the builder, so every add path ends with orders that match the builder. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to Added-field ordering applies server orders before builder-position renumbering. The server omits empty order data rather than emitting malformed JSON, and no actionable merge risk remains. Pre-merge checks |
  |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 1 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: preserving the submit button as the last field when adding fields in the builder. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
- Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
-
📝 Generate docstrings 💡
-
-
- Commit to this branch
- Create a new PR
-
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 help to get the list of available commands.
 |
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| PHP |  |
Oct 10, 2026 7:58a.m. | Review ↗ |
| JavaScript | |
Oct 10, 2026 7:58a.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
franky-the-going-merry
Bot
added
the
franky-working
![franky-the-going-merry[bot]](https://avatars.githubusercontent.com/in/4233782?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approve. Reproduced the bug on base and confirmed the fix on this branch in a live builder (Playground, Lite at e927e9e).
Setup: a form saved with field orders A=1, B=2, C=3, Submit=4 (the template/import case from the description).
Base (master): dragged a Text field above Submit. Orders: C=6, Submit=6 (tied), new field=8. After Update and reload, Submit sits above the new field:
This PR: same steps. New field=8, Submit=10. After Update and reload, Submit is last:
Then click-added Paragraph, Section and Email on the same form. The orders matched the numbers in the description (Paragraph 10 / Submit 12; section 12, end of section 13, Submit 15; Email 15, Submit 17), and Submit stayed last each time.
Source: afterAddField() now copies the server orders and then calls updateFieldOrder(), which also renumbers page breaks, so removing the three separate updateFieldOrder() calls and the frm-collapse-page check loses nothing. js/formidable_admin.js differs from master only in those same edits (diffed token by token), so the build matches js/src.
Not exercised: duplicating a field, duplicating into an existing row, dragging a field into a section, and Update + reload after the click-add sequence. I also didn't run the review skills. CI is green where it ran.
franky-the-going-merry
Bot
removed
franky-review
franky-working
1 participant
Sometimes, after adding a field in the form builder and saving, the submit button is no longer the last field.
There were two field order schemes:
updateFieldOrder()numbers fields by their position in the builder. Row wrappers count too, so one-field rows get 2, 4, 6...field_order+ 1 and moves the submit row to + 2. It sends those orders back in#frm-last-row-fields-order.When a field was dragged in, the builder renumbered first and
afterAddField()then copied the server's orders over the submit row. If the saved orders are lower than the builder positions, the submit row got a lower order than the fields above it. This happens on forms from templates, imports or the API, which save orders 1, 2, 3... It can also happen after unsaved layout changes. After Update and a reload, the submit button showed above the new field, or inside a section. Click-to-add never renumbered, so it could mix the two schemes as well.afterAddField()now copies the server's orders first, so the inputs match the database, and then callsupdateFieldOrder(). The builder position now decides the order on every add path. The separateupdateFieldOrder()calls in the drag,insertFormField()and duplicate paths are removed becauseafterAddField()handles it. The extrarenumberPageBreaks()call is removed too, sinceupdateFieldOrder()already does that. No PHP changes.On forms whose saved orders already match the builder, the extra renumber only changes the new field.
js/formidable_admin.jscontains only this change on top of master's build. A full rebuild with my local packages also changed two unrelated Babel helpers and other bundles, so I left those out.Testing
On a form with saved orders A=1, B=2, C=3, Submit=4:
🤖 Generated with Claude Code
Summary by CodeRabbit