[Crabcyborg]

Batch field markup parsing outside the live builder DOM and insert adjacent loaded fields together with a DocumentFragment. Keep the existing render time slices and initialize controls after each slice is inserted.

Testing

• Compiled the production formidable_admin.js bundle.
• Oxlint passed on both changed JavaScript source files.
• ESLint completed successfully. admin.js reports existing warnings elsewhere in the file.
• Browser verification was not run.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 35 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: Strategy11/formidable-forms
• Branch: perf/batch-form-builder-field-inserts
• File: .coderabbit.yaml
• Commit: bda6222
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: a8816f0c-a86d-41c6-a94c-f4f47a826dc9

Commits

Reviewing files that changed from the base of the PR and between 826f89e and bda6222.

Files selected for processing: 3

• js/formidable_admin.js
• js/src/admin/admin.js
• js/src/admin/fieldLoadBatch.js

• 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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[deepsource-io]

DeepSource Code Review

We reviewed changes in 826f89e...bda6222 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

Important

Some issues found as part of this review are outside of the diff in this pull request and aren't shown in the inline review comments due to GitHub's API limitations. You can see those issues on the DeepSource dashboard.

PR Report Card

Overall Grade

Security   Reliability   Complexity   Hygiene

Code Review Summary

Analyzer	Status	Updated (UTC)	Details
PHP		Oct 29, 2026 3:37p.m.	Review ↗
JavaScript		Oct 29, 2026 3:37p.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]

Approving. The PR says browser verification was not run, so I ran it. The builder loads the same DOM as master, and nothing I exercised broke. One non-blocking note below.

What I ran (Lite at this head in the Playground preview env, one form with 30 fields so the builder uses the lazy placeholders, 3 fields per row in most rows):

Check	master bundle	PR bundle
Fields in the builder after load	31	31
Placeholders still loading	0	0
Field order / duplicate IDs	ascending / 0	ascending / 0
Rows holding 3 adjacent loaded fields (the fragment-group path)	19 rows, max 3	19 rows, max 3
Normalized #frm-show-fields HTML (length + hash, inline style and ui-* classes stripped)	identical	identical
data-bs-toggle, dropdown-menu, droppable widget counts	31 / 31 / 7	31 / 31 / 7

I loaded master's js/formidable_admin.js in a second browser session to confirm the baseline really was the master bundle (265445 bytes served). Then I clicked a loaded field: it was selected, and its settings panel showed the right field. A screenshot of the builder shows the three-per-row layout rendering correctly, and there were no new console errors (only 404s from unrelated add-ons in the env).

Source read: prepareLoadedFieldMarkup() and resolveDeferredTooltip() only touch the element itself, so running them while it is detached is safe. Dividers are never placeholders ($ajax_this_field in FrmFieldsController.php excludes divider and end_divider), so a placeholder is never nested inside another one being replaced. initializeLoadedFieldSlice() still runs after insertion, so frm_ajax_loaded_field listeners (Pro listens for it) see the field in the DOM as before.

The committed js/formidable_admin.js contains the new code. I could not reproduce it byte for byte: a rebuild here changes every webpack module id (files the PR does not touch change too), so I can only say the bundle is built from this source, not that it is identical.

Not exercised: drag-and-drop and Pro-only fields (sections, repeaters) in the builder. I also did not measure load time, so the performance gain itself is unmeasured. jQuery.replaceWith( html ) used to run inline <script> tags in the field markup and template.innerHTML does not. Lite's builder field markup has none, so that is only relevant if Pro field markup carries any.

One non-blocking note: in renderLoadedField() (js/src/admin/admin.js), if field.html produces no element, the function now returns null before the placeholder is touched. The old code had already replaced the placeholder at that point, so it was gone. Now the placeholder and its spinner stay in the builder. It only matters if the server sends empty markup for a field, but removing the placeholder in that branch (oldField.remove(); before return null;) would keep the old behavior.

CI shows every check as skipped on this head, so I relied on the live run above.

[franky-the-going-merry]

Approving. This is a re-review of the whole PR at bda6222. My earlier approval at 289da84 had one non-blocking note, and the author fixed it in this commit.

The fix, run live (Lite at this head in the Playground preview env, a 30-field form with the builder forced into ajax loading, and the ajax response for one field rewritten in the browser to carry empty markup):

Bundle	Fields in the builder	Spinner / placeholder left behind	Other fields
289da84 (previous commit)	31, empty field still shown as a loading placeholder	1	loaded
bda6222 (this head)	30, the empty field is gone	0	all 30 loaded, in order, including Submit

The committed js/formidable_admin.js contains the change (if(!a)return i.remove(),null), and js/src/admin/admin.js has the matching one-line oldField.remove();. A null result is already handled in insertLoadedFieldSlice(): it flushes the current group, so removing the placeholder cannot join two non-adjacent fields.

Still true from the first review: at 289da84 the builder DOM was identical to master's (same fields, order, normalized markup, drag/dropdown widget counts) with the fragment-group path exercised (19 rows of 3). This commit only changes the empty-markup branch, so I did not repeat that comparison, and the diff between the two commits is those two lines plus the rebuilt bundle.

Not exercised: drag-and-drop, Pro-only fields (sections, repeaters), load-time measurement, and the PHPUnit/Cypress suites (CI shows every relevant check as skipped). Psalm and DeepSource JavaScript show red. This PR changes no PHP, so I read the Psalm failure as unrelated, but I did not open either report.