Fix multiselect issue with section field by AbdiTolesa · Pull Request #2521 · Strategy11/formidable-forms

AbdiTolesa · 2025-10-01T12:12:52Z

Fix https://github.com/Strategy11/formidable-pro/issues/5229

Test steps

Add a few fields, a Section field and some fields into the section.
Try multi selecting with ctrl + click on fields outside the Section and then try adding the Section field to the selected groups by clicking on the Section header.
Confirm that the section field is selectable with other fields.

CleanShot.2025-10-01.at.16.26.33.mp4

CleanShot.2025-10-01.at.16.16.00.mp4

coderabbitai · 2025-10-01T12:19:58Z

Warning

Rate limit exceeded

@AbdiTolesa has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 20 minutes and 25 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📝 Walkthrough

Walkthrough

Adds Ctrl/Meta-key handling to the admin field builder click handler: Ctrl/Meta-click adds the clicked field's containing ul to frm-selected-field-group and returns early, skipping the normal clickAction; plain clicks remain unchanged.

Changes

Cohort / File(s)	Change Summary
Form builder multi-select trigger `js/src/admin/admin.js`	On click, if `e?.metaKey` or `e?.ctrlKey` is truthy, add the clicked field’s containing `ul` to `frm-selected-field-group` and return early (bypassing the normal `clickAction`); unchanged behavior when modifiers are not pressed.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant U as User
  participant UI as Field Builder UI
  participant H as admin.js click handler

  U->>UI: Click field (with/without modifiers)
  UI->>H: onClick(event)
  alt Ctrl or Meta pressed
    H->>H: Locate clicked field's containing `ul`
    H->>UI: Add class "frm-selected-field-group" to that `ul`
    Note right of H#lightblue: Early return — skip normal clickAction
    H-->>UI: Return
  else No Ctrl/Meta
    H->>UI: Proceed with normal clickAction
  end

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

Strategy11/formidable-forms#2522 — Both concern builder field selection behavior; this change adds Ctrl/Meta multi-select handling which may address selection/row issues mentioned.

Suggested labels

action: needs qa, run analysis, run tests

Suggested reviewers

lauramekaj1
truongwp

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The pull request title "Fix multiselect issue with section field" directly corresponds to the main change in the changeset. The raw summary indicates that the code change adds Ctrl/Meta key handling to enable multi-selection of section fields in the admin builder. The title clearly and concisely describes the primary purpose of the change and is specific enough for a reviewer scanning the history to understand the modification addresses multiselect functionality for section fields.
Description Check	✅ Passed	The pull request description is directly related to the changeset, providing a reference to the specific issue being fixed (issue `#5229`), detailed test steps for reproducing and verifying the fix, and video attachments demonstrating the multiselect behavior with section fields. The description provides meaningful context about what problem is being solved and how to validate the solution, making it clearly relevant to the code changes.

✨ Finishing touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch issue-5229

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

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

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a24133c and 92a1567.

⛔ Files ignored due to path filters (2)

js/formidable-settings-components.js.map is excluded by !**/*.map, !**/*.map
js/formidable_dashboard.js.map is excluded by !**/*.map, !**/*.map

📒 Files selected for processing (1)

js/src/admin/admin.js (1 hunks)

js/src/admin/admin.js

lauramekaj1 · 2025-10-09T11:20:18Z

Hi @AbdiTolesa,
Multi-selecting including the Section field works as expected.
However, when trying to deselect fields in a group selection (e.g., Ctrl + Click to remove one item from the group), it doesn't work properly since the field remains selected.
Please see screen recording for reference:
https://www.loom.com/share/1bd785331c9b44ffa884dd3c3bfa959e

AbdiTolesa · 2025-10-09T14:44:28Z

Hi @AbdiTolesa,
Multi-selecting including the Section field works as expected.
However, when trying to deselect fields in a group selection (e.g., Ctrl + Click to remove one item from the group), it doesn't work properly since the field remains selected.

