You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Add full-width class to single-select dropdown Other input - #3624
Part of Strategy11/formidable-pro#6909 (the Pro fix is a separate PR there).
Broken: The fallback in FrmFieldsHelper::include_other_input() (used when Pro is missing or older than 7.0) adds frm_other_full only to multi-select dropdowns, so a single-select dropdown's Other input is narrower and has no top gap.
Changed: Any dropdown (type === 'select') gets frm_other_full. Added a test to test_FrmFieldsHelper.
Verified: New test red on master (output has no frm_other_full), green on the branch; test_FrmFieldsHelper 6/6. The fallback path was not rendered live. The images below are from the Pro-side fix, which adds the same class (master vs branch, "Other" selected).
We reviewed changes in c5b435b...2df5c50 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.
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.
The reason will be displayed to describe this comment to others. Learn more.
Approve.
Bug reproduced and fixed live. Preview env (Lite only, so Pro is not active and the fallback in include_other_input() is the path that runs), a form with a single-select dropdown, a multi-select dropdown and a radio, each with an Other option, rendered through FrmFormsController::get_form_shortcode():
field
base
PR
single select
frm_other_input frm_pos_none
frm_other_input frm_pos_none frm_other_full
multi select
... frm_other_full
... frm_other_full (unchanged)
radio
frm_other_input frm_pos_none
unchanged
Checked: the condition now matches the intent in the PR text (every dropdown). The class has no CSS in Lite; the width/margin rules live in Pro (css/single-style.css.php, css/pro_fields.css.php), so the visible fix only reaches sites on Pro older than 7.0, which is exactly the fallback's audience. CI PHPUnit passes on 7.4, 8.0, 8.2 and 8.4; Psalm is red with the same MissingPureAnnotation errors as other PRs on master. The DeepSource PHP failure ("undefined assertStringContainsString()") is a false positive: the method exists in PHPUnit 9.6+ and the test runs green in CI.
Not exercised: the visual width/margin of the Other input (needs Pro older than 7.0 for the CSS), the new PHPUnit test locally (CI green only), review skills (one-line diff read in full), browser screenshot.
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
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.
Part of Strategy11/formidable-pro#6909 (the Pro fix is a separate PR there).
Broken: The fallback in
FrmFieldsHelper::include_other_input()(used when Pro is missing or older than 7.0) addsfrm_other_fullonly to multi-select dropdowns, so a single-select dropdown's Other input is narrower and has no top gap.Changed: Any dropdown (
type === 'select') getsfrm_other_full. Added a test totest_FrmFieldsHelper.Verified: New test red on master (output has no
frm_other_full), green on the branch;test_FrmFieldsHelper6/6. The fallback path was not rendered live. The images below are from the Pro-side fix, which adds the same class (master vs branch, "Other" selected).🤖 Generated with Claude Code