Fix multi-select deselect being undone when field is inside a Section - #3351
vivi-the-going-merry[bot] wants to merge 1 commit into
Conversation
ul.frm_sorting nests (row inside section inside form), and this click
handler is delegated on the same event for every matching ancestor.
stopPropagation would also block the unrelated document-level handler
that closes an open field-settings dropdown on outside click, so guard
with a one-shot flag on the shared event object instead.
Closes Strategy11/formidable-pro#6029
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
 |
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| PHP |  |
Sep 16, 2026 11:49p.m. | Review ↗ |
| JavaScript | |
Sep 16, 2026 11:49p.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.
1 participant
What
Ctrl/Cmd+Click (or Shift+Click) to deselect a field group in the builder's multi-select doesn't stick when the field is inside a Section — the group re-selects itself immediately. Reported against
formidable-probut the code is Lite's (free pluginlabel on the issue).Root cause (mechanism confirmed, exact repro not fully pinned down)
fieldGroupClickis delegated onul.frm_sortingfrom#frm-show-fields(js/src/admin/admin.js). A row's ownul.frm_sortingnests inside the form-wide one, and — for a field inside a Section — inside the Section's ownul.frm_sortingtoo. I confirmed via instrumentation that a single click on a nestedul.frm_sortingdoes invoke a delegated handler bound atdocumenttwice (once per matching ancestor), which is the double-invocation mechanism the original fix targeted.Caveat: a synthetic-click test against the simplest repro I could build (two plain Text fields directly inside one Section, nothing merged into a row) did not reproduce the visible symptom against pre-fix
master— the deselect worked correctly in that specific case, so I can't 100% confirm this is the exact trigger the issue reporter hit. The issue's own thread mentions "group selection" specifically, which may mean fields merged into one row (afrm-selected-field-groupspanning >1 field) rather than a bare single-field row — that combination inside a Section is untested here. Flagging for reviewer verification against the original repro steps before merging, rather than blocking on further investigation.Fix
Guard with a one-shot flag set on the event object (
e.frmFieldGroupClickHandled), note.stopPropagation()— this same click also needs to reach an unrelateddocument-level delegated handler (handleClickOutsideOfFieldSettings, bound on#frm_builder_page) that closes an open field-settings dropdown on outside click.stopPropagation()would silently break that. This guard is a no-op if the double-invocation this targets isn't actually happening for a given click, so it's safe to land even if it doesn't turn out to be the whole story.Build note
No JS toolchain (
node_modules) available in this environment to runnpm run build, sojs/formidable_admin.jswas hand-mirrored — diffed byte-for-byte againstHEAD's built file to confirm the only change is the exact new guard clause, nothing else drifted.node --checkpasses on both files.Closes Strategy11/formidable-pro#6029