Skip to content

Fix inline Submit button layout and position#2969

Open
AbdiTolesa wants to merge 2 commits intomasterfrom
issue-6326
Open

Fix inline Submit button layout and position#2969
AbdiTolesa wants to merge 2 commits intomasterfrom
issue-6326

Conversation

@AbdiTolesa
Copy link
Copy Markdown
Contributor

@AbdiTolesa AbdiTolesa commented Feb 24, 2026

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Feb 24, 2026

📝 Walkthrough

Walkthrough

Adds logic to detect and reflow inline submit buttons when they share a row with other fields, plus a helper to compare element top positions; updates call sites during form init/destroy. Also adds CSS rules to position the inline submit button container and align the submit button.

Changes

Cohort / File(s) Summary
Submit Button Layout Logic
js/formidable.js
Adds maybeUpdateInlineSubmitButtonLayout to detect submit buttons aligned with previous fields and add frm_inline_submit_button class; adds getTopPositionOfElement helper; calls inserted on form init (iframe/JS init) and on destroy/redirect paths.
Inline Submit Button Styling
css/custom_theme.css.php
Adds .with_frm_style .frm_inline_submit_button { position: relative; }, positions .frm_submit absolutely at the bottom of its container, and removes bottom margin from .frm_inline_submit_button .frm_submit .frm_button_submit to support the new inline layout.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 I nudged the button into line,
Beneath the fields it now does shine.
A top check, a gentle shove,
Inline and snug — a layout I love! 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Fix inline Submit button layout and position' directly and clearly summarizes the main changes across both modified files, which involve repositioning and reflowing the inline submit button.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch issue-6326

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


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.

@deepsource-io
Copy link
Copy Markdown

deepsource-io bot commented Feb 24, 2026

DeepSource Code Review

We reviewed changes in d3a993b...5fb5860 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 ↗

PR Report Card

Overall Grade   Security  

Reliability  

Complexity  

Hygiene  

Code Review Summary

Analyzer Status Updated (UTC) Details
PHP Feb 24, 2026 12:11p.m. Review ↗
JavaScript Feb 24, 2026 12:11p.m. Review ↗

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@js/formidable.js`:
- Around line 1687-1699: Guard against null DOM nodes and replace strict top
equality with a tolerance check: when iterating submitButtons, ensure
submitContainer (from button.closest('.frm_form_field')) and previousSibling
exist before calling getTopPositionOfElement to avoid getBoundingClientRect()
errors; then compare tops using a tolerance (e.g.,
Math.abs(getTopPositionOfElement(submitContainer) -
getTopPositionOfElement(previousSibling)) > tolerance) instead of === so
sub‑pixel differences don’t skip inline layouts; keep references to
submitContainer, previousSibling, submitWrapper, and button when applying style
changes.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d3a993b and 55c730d.

📒 Files selected for processing (1)
  • js/formidable.js

js/formidable.js Outdated
Comment on lines +1687 to +1699
submitButtons.forEach( button => {
const submitContainer = button.closest( '.frm_form_field' );
const previousSibling = submitContainer.previousElementSibling;
if ( getTopPositionOfElement( submitContainer ) !== getTopPositionOfElement( previousSibling ) ) {
// If the submit button is not inline with other fields, do not update the layout.
return;
}

submitContainer.style.position = 'relative';
const submitWrapper = button.closest( 'div' );
submitWrapper.style.position = 'absolute';
submitWrapper.style.bottom = '0';
button.style.marginBottom = '0';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Guard null elements and avoid brittle top equality checks.

If a submit button isn’t inside .frm_form_field, or the container has no previous sibling, getBoundingClientRect() will throw and break init. Also, strict top equality is fragile due to sub‑pixel rounding, so inline layouts may be skipped unintentionally. Consider null guards plus a small tolerance.

🔧 Proposed fix
 submitButtons.forEach( button => {
 	const submitContainer = button.closest( '.frm_form_field' );
-	const previousSibling = submitContainer.previousElementSibling;
-	if ( getTopPositionOfElement( submitContainer ) !== getTopPositionOfElement( previousSibling ) ) {
+	if ( ! submitContainer ) {
+		return;
+	}
+	const previousSibling = submitContainer.previousElementSibling;
+	if ( ! previousSibling ) {
+		return;
+	}
+	const submitTop = getTopPositionOfElement( submitContainer );
+	const prevTop = getTopPositionOfElement( previousSibling );
+	if ( Math.abs( submitTop - prevTop ) > 1 ) {
 		// If the submit button is not inline with other fields, do not update the layout.
 		return;
 	}

 	submitContainer.style.position = 'relative';
 	const submitWrapper = button.closest( 'div' );
+	if ( ! submitWrapper ) {
+		return;
+	}
 	submitWrapper.style.position = 'absolute';
 	submitWrapper.style.bottom = '0';
 	button.style.marginBottom = '0';
 } );
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@js/formidable.js` around lines 1687 - 1699, Guard against null DOM nodes and
replace strict top equality with a tolerance check: when iterating
submitButtons, ensure submitContainer (from button.closest('.frm_form_field'))
and previousSibling exist before calling getTopPositionOfElement to avoid
getBoundingClientRect() errors; then compare tops using a tolerance (e.g.,
Math.abs(getTopPositionOfElement(submitContainer) -
getTopPositionOfElement(previousSibling)) > tolerance) instead of === so
sub‑pixel differences don’t skip inline layouts; keep references to
submitContainer, previousSibling, submitWrapper, and button when applying style
changes.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (2)
js/formidable.js (2)

