Skip to content

Improve subfield error handling #3489

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Crabcyborg wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Improve subfield error handling #3489

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
1b6e83e
Improve subfield error handling
Crabcyborg Sep 24, 2026
f7e68a6
Fix combo sub-field error handling: blur regression + repeater key co…
Sep 28, 2026
907767a
Merge master into improve_subfield_error_handling
vivi-the-going-merry[bot] Sep 30, 2026
dd3a05e
Address combo field review items in JS
vivi-the-going-merry[bot] Sep 30, 2026
2319efa
Address combo field review items in PHP and CSS
vivi-the-going-merry[bot] Sep 30, 2026
c21df3f
Avoid for...of in the error container lookup
vivi-the-going-merry[bot] Sep 30, 2026
85ddc78
Take the aria-describedby ID from the rendered error HTML
vivi-the-going-merry[bot] Sep 30, 2026
f82ae8f
Tighten combo sub field key match and word count
vivi-the-going-merry[bot] Sep 30, 2026
7a3abd0
Fix lint: JSDoc alignment and short function blank line
vivi-the-going-merry[bot] Sep 30, 2026
c6b94c3
Merge master into improve_subfield_error_handling
vivi-the-going-merry[bot] Oct 1, 2026
5164e8d
Keep server errors on other combo sub fields when one changes
vivi-the-going-merry[bot] Oct 1, 2026
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
8 changes: 8 additions & 0 deletions classes/controllers/FrmComboFieldsController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,14 @@ public static function add_atts_to_input( $atts ) {
$atts['sub_field']['atts']['disabled'] = 'disabled';
}

// Lets FrmFieldsController::add_shortcodes_to_html() pick this sub field's own aria-invalid
// value, and name this sub field in its required message for JS validation.
$atts['field']['subfield_name'] = $atts['key'];

if ( $field_obj instanceof FrmFieldCombo ) {
$atts['field']['subfield_label'] = $field_obj->get_sub_field_label( $atts['key'] );

@franky-the-going-merry franky-the-going-merry Bot Oct 1, 2026

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.

The JS and server messages for a sub field can differ once its description is customised. Reproduced on the preview env with a required Name field whose last_desc is set to "Last":

  • Server-side error and summary link: "Last cannot be blank." (uses the description, as get_sub_field_label() intends).
  • This line's data-reqmsg, which JS validation shows: "Last Name cannot be blank."

With the default descriptions (the same text as the sub field label) the two happen to match, so the stated goal of consistent errors holds out of the box but not after someone edits a description. Likely cause, not confirmed: the field array in $atts['field'] at this hook does not carry the *_desc options or the field name, so get_sub_field_label() falls to the sub field label only. If so, build $field_obj from the full field (or have get_sub_field_label() read the saved options) so this line gets the same string as FrmFieldCombo::get_sub_field_error_msg().

}

