Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions classes/controllers/FrmFormActionsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,21 @@ private static function should_show_log_message( $action_type ) {
return in_array( $action_type, $logging, true ) && ! function_exists( 'frm_log_autoloader' );
}

/**
* @since x.x
*
* @param object $form_action
*
* @return bool
*/
public static function should_show_notice_about_using_the_same_to_from_email( $form_action ) {
$settings = new FrmSettings();
if ( ! empty( $settings->default_email ) && $settings->default_email !== $settings->from_email ) {
return false;
}
return $form_action->post_excerpt === 'email' && ! get_user_meta( wp_get_current_user()->ID, 'frm_dismiss_default_email_message', true );
}

/**
* @param int|string $form_id
* @param array $values
Expand Down
17 changes: 17 additions & 0 deletions classes/controllers/FrmFormsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -2677,6 +2677,23 @@ public static function get_met_on_submit_actions( $args, $event = 'create' ) {
return $met_actions;
}

/**
* Handles the AJAX request to dismiss the default email message.
*
* @since x.x
*
* @return void
*/
public static function dismiss_default_email_message() {
$permission_error = FrmAppHelper::permission_nonce_error( 'frm_edit_forms', 'nonce', 'frm_ajax' );

if ( $permission_error !== false ) {
wp_send_json_error( $permission_error, 403 );
}
update_user_meta( get_current_user_id(), 'frm_dismiss_default_email_message', 1 );
wp_send_json_success();
}

/**
* Checks if a Confirmation action has the valid data.
*
Expand Down
1 change: 1 addition & 0 deletions classes/controllers/FrmHooksController.php
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,7 @@ public static function load_ajax_hooks() {
add_action( 'wp_ajax_frm_lite_settings_upgrade', 'FrmSettingsController::settings_cta_dismiss' );
add_action( 'wp_ajax_frm_settings_tab', 'FrmSettingsController::load_settings_tab' );
add_action( 'wp_ajax_frm_page_search', 'FrmSettingsController::page_search' );
add_action( 'wp_ajax_frm_dismiss_default_email_message', 'FrmFormsController::dismiss_default_email_message' );

// Styles Controller.
add_action( 'wp_ajax_frm_settings_reset', 'FrmStylesController::reset_styling' );
Expand Down
15 changes: 15 additions & 0 deletions classes/views/frm-form-actions/_action_inside.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,21 @@
<input type="hidden" name="<?php echo esc_attr( $action_control->get_field_name( 'ID', '' ) ); ?>" value="<?php echo esc_attr( $form_action->ID ); ?>" />

<?php
if ( FrmFormActionsController::should_show_notice_about_using_the_same_to_from_email( $form_action ) ) {
?>
<div class="frm_no_p_margin frm_default_email_message frm-flex frm-gap-sm">
<p>
<b style="color:var(--grey-800);"><?php esc_html_e( 'Heads up!', 'formidable' ); ?></b>
<span><?php esc_html_e( 'Using the same \'To\' and \'From\' email address can sometimes cause delivery issues. We recommend updating your default email addresses to maximize deliverability.', 'formidable' ); ?></span>
</p>
<p class="frm-flex-box frm-flex-center" style="white-space:nowrap;">
<a href="#" class="frm_dismiss_default_email_message button frm-button-secondary"><?php esc_html_e( 'Got it', 'formidable' ); ?></a>
<a href="<?php echo esc_url( admin_url( 'admin.php?page=formidable-settings' ) ); ?>" class="button frm-button-primary"><?php esc_html_e( 'Setup emails', 'formidable' ); ?></a>
</p>
</div>
<?php
}

/**
* @since 6.25
*
Expand Down
2 changes: 1 addition & 1 deletion css/admin/frm-settings-components.css

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions css/frm_admin.css

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion css/frm_testing_mode.css

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion js/addons-page.js

Large diffs are not rendered by default.

23 changes: 23 additions & 0 deletions js/admin/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
sendTestEmailModal: null
};

const { doJsonPost } = frmDom.ajax;
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 | 🔴 Critical

Duplicate handleClickEvent definitions break the new dismiss behavior

You now have two top‑level function handleClickEvent declarations:

  • Lines 34–49: handles .frm_dismiss_default_email_message.
  • Lines 55–69: existing handler for email styles / test email.

Because function declarations are hoisted, the second definition (lines 55–69) overwrites the first. The document click listener wired in addEventListeners() will call only the second version, so clicks on the “Got it” link never trigger the dismiss AJAX call.

This effectively disables the new banner dismissal.

I’d suggest merging the logic into a single handleClickEvent:

-	function handleClickEvent( e ) {
-		if ( ! e.target.classList.contains( 'frm_dismiss_default_email_message' ) ) {
-			return;
-		}
-		e.preventDefault();
-		const formData = new FormData();
-		formData.append( 'action', 'frm_dismiss_default_email_message' );
-		formData.append( 'nonce', frmGlobal.nonce );
-		doJsonPost( 'dismiss_default_email_message', formData )
-		.then( () => {
-			e.target.closest( '.frm_default_email_message' ).remove();
-		})
-		.catch( error => {
-			console.error( error );
-		});
-	}
-
-	function handleClickEvent( e ) {
+	function handleClickEvent( e ) {
+		// Dismiss default email notice.
+		if ( e.target.classList.contains( 'frm_dismiss_default_email_message' ) ) {
+			e.preventDefault();
+
+			const formData = new FormData();
+			// If `doJsonPost` already appends action/nonce, you can omit these two lines.
+			formData.append( 'action', 'frm_dismiss_default_email_message' );
+			formData.append( 'nonce', frmGlobal.nonce );
+
+			doJsonPost( 'dismiss_default_email_message', formData )
+				.then( () => {
+					const notice = e.target.closest( '.frm_default_email_message' );
+					if ( notice ) {
+						notice.remove();
+					}
+				} )
+				.catch( error => {
+					// Keep failure non‑fatal for the settings page.
+					console.error( error );
+				} );
+			return;
+		}
+
 		if ( 'BUTTON' === e.target.nodeName && 'choose' === e.target.dataset.action && e.target.closest( '.frm-email-style' ) ) {
 			handleClickChooseEmailStyle( e );
 			return;
 		}
@@
 		if ( 'frm-send-test-email' === e.target.id ) {
 			showSendTestEmailModal( e );
 			return;
 		}
@@
 		if ( 'frm-send-test-email-btn' === e.target.id ) {
 			handleClickSendTestEmailBtn( e );
 		}
 	}

Optionally, you could also switch the send_test_email call at the bottom to use the same doJsonPost alias for consistency.

Also applies to: 34-49, 55-69

🤖 Prompt for AI Agents
In js/admin/settings.js (conflicting function declarations at lines 34-49 and
55-69), there are two top-level function handleClickEvent definitions where the
second overwrites the first, preventing the new banner dismiss AJAX from firing;
fix this by merging both handlers into a single handleClickEvent function that
tests the clicked element for both cases (the .frm_dismiss_default_email_message
dismiss link and the existing email-style/test-email targets) and executes the
appropriate doJsonPost call for each branch, remove the duplicate declaration so
only one function exists, ensure addEventListeners() binds that single handler,
and optionally replace the send_test_email call at the bottom to use the
doJsonPost alias for consistency.


function addEventListeners() {
document.addEventListener( 'change', handleChangeEvent );
document.addEventListener( 'keydown', handleKeyDownEvent );
Expand All @@ -29,6 +31,23 @@
}
}

function handleClickDismissDefaultEmailMessage( e ) {
if ( ! e.target.classList.contains( 'frm_dismiss_default_email_message' ) ) {
return;
}
e.preventDefault();
const formData = new FormData();
formData.append( 'action', 'frm_dismiss_default_email_message' );
formData.append( 'nonce', frmGlobal.nonce );
doJsonPost( 'dismiss_default_email_message', formData )
.then( () => {
e.target.closest( '.frm_default_email_message' ).remove();
} )
.catch( error => {
console.error( error );
} );
}
Comment on lines +34 to +49
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 | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find where handleClickDismissDefaultEmailMessage is called from
rg -n 'handleClickDismissDefaultEmailMessage' --type=js -B 3 -A 3

Repository: Strategy11/formidable-forms

Length of output: 751


🏁 Script executed:

#!/bin/bash
# Find doJsonPost implementation and usage patterns
rg -n 'doJsonPost\s*[:=]' --type=js -A 15

Repository: Strategy11/formidable-forms

Length of output: 927


🏁 Script executed:

#!/bin/bash
# Also check all doJsonPost calls to understand the pattern
rg -n 'doJsonPost\s*\(' --type=js -B 2 -A 2

Repository: Strategy11/formidable-forms

Length of output: 6584


🏁 Script executed:

#!/bin/bash
# Check how FormData with 'action' field is typically handled in WordPress
# Let's see if there are other patterns where action is appended to FormData
rg -n "formData\.append\(\s*['\"]action['\"]" --type=js -B 5 -A 5

Repository: Strategy11/formidable-forms

Length of output: 2570


🏁 Script executed:

#!/bin/bash
# Read more context from settings.js around the test email handler for comparison
sed -n '100,130p' js/admin/settings.js | cat -n

Repository: Strategy11/formidable-forms

Length of output: 1256


🏁 Script executed:

#!/bin/bash
# Look for ESLint config to understand the violations
fd -e '.eslintrc*' -e 'eslint.config*' --type=f

Repository: Strategy11/formidable-forms

Length of output: 53


🏁 Script executed:

#!/bin/bash
# Check if there's a package.json with ESLint config
cat package.json 2>/dev/null | grep -A 20 '"eslint'

Repository: Strategy11/formidable-forms

Length of output: 53


Based on my verification, I can now provide the rewritten review comment with confirmed findings.


Remove redundant class check, redundant FormData fields, and add defensive null check.

This function duplicates checks and appends unnecessary FormData fields:

  1. Redundant class check (lines 35-37): The caller at line 69 already verifies the target has class frm_dismiss_default_email_message before invoking this function.

  2. Redundant FormData fields (lines 40-41):

    • Line 41 (nonce): doJsonPost internally appends nonce at line 125 of js/admin/dom.js. Appending it here creates a duplicate.
    • Line 40 (action): The action parameter passed to doJsonPost is used for URL construction (?action=frm_dismiss_default_email_message), not FormData. Other functions using doJsonPost (e.g., test email handler at line 115) don't append the action to FormData.
  3. Missing defensive null check (line 44): .closest() can return null if the DOM structure changes. Add a null check before calling .remove().

  4. Silent error handling (lines 47-48): Unlike the test email handler (lines 117-119) which provides user-facing feedback, errors here are only logged to console. Consider consistent error messaging.

Apply this diff:

 function handleClickDismissDefaultEmailMessage( e ) {
-	if ( ! e.target.classList.contains( 'frm_dismiss_default_email_message' ) ) {
-		return;
-	}
 	e.preventDefault();
+
 	const formData = new FormData();
-	formData.append( 'action', 'frm_dismiss_default_email_message' );
-	formData.append( 'nonce', frmGlobal.nonce );
+
 	doJsonPost( 'dismiss_default_email_message', formData )
-	.then( () => {
-		e.target.closest( '.frm_default_email_message' ).remove();
-	})
-	.catch( error => {
-		console.error( error );
-	});
+		.then( () => {
+			const notice = e.target.closest( '.frm_default_email_message' );
+			if ( notice ) {
+				notice.remove();
+			}
+		} )
+		.catch( error => {
+			console.error( error );
+		} );
 }
📝 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
function handleClickDismissDefaultEmailMessage( e ) {
if ( ! e.target.classList.contains( 'frm_dismiss_default_email_message' ) ) {
return;
}
e.preventDefault();
const formData = new FormData();
formData.append( 'action', 'frm_dismiss_default_email_message' );
formData.append( 'nonce', frmGlobal.nonce );
doJsonPost( 'dismiss_default_email_message', formData )
.then( () => {
e.target.closest( '.frm_default_email_message' ).remove();
})
.catch( error => {
console.error( error );
});
}
function handleClickDismissDefaultEmailMessage( e ) {
e.preventDefault();
const formData = new FormData();
doJsonPost( 'dismiss_default_email_message', formData )
.then( () => {
const notice = e.target.closest( '.frm_default_email_message' );
if ( notice ) {
notice.remove();
}
} )
.catch( error => {
console.error( error );
} );
}
🧰 Tools
🪛 GitHub Check: Run ESLint

[failure] 48-48:
There must be a space before this paren


[failure] 48-48:
Expected indentation of 3 tabs but found 2


[failure] 47-47:
Expected indentation of 4 tabs but found 3


[failure] 46-46:
Expected indentation of 3 tabs but found 2


[failure] 45-45:
There must be a space before this paren


[failure] 45-45:
Expected indentation of 3 tabs but found 2


[failure] 44-44:
Expected indentation of 4 tabs but found 3


[failure] 43-43:
Expected indentation of 3 tabs but found 2

🤖 Prompt for AI Agents
In js/admin/settings.js around lines 34 to 49, remove the redundant class check
at the top (the caller already ensures the target has
'frm_dismiss_default_email_message'), stop appending both 'action' and 'nonce'
to the FormData (doJsonPost builds the URL with the action and appends nonce
internally), add a defensive null check before calling .remove() on
e.target.closest('.frm_default_email_message'), and replace the silent
console.error with the same user-facing error handling used by the test email
handler (e.g., surface an error message to the user) so failures are visible.


function handleToggleChangeEvent( e ) {
e.target.nextElementSibling.setAttribute( 'aria-checked', e.target.checked ? 'true' : 'false' );
}
Expand All @@ -47,6 +66,10 @@
if ( 'frm-send-test-email-btn' === e.target.id ) {
handleClickSendTestEmailBtn( e );
}

if ( e.target.classList.contains( 'frm_dismiss_default_email_message' ) ) {
handleClickDismissDefaultEmailMessage( e );
}
}

function handleClickChooseEmailStyle( e ) {
Expand Down
2 changes: 1 addition & 1 deletion js/form-templates.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion js/formidable-settings-components.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion js/formidable_admin.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion js/formidable_blocks.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion js/formidable_dashboard.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion js/formidable_overlay.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion js/formidable_styles.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion js/frm_testing_mode.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion js/onboarding-wizard.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion js/welcome-tour.js

Large diffs are not rendered by default.

6 changes: 6 additions & 0 deletions resources/scss/admin/components/form/_form-actions.scss
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@
padding-top: 0;
}

.frm_email_settings .frm_default_email_message {
background-color: var(--primary-25);
padding:var(--gap-sm) var(--gap-md);
border-radius:var(--small-radius);
}

.frm_email_settings.frm_email_settings.widgets-holder-wrap {
overflow: auto;
box-shadow: none;
Expand Down
7 changes: 7 additions & 0 deletions resources/scss/admin/media-queries/_screen-desktop.scss
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,11 @@
/* There isn't enough room for the title on a screen this size so just hide it. */
display: none !important;
}

.frm_default_email_message {
flex-direction: column;
}
.frm_default_email_message .frm-flex-box.frm-flex-center {
justify-content: start;
}
}
Loading