1688-1693: Null guards for submitContainer and previousSibling are still absent.

button.closest('.frm_form_field') returns null when the button is not inside a field container, and submitContainer.previousElementSibling can be null when there is no preceding sibling. Both cases cause getBoundingClientRect() to throw, breaking init(). The strict-equality check is also still fragile for sub-pixel layout values.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@js/formidable.js` around lines 1688 - 1693, Protect against nulls and fragile
exact-equality by checking button.closest('.frm_form_field') and its
previousElementSibling before calling getTopPositionOfElement inside init(); if
either submitContainer or previousSibling is null, skip the alignment logic.
Replace the strict equality test with a tolerant comparison such as
Math.abs(getTopPositionOfElement(submitContainer) -
getTopPositionOfElement(previousSibling)) < 1 to allow sub-pixel differences,
and only add the 'frm_inline_submit_button' class when that tolerant check
passes.

1688-1693: ⚠️ Potential issue | 🟠 Major

False-positive: hidden forms cause frm_inline_submit_button to be applied to every submit button.

getBoundingClientRect().top returns 0 for all elements inside hidden containers (e.g., Elementor popups — explicitly supported in this codebase at line 1803). During init() every hidden form's submit button satisfies 0 === 0, so frm_inline_submit_button is unconditionally added to each, regardless of actual field layout. When the popup is later revealed, the CSS rule position: absolute; bottom: 0 fires on non-inline submit buttons, mispositions them.

Add a visibility guard before comparing positions.

🔧 Proposed fix
 submitButtons.forEach( button => {
 	const submitContainer = button.closest( '.frm_form_field' );
+	if ( ! submitContainer ) {
+		return;
+	}
+	// Skip elements that are not rendered (e.g. hidden Elementor popup forms).
+	if ( submitContainer.getBoundingClientRect().height === 0 ) {
+		return;
+	}
 	const previousSibling = submitContainer.previousElementSibling;
-	if ( getTopPositionOfElement( submitContainer ) === getTopPositionOfElement( previousSibling ) ) {
+	if ( ! previousSibling ) {
+		return;
+	}
+	if ( Math.abs( getTopPositionOfElement( submitContainer ) - getTopPositionOfElement( previousSibling ) ) <= 1 ) {
 		// If the submit button is inline with other fields, vertically align it with other fields in the same row.
 		submitContainer.classList.add( 'frm_inline_submit_button' );
 	}
 } );
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@js/formidable.js` around lines 1688 - 1693, The current logic in init() that
compares getTopPositionOfElement(submitContainer) and
getTopPositionOfElement(previousSibling) can produce false positives for hidden
forms (e.g., Elementor popups) because getBoundingClientRect().top returns 0 for
invisible elements; before comparing positions, ensure visibility by checking
the submitContainer and previousSibling are visible (e.g.,
element.getClientRects().length > 0 or offsetParent !== null) and only then run
the top-position comparison and add the frm_inline_submit_button class; update
the block around submitContainer/previousSibling and getTopPositionOfElement to
short-circuit when either element is not visible.
🧹 Nitpick comments (1)
js/formidable.js (1)

1806-1810: maybeUpdateInlineSubmitButtonLayout is not re-invoked on frmPageChanged.

For multi-page forms, the submit button typically lives only on the last page. When init() runs, no .frm_button_submit elements are in the DOM yet, so the function returns immediately. After an AJAX page transition injects the last page's content, frmPageChanged fires and destroyhCaptcha runs, but maybeUpdateInlineSubmitButtonLayout is never called, so the inline-submit layout never gets applied on paginated forms.

♻️ Proposed fix
 jQuery( document ).on(
 	'frmPageChanged',
 	destroyhCaptcha
 );