if ( ! empty( $atts['sub_field']['optional'] ) ) {
add_filter( 'frm_field_classes', 'FrmAddressesController::add_optional_class', 20, 2 );
do_action( 'frm_field_input_html', $atts['field'] );
Expand Down
14 changes: 13 additions & 1 deletion classes/controllers/FrmEntriesAJAXSubmitController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,18 @@ private static function print_ajax_scripts() {
* @return string
*/
private static function maybe_modify_ajax_error( $error, $field_id, $form, $errors ) {
$sub_field = '';

if ( ! is_numeric( $field_id ) && preg_match( '/^(\d+)-([a-z]\w*)$/', $field_id, $matches ) ) {
// An error for a combo sub field, like 12-first. An empty one only flags the sub field.
if ( '' === $error ) {
return $error;
}

$field_id = $matches[1];
$sub_field = $matches[2];
}

if ( ! is_numeric( $field_id ) ) {
return $error;
}
Expand All @@ -186,7 +198,7 @@ private static function maybe_modify_ajax_error( $error, $field_id, $form, $erro

if ( false !== $error_body ) {
$error = str_replace( '[error]', $error, $error_body );
$error = str_replace( '[key]', $use_field['field_key'], $error );
$error = str_replace( '[key]', $use_field['field_key'] . ( $sub_field ? '_' . $sub_field : '' ), $error );
}

return $error;
Expand Down
8 changes: 7 additions & 1 deletion classes/controllers/FrmFieldsController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -964,7 +964,13 @@ private static function add_validation_messages( $field, array &$add_html ) {
$field_validation_messages_status = self::get_validation_data_attribute_visibility_info( $field );

if ( FrmField::is_required( $field ) && ! empty( $field_validation_messages_status['data-reqmsg'] ) ) {
$required_message = FrmFieldsHelper::get_error_msg( $field, 'blank' );
if ( empty( $field['subfield_label'] ) ) {
$required_message = FrmFieldsHelper::get_error_msg( $field, 'blank' );
} else {
// A combo sub field, like a first name, is named on its own in its required message.
$required_message = FrmFieldsHelper::get_error_msg_for_name( $field, 'blank', $field['subfield_label'] );
}

$add_html['data-reqmsg'] = 'data-reqmsg="' . esc_attr( $required_message ) . '"';
self::maybe_add_html_required( $field, $add_html );
}
Expand Down
22 changes: 22 additions & 0 deletions classes/helpers/FrmFieldsHelper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,28 @@ public static function fill_field( &$values, $field, $form_id, $new_key = '' ) {
}
}

/**
* Gets an error message with a different name in place of the field name.
*
* @since x.x
*
* @param array|object $field
* @param string $error
* @param string $name The name to use in the message.
*
* @return string
*/
public static function get_error_msg_for_name( $field, $error, $name ) {
if ( is_object( $field ) ) {
$field = clone $field;
$field->name = $name;
} elseif ( is_array( $field ) ) {
$field['name'] = $name;
}

return self::get_error_msg( $field, $error );
}

/**
* @since 2.0
*
Expand Down
108 changes: 99 additions & 9 deletions classes/models/fields/FrmFieldCombo.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,8 @@ protected function print_input_atts( $args ) {
$field['default_value'] = '';

if ( ! empty( $sub_field['name'] ) ) {
$field['subfield_name'] = $sub_field['name'];
$field['subfield_name'] = $sub_field['name'];
$field['subfield_label'] = $this->get_sub_field_label( $sub_field['name'] );
}

do_action( 'frm_field_input_html', $field );
Expand Down Expand Up @@ -437,22 +438,104 @@ public function validate( $args ) {
return $errors;
}

$blank_msg = FrmFieldsHelper::get_error_msg( $this->field, 'blank' );
$sub_fields = $this->get_processed_sub_fields();
$required_count = 0;
$missing = array();

// Validate not empty.
foreach ( $sub_fields as $name => $sub_field ) {
if ( ! empty( $sub_field['optional'] ) || ! empty( $args['value'][ $name ] ) ) {
foreach ( $this->get_processed_sub_fields() as $name => $sub_field ) {
if ( ! empty( $sub_field['optional'] ) ) {
continue;
}

$errors[ 'field' . $args['id'] . '-' . $name ] = '';
$errors[ 'field' . $args['id'] ] = $blank_msg;
++$required_count;

if ( empty( $args['value'][ $name ] ) ) {
$missing[] = $name;
}
}

if ( ! $missing ) {
return $errors;
}

if ( count( $missing ) === $required_count ) {
// Nothing was filled in, so show one error for the whole field. The empty sub field
// errors flag each required input without repeating the message under every one.
foreach ( $missing as $name ) {
$errors[ 'field' . $args['id'] . '-' . $name ] = '';
}

$errors[ 'field' . $args['id'] ] = FrmFieldsHelper::get_error_msg( $this->field, 'blank' );

return $errors;
}

// Only some sub fields are missing, so name each one in its own error.
foreach ( $missing as $name ) {
$errors[ 'field' . $args['id'] . '-' . $name ] = $this->get_sub_field_error_msg( $name, 'blank' );
}

return $errors;
}

/**
* Gets the label a sub field is referred to by in its error messages.
*
* The sub field description is used when it is a short phrase, since that is the label shown
* under the input. Otherwise the sub field label is combined with the field label, like "Address Line 1".
*
* @since x.x
*
* @param string $name Sub field name, like 'first' or 'line1'.
*
* @return string
*/
public function get_sub_field_label( $name ) {

@franky-the-going-merry franky-the-going-merry Bot Sep 26, 2026

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.

get_sub_field_label() uses the sub-field's {name}_desc option verbatim as the grammatical subject substituted into the blank-message template ([field_name] cannot be blank.) via FrmFieldsHelper::get_error_msg(). That option is authored as the caption shown under the input, not necessarily a short noun phrase — an admin who writes an actual sentence there (e.g. "We use this to personalize your emails.") gets "We use this to personalize your emails. cannot be blank." as the live per-sub-field error.

Not a regression against the old code (which never substituted per-sub-field description text into a sentence at all), so non-blocking — but worth a fallback to the sub-field's plain label when the description doesn't read as a short phrase, or at least a doc note on the authoring convention this now assumes.

@vivi-the-going-merry vivi-the-going-merry Bot Sep 30, 2026

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.

Fixed. The description is only used when it is a short phrase (4 words or fewer, no trailing sentence punctuation); otherwise it falls back to the field + sub field label. Added a PHPUnit case. Also shared the clone-and-rename error message code between FrmFieldCombo and FrmFieldsController via FrmFieldsHelper::get_error_msg_for_name, and the AJAX [if error] wrapper now applies to keys like 12-first.

$desc = FrmField::get_option( $this->field, $name . '_desc' );

if ( is_string( $desc ) && $this->is_short_phrase( $desc ) ) {
return $desc;
}

$label = $this->sub_fields[ $name ]['label'] ?? '';

if ( ! $label ) {
return (string) $this->get_field_column( 'name' );
}

/* translators: 1: Field label, 2: Sub field label */
return sprintf( __( '%1$s %2$s', 'formidable' ), $this->get_field_column( 'name' ), $label );
}

/**
* Checks if text reads as a label, so it can be the subject of an error message. A sentence
* would not.
*
* @since x.x
*
* @param string $text
*
* @return bool
*/
private function is_short_phrase( $text ) {
$text = trim( $text );
return '' !== $text && count( (array) preg_split( '/\s+/', $text ) ) <= 4 && ! preg_match( '/[.!?:;]$/', $text );
}

/**
* Gets an error message for a single sub field, with the sub field label in place of the
* field label.
*
* @since x.x
*
* @param string $name Sub field name, like 'first' or 'line1'.
* @param string $error Error type, like 'blank'.
*
* @return string
*/
public function get_sub_field_error_msg( $name, $error ) {
return FrmFieldsHelper::get_error_msg_for_name( $this->field, $error, $this->get_sub_field_label( $name ) );
}

/**
* Gets export headings.
*
Expand Down Expand Up @@ -507,10 +590,17 @@ protected function should_print_hidden_sub_fields() {
* @return array
*/
public function get_inputs_container_attrs() {
return array(
$attrs = array(
'class' => 'frm_combo_inputs_container',
'id' => 'frm_combo_inputs_container_' . $this->field_id,
);

if ( $this->field && $this->get_field_column( 'required' ) ) {
// JS validation shows this for the whole field when every required sub field is empty.
$attrs['data-reqmsg'] = FrmFieldsHelper::get_error_msg( $this->field, 'blank' );
}

return $attrs;
}

/**
Expand Down
21 changes: 21 additions & 0 deletions css/_single_theme.css.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,27 @@
border-style:var(--border-style-error)<?php echo esc_html( $important ); ?>;
}

/* A combo field error only marks the sub fields that failed, not optional or filled ones. */
.<?php echo esc_html( $style_class ); ?> .frm_blank_field .frm_combo_inputs_container > .frm_form_field:not(.frm_blank_field) input:not(:focus),

@deepsource-io deepsource-io Bot Oct 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Variable $style_class might not be defined


A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.

@deepsource-io deepsource-io Bot Oct 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Variable $style_class might not be defined


A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.

.<?php echo esc_html( $style_class ); ?> .frm_blank_field .frm_combo_inputs_container > .frm_form_field:not(.frm_blank_field) select:not(:focus),

@deepsource-io deepsource-io Bot Oct 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Variable $style_class might not be defined


A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.

@deepsource-io deepsource-io Bot Oct 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Variable $style_class might not be defined


A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.

.<?php echo esc_html( $style_class ); ?> .frm_blank_field .frm_combo_inputs_container > .frm_form_field:not(.frm_blank_field) textarea:not(:focus),

@deepsource-io deepsource-io Bot Oct 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Variable $style_class might not be defined


A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.

@deepsource-io deepsource-io Bot Oct 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Variable $style_class might not be defined


A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.

<?php if ( $pro_is_installed ) { ?>
.<?php echo esc_html( $style_class ); ?> .frm_blank_field .frm_combo_inputs_container > .frm_form_field:not(.frm_blank_field) .mce-edit-area iframe,

@deepsource-io deepsource-io Bot Oct 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Variable $style_class might not be defined


A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.

@deepsource-io deepsource-io Bot Oct 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Variable $style_class might not be defined


A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.

<?php } ?>
<?php if ( $use_chosen_js ) { ?>
.<?php echo esc_html( $style_class ); ?> .frm_blank_field .frm_combo_inputs_container > .frm_form_field:not(.frm_blank_field) .chosen-container-multi .chosen-choices,

@deepsource-io deepsource-io Bot Oct 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Variable $style_class might not be defined


A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.

@deepsource-io deepsource-io Bot Oct 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Variable $style_class might not be defined


A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.

.<?php echo esc_html( $style_class ); ?> .frm_blank_field .frm_combo_inputs_container > .frm_form_field:not(.frm_blank_field) .chosen-container-single .chosen-single,

@deepsource-io deepsource-io Bot Oct 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Variable $style_class might not be defined


A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.

@deepsource-io deepsource-io Bot Oct 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Variable $style_class might not be defined


A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.

<?php } ?>
.<?php echo esc_html( $style_class ); ?> .frm_blank_field .frm_combo_inputs_container > .frm_form_field:not(.frm_blank_field) .frm-g-recaptcha iframe,

@deepsource-io deepsource-io Bot Oct 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Variable $style_class might not be defined


A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.

@deepsource-io deepsource-io Bot Oct 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Variable $style_class might not be defined


A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.

.<?php echo esc_html( $style_class ); ?> .frm_blank_field .frm_combo_inputs_container > .frm_form_field:not(.frm_blank_field) .g-recaptcha iframe,

@deepsource-io deepsource-io Bot Oct 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Variable $style_class might not be defined


A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.

@deepsource-io deepsource-io Bot Oct 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Variable $style_class might not be defined


A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.

.<?php echo esc_html( $style_class ); ?> .frm_blank_field .frm_combo_inputs_container > .frm_form_field:not(.frm_blank_field) .frm-card-element.StripeElement {

@deepsource-io deepsource-io Bot Oct 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Variable $style_class might not be defined


A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.

@deepsource-io deepsource-io Bot Oct 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Variable $style_class might not be defined


A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.

color:var(--text-color)<?php echo esc_html( $important ); ?>;
background-color:var(--bg-color)<?php echo esc_html( $important ); ?>;
border-color:var(--border-color)<?php echo esc_html( $important ); ?>;
border-width:var(--field-border-width)<?php echo esc_html( $important ); ?>;
border-style:var(--field-border-style)<?php echo esc_html( $important ); ?>;
}

<?php
// Only include this style when the signatures add-on is active
if ( class_exists( 'FrmSigField' ) ) :
Expand Down
Loading
Loading