Hi @lauramekaj1 I just created a separate issue for that: https://github.com/Strategy11/formidable-pro/issues/6029
This PR is just intended to fix multi-select issues for fields inside Section fields.

lauramekaj1 · 2025-10-09T16:05:11Z

Hi @AbdiTolesa,

Multi-selecting including the Section field works as expected.

However, when trying to deselect fields in a group selection (e.g., Ctrl + Click to remove one item from the group), it doesn't work properly since the field remains selected.

Hi @lauramekaj1 I just created a separate issue for that: https://github.com/Strategy11/formidable-pro/issues/6029

This PR is just intended to fix multi-select issues for fields inside Section fields.

@AbdiTolesa ok so I validated that the issue in this PR is fixed. Thank you!

truongwp · 2025-10-10T19:36:56Z

@AbdiTolesa Does this cover this comment? https://github.com/Strategy11/formidable-pro/issues/6029#issuecomment-3387004193

AbdiTolesa · 2025-10-13T13:54:40Z

@AbdiTolesa Does this cover this comment? Strategy11/formidable-pro#6029 (comment)

@truongwp The link takes to the issue created probably for the comment you were referring to, we are tracking that separately anyway.

truongwp · 2025-10-15T11:22:45Z

@AbdiTolesa If I click on the first field, then hold Shift and click on a field inside a section, it selects all fields in the form. Please see this screencast:

Screen.Recording.2025-10-15.at.18.21.37.mov

AbdiTolesa · 2025-10-15T12:46:33Z

@AbdiTolesa If I click on the first field, then hold Shift and click on a field inside a section, it selects all fields in the form. Please see this screencast:

@truongwp We are tracking that issue separately: https://github.com/Strategy11/formidable-pro/issues/6017

js/src/admin/admin.js

truongwp

This looks good now. Thanks @AbdiTolesa!

truongwp · 2025-10-17T16:22:52Z

@AbdiTolesa Please resolve the conflicts with the master branch so we can merge it.

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

js/src/admin/admin.js (1)
5690-5694: Keep multi‑select UI in sync on Ctrl/Cmd click

You add the class and return, but don’t call the existing multi‑select sync flow. The popup/controls won’t appear until a later action. Reuse the same path as group clicks by invoking syncAfterMultiSelect immediately.

Apply this minimal change:
-		if ( e?.metaKey || e?.ctrlKey ) {
-			// Add the target to list of selected fields.
-			this.closest( 'ul' ).classList.add( 'frm-selected-field-group' );
-			return;
-		}
+		if ( e?.metaKey || e?.ctrlKey ) {
+			// Add the target to list of selected fields and sync UI state.
+			const ul = this.closest( 'ul.frm_sorting' ) || this.closest( 'ul' );
+			if ( ul ) {
+				ul.classList.add( 'frm-selected-field-group' );
+				const count = jQuery( '.frm-selected-field-group' ).length || 1;
+				syncAfterMultiSelect( count );
+				maybeHideFieldGroupMessage();
+			}
+			return;
+		}
Note: Ctrl/Cmd‑click deselect toggling is tracked in formidable-pro#6029 and can remain out of scope here. Please just verify the popup appears right after the first Ctrl/Cmd click.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a636cfa and 597dea4.

⛔ Files ignored due to path filters (1)

js/formidable-settings-components.js.map is excluded by !**/*.map, !**/*.map

📒 Files selected for processing (1)

js/src/admin/admin.js (1 hunks)

truongwp

This looks good now. Thanks @AbdiTolesa!

js/src/admin/admin.js

deepsource-io · 2026-02-06T15:11:27Z

Here's the code health analysis summary for commits cf4606a..aa08534. View details on DeepSource ↗.

Analysis Summary

Analyzer	Status	Summary	Link
PHP	✅ Success		View Check ↗
JavaScript	✅ Success		View Check ↗

💡 If you’re a repository administrator, you can configure the quality gates from the settings.

Fix multiselect issue with section field

92a1567

coderabbitai bot reviewed Oct 1, 2025

View reviewed changes

js/src/admin/admin.js Outdated Show resolved Hide resolved

Consider multiselecting with ctrl too in the fix

27a59f6

AbdiTolesa mentioned this pull request Oct 1, 2025

Fix multiselecting issue with section field #2186

Closed

AbdiTolesa requested a review from lauramekaj1 October 2, 2025 11:44

lauramekaj1 approved these changes Oct 9, 2025

View reviewed changes

AbdiTolesa requested a review from truongwp October 9, 2025 17:02

truongwp reviewed Oct 15, 2025

View reviewed changes

js/src/admin/admin.js Outdated Show resolved Hide resolved

Remove a check not needed to fix issue-5229

a636cfa

AbdiTolesa requested a review from truongwp October 17, 2025 13:40

truongwp approved these changes Oct 17, 2025

View reviewed changes

truongwp requested a review from Crabcyborg October 17, 2025 16:22

AbdiTolesa added 2 commits October 22, 2025 18:06

Fix merge conflicts

c61b7cd

Build files

597dea4

coderabbitai bot reviewed Oct 22, 2025

View reviewed changes

AbdiTolesa requested a review from truongwp October 23, 2025 10:55

truongwp approved these changes Oct 30, 2025

View reviewed changes

truongwp mentioned this pull request Oct 30, 2025

Fix range selection that includes fields in section #2527

Open

truongwp requested review from Crabcyborg and removed request for Crabcyborg November 3, 2025 18:10

Crabcyborg added this to the 6.26 milestone Nov 3, 2025

Merge branch 'master' into issue-5229

45439bd

Crabcyborg reviewed Dec 8, 2025

View reviewed changes

js/src/admin/admin.js Outdated Show resolved Hide resolved

Crabcyborg modified the milestones: 6.26, 6.27 Dec 8, 2025

Crabcyborg modified the milestones: 6.27, 6.28 Jan 5, 2026

Crabcyborg modified the milestones: 6.28, 6.29 Feb 3, 2026

AbdiTolesa added 4 commits February 6, 2026 17:57

Run functions that should run after multiselecting a section

da2acc1

Merge master

1290466

Fix merge conflict

63c7e54

Add fic

caa00aa

AbdiTolesa added 7 commits February 6, 2026 18:14

Reset all other changes

046ddd1

Add back fix

9745523

Delete all *.js.map files

83d7846

Delete all *.js.map files

aeae5b5

Rebuild files

15ecf15

Reshuffle code

c2475ad

Remove null check not needed

aa08534

AbdiTolesa requested a review from Crabcyborg February 6, 2026 15:32

Crabcyborg modified the milestones: 6.29, 6.30 Mar 9, 2026

Conversation

AbdiTolesa commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test steps

Uh oh!

coderabbitai bot commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rate limit exceeded

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related issues

Suggested labels

Suggested reviewers

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lauramekaj1 commented Oct 9, 2025

Uh oh!

AbdiTolesa commented Oct 9, 2025

Uh oh!

lauramekaj1 commented Oct 9, 2025

Uh oh!

truongwp commented Oct 10, 2025

Uh oh!

AbdiTolesa commented Oct 13, 2025

Uh oh!

truongwp commented Oct 15, 2025

Uh oh!

AbdiTolesa commented Oct 15, 2025

Uh oh!

Uh oh!

truongwp left a comment

Choose a reason for hiding this comment

Uh oh!

truongwp commented Oct 17, 2025

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

truongwp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

deepsource-io bot commented Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Analysis Summary

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

AbdiTolesa commented Oct 1, 2025 •

edited

Loading

coderabbitai bot commented Oct 1, 2025 •

edited

Loading

deepsource-io bot commented Feb 6, 2026 •

edited

Loading