+jQuery( document ).on(
+	'frmPageChanged',
+	maybeUpdateInlineSubmitButtonLayout
+);
 maybeUpdateInlineSubmitButtonLayout();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@js/formidable.js` around lines 1806 - 1810, The inline-submit layout isn't
applied after AJAX page transitions because maybeUpdateInlineSubmitButtonLayout
isn't called when frmPageChanged fires; update the event handler registration
that currently calls destroyhCaptcha so it also invokes
maybeUpdateInlineSubmitButtonLayout (e.g., call both destroyhCaptcha and
maybeUpdateInlineSubmitButtonLayout inside the frmPageChanged handler or
register a second listener), ensuring the function runs after page content is
injected so the final-page .frm_button_submit elements are detected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@css/custom_theme.css.php`:
- Around line 611-613: The rule ".with_frm_style .frm_inline_submit_button
.frm_submit .frm_button_submit { margin-bottom: 0; }" is being overridden by the
stronger ".frm_center_submit .frm_submit button { margin-bottom: 8px !important;
}"; update the inline-submit selector to win (either by matching or exceeding
specificity and/or adding !important) so margin-bottom:0 actually applies when
both classes are present—modify the ".with_frm_style .frm_inline_submit_button
.frm_submit .frm_button_submit" rule to use a higher-specificity selector or
include !important to override ".frm_center_submit .frm_submit button".

---

Duplicate comments:
In `@js/formidable.js`:
- Around line 1688-1693: Protect against nulls and fragile exact-equality by
checking button.closest('.frm_form_field') and its previousElementSibling before
calling getTopPositionOfElement inside init(); if either submitContainer or
previousSibling is null, skip the alignment logic. Replace the strict equality
test with a tolerant comparison such as
Math.abs(getTopPositionOfElement(submitContainer) -
getTopPositionOfElement(previousSibling)) < 1 to allow sub-pixel differences,
and only add the 'frm_inline_submit_button' class when that tolerant check
passes.
- Around line 1688-1693: The current logic in init() that compares
getTopPositionOfElement(submitContainer) and
getTopPositionOfElement(previousSibling) can produce false positives for hidden
forms (e.g., Elementor popups) because getBoundingClientRect().top returns 0 for
invisible elements; before comparing positions, ensure visibility by checking
the submitContainer and previousSibling are visible (e.g.,
element.getClientRects().length > 0 or offsetParent !== null) and only then run
the top-position comparison and add the frm_inline_submit_button class; update
the block around submitContainer/previousSibling and getTopPositionOfElement to
short-circuit when either element is not visible.

---

Nitpick comments:
In `@js/formidable.js`:
- Around line 1806-1810: The inline-submit layout isn't applied after AJAX page
transitions because maybeUpdateInlineSubmitButtonLayout isn't called when
frmPageChanged fires; update the event handler registration that currently calls
destroyhCaptcha so it also invokes maybeUpdateInlineSubmitButtonLayout (e.g.,
call both destroyhCaptcha and maybeUpdateInlineSubmitButtonLayout inside the
frmPageChanged handler or register a second listener), ensuring the function
runs after page content is injected so the final-page .frm_button_submit
elements are detected.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 55c730d and 5fb5860.

⛔ Files ignored due to path filters (1)
  • js/formidable-settings-components.js.map is excluded by !**/*.map
📒 Files selected for processing (3)
  • css/custom_theme.css.php
  • js/formidable-web-components.js
  • js/formidable.js

Comment on lines +611 to +613
.with_frm_style .frm_inline_submit_button .frm_submit .frm_button_submit{
margin-bottom: 0;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

margin-bottom: 0 is overridden by the existing !important rule when frm_center_submit is present.

The rule at lines 660–664 sets margin-bottom: 8px !important on .frm_center_submit .frm_submit button. Because !important wins regardless of specificity, any form that combines frm_center_submit with an inline submit button that receives frm_inline_submit_button will retain the 8 px margin, nullifying the intent of this rule.

🔧 Proposed fix
 .with_frm_style .frm_inline_submit_button .frm_submit .frm_button_submit{
-	margin-bottom: 0;
+	margin-bottom: 0 !important;
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
.with_frm_style .frm_inline_submit_button .frm_submit .frm_button_submit{
margin-bottom: 0;
}
.with_frm_style .frm_inline_submit_button .frm_submit .frm_button_submit{
margin-bottom: 0 !important;
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@css/custom_theme.css.php` around lines 611 - 613, The rule ".with_frm_style
.frm_inline_submit_button .frm_submit .frm_button_submit { margin-bottom: 0; }"
is being overridden by the stronger ".frm_center_submit .frm_submit button {
margin-bottom: 8px !important; }"; update the inline-submit selector to win
(either by matching or exceeding specificity and/or adding !important) so
margin-bottom:0 actually applies when both classes are present—modify the
".with_frm_style .frm_inline_submit_button .frm_submit .frm_button_submit" rule
to use a higher-specificity selector or include !important to override
".frm_center_submit .frm_submit button".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant