enhance: repeat field enhancement #1389
Conversation
- Reactivity for field title, help etc fixed
Slight optimization when removing the fields
Rat01047
added
needs: testing
needs: developer feedback
sapayth
added
needs: testing
and removed
bug
needs: dev review
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
wpuf-functions.php (2)
1802-1838: Extract backward compatibility logic for better maintainabilityThe nested conditionals and field reassignments make this code difficult to understand and maintain. Consider refactoring as suggested in the previous review.
924-957: Consider extracting repeat field rendering logic into a separate functionThe nested loop structure for rendering repeat fields (lines 925-957) adds significant complexity to the
wpuf_show_custom_fieldsfunction. This duplicates the pattern used for column fields and makes the function harder to maintain.Extract the repeat field rendering logic into a dedicated function:
+/** + * Render repeat field data for post display + * + * @param WP_Post $post + * @param array $attr + * @return string + */ +function wpuf_render_repeat_field_data( $post, $attr ) { + $html = ''; + $inner_fields = $attr['inner_fields']; + + foreach ( $inner_fields as $column_key => $column_fields ) { + if ( ! empty( $column_fields ) ) { + foreach ( $column_fields as $column_field_key => $column_field ) { + if ( isset( $column_field['show_in_post'] ) && 'yes' === $column_field['show_in_post'] ) { + $repeat_field_name = ! empty( $attr['name'] ) ? $attr['name'] : ''; + $repeat_rows = get_post_meta( $post->ID, $repeat_field_name, true ); + + if ( ! empty( $repeat_rows ) ) { + for ( $index = 0; $index < $repeat_rows; $index++ ) { + $field_value = get_post_meta( $post->ID, $repeat_field_name . '_' . $index . '_' . $column_field['name'], true ); + $hide_label = ! empty( $column_field['hide_field_label'] ) ? $column_field['hide_field_label'] : 'no'; + + if ( is_array( $field_value ) ) { + $field_value = implode( ', ', $field_value ); + } + + $html .= '<li>'; + + if ( 'no' === $hide_label ) { + $html .= '<label>' . $column_field['label'] . ': </label>'; + } + + $html .= make_clickable( $field_value ) . '</li>'; + } + } + } + } + } + } + + return $html; +} // In wpuf_show_custom_fields function, replace lines 924-957 with: if ( 'repeat' === $attr['input_type'] ) { - $inner_fields = $attr['inner_fields']; - foreach ( $inner_fields as $column_key => $column_fields ) { - // ... existing code ... - } + $html .= wpuf_render_repeat_field_data( $post, $attr ); }admin/form-builder/assets/js/form-builder.js (1)
138-138: UseObject.hasOwn()instead ofhasOwnProperty.The past review comment about replacing
hasOwnPropertywithObject.hasOwn()still applies to these lines, including the new repeat field code.Also applies to: 143-143, 164-164
🧹 Nitpick comments (1)
wpuf-functions.php (1)
1813-1822: Remove redundant template assignmentLine 1813 sets
$field['template'] = 'text_field'but this is immediately overwritten at line 1822 with$field['template'] = 'repeat_field'. This creates confusion about the intended behavior.Remove the redundant assignment:
$new_id = wpuf_form_field_id_generator(); -$field['template'] = 'text_field'; // set the new compatible values $field['id'] = $new_id; $field['name'] = $old_meta . '_' . $new_id; $field['label'] = ''; +// The inner field will have text_field template +$inner_field = $field; +$inner_field['template'] = 'text_field'; -$field['inner_fields']['column-1'] = [ $field ]; +$field['inner_fields']['column-1'] = [ $inner_field ]; $field['inner_fields']['column-2'] = []; $field['inner_fields']['column-3'] = []; $field['template'] = 'repeat_field';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
admin/form-builder/assets/js/components/builder-stage-v4-1/template.php(2 hunks)admin/form-builder/assets/js/form-builder.js(6 hunks)assets/css/admin.css(1 hunks)assets/js-templates/form-components.php(2 hunks)assets/js/wpuf-form-builder-wpuf-forms.js(1 hunks)assets/js/wpuf-form-builder.js(6 hunks)assets/less/admin.less(1 hunks)wpuf-functions.php(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- assets/css/admin.css
- assets/js-templates/form-components.php
- admin/form-builder/assets/js/components/builder-stage-v4-1/template.php
- assets/js/wpuf-form-builder-wpuf-forms.js
- assets/less/admin.less
- assets/js/wpuf-form-builder.js
🔇 Additional comments (5)
admin/form-builder/assets/js/form-builder.js (5)
142-146: LGTM! Addresses meta_key retention issue.This addition correctly prevents modification of existing meta keys for column fields, which aligns with issue #1240 mentioned in the PR objectives. The logic is consistent with the same check performed for regular form fields.
157-181: LGTM! Well-structured repeat field support.This implementation correctly extends the
update_editing_form_fieldmutation to support repeat fields, following the same pattern as column fields. The defensiveArray.isArray()check and proper handling of field property interactions ('read_only' and 'required') demonstrate good coding practices.
437-470: LGTM! Well-implemented repeat field mutations.These new mutations (
add_repeat_inner_field_element,delete_repeat_inner_field_element,clone_repeat_inner_field_element) are well-structured and follow established patterns from the existing column field mutations. The defensive checks, proper use ofVue.setfor reactivity, and correct clone operation handling demonstrate good coding practices.
549-568: LGTM! Consistent repeat field settings implementation.This mutation correctly implements the repeat field settings functionality, following the same pattern as the existing
open_column_field_settingsmutation. The state management and timing approach are consistent with established patterns.
923-923: LGTM! Minor formatting improvement.
| function wpuf_form_field_id_generator( $min = 999999, $max = 9999000001 ) { | ||
| return rand( $min, $max ); | ||
| } |
There was a problem hiding this comment.
Fix potential integer overflow and improve ID generation
The default maximum value 9999000001 exceeds PHP's 32-bit integer limit (PHP_INT_MAX = 2147483647 on 32-bit systems), which could cause unexpected behavior.
Apply this fix to ensure compatibility and better ID generation:
-function wpuf_form_field_id_generator( $min = 999999, $max = 9999000001 ) {
- return rand( $min, $max );
+function wpuf_form_field_id_generator( $min = 999999, $max = null ) {
+ if ( null === $max ) {
+ $max = min( 2147483647, PHP_INT_MAX ); // Ensure 32-bit compatibility
+ }
+
+ // Validate parameters
+ $min = max( 0, intval( $min ) );
+ $max = min( $max, PHP_INT_MAX );
+
+ if ( $min >= $max ) {
+ $min = 0;
+ }
+
+ // Use more random generation if available
+ if ( function_exists( 'random_int' ) ) {
+ return random_int( $min, $max );
+ }
+
+ return mt_rand( $min, $max );
}Additionally, consider implementing a mechanism to track generated IDs to prevent duplicates within the same request.
📝 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.
| function wpuf_form_field_id_generator( $min = 999999, $max = 9999000001 ) { | |
| return rand( $min, $max ); | |
| } | |
| function wpuf_form_field_id_generator( $min = 999999, $max = null ) { | |
| if ( null === $max ) { | |
| // Ensure compatibility on 32-bit PHP builds | |
| $max = min( 2147483647, PHP_INT_MAX ); | |
| } | |
| // Validate and sanitize parameters | |
| $min = max( 0, intval( $min ) ); | |
| $max = min( $max, PHP_INT_MAX ); | |
| if ( $min >= $max ) { | |
| $min = 0; | |
| } | |
| // Prefer cryptographically secure random if available | |
| if ( function_exists( 'random_int' ) ) { | |
| return random_int( $min, $max ); | |
| } | |
| return mt_rand( $min, $max ); | |
| } |
🤖 Prompt for AI Agents
In wpuf-functions.php around lines 1862 to 1864, the function uses a max value
for rand() that exceeds the 32-bit integer limit, risking overflow and
unexpected behavior. Adjust the max value to be within PHP_INT_MAX for
compatibility across systems. Also, implement a mechanism such as a static or
global array to track generated IDs during the request and ensure no duplicates
are returned by regenerating IDs if a duplicate is detected.
Rubaiyat-E-Mohammad
added
QA In Progress
bug
needs: dev review
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
assets/css/admin/form-builder.css (1)
1000-1015: Same duplication pattern inside.wpuf-dropdown-contentThe three consecutive
transition-propertylines repeat exactly the values already defined (Lines 1009-1013). Keep only one as shown above to silence the linter and trim bytes.
🧹 Nitpick comments (2)
assets/css/admin/form-builder.css (2)
846-872: Duplicatetransition-propertyentries – keep only oneWithin
.wpuf-badgethe sametransition-propertyis declared three times (Lines 850-856). They are identical and bring no cross-browser benefit, so they just bloat the stylesheet and trigger linter errors (noDuplicateProperties). Drop the two extras:- transition-property: color, background-color, border-color, text-decoration-color, fill, stroke, opacity, box-shadow, transform, filter, -webkit-backdrop-filter; - transition-property: color, background-color, border-color, text-decoration-color, fill, stroke, opacity, box-shadow, transform, filter, backdrop-filter; transition-property: color, background-color, border-color, text-decoration-color, fill, stroke, opacity, box-shadow, transform, filter, backdrop-filter, -webkit-backdrop-filter;
1790-1803: Unnecessary repeat oftransition-propertyagainThe modal box block re-introduces the same duplicate properties (Lines 1798-1803). Consider deduplicating here too or moving the rule to a shared utility class so you don’t need to maintain it in three places.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
assets/js/forms-list.min.jsis excluded by!**/*.min.jsassets/js/forms-list.min.js.mapis excluded by!**/*.map,!**/*.min.js.mapassets/js/subscriptions.min.jsis excluded by!**/*.min.jsassets/js/subscriptions.min.js.mapis excluded by!**/*.map,!**/*.min.js.map
📒 Files selected for processing (3)
assets/css/admin/form-builder.css(35 hunks)assets/js-templates/form-components.php(3 hunks)wpuf-functions.php(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- assets/js-templates/form-components.php
- wpuf-functions.php
🧰 Additional context used
🪛 Biome (2.1.2)
assets/css/admin/form-builder.css
[error] 851-851: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
transition-property is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 1799-1799: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
transition-property is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
Rubaiyat-E-Mohammad
assigned
sapayth,
arifulhoque7 and
Rubaiyat-E-Mohammad and unassigned
Rat01047
Updated the default select option text from '— Select —' to '-- Select --' for consistency. Enhanced the handling of repeatable custom field data by filtering out empty or invalid string values before rendering, ensuring only meaningful data is displayed.
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
assets/js/wpuf-form-builder-components.js (1)
1647-1651: Repeat‐field sort targets aren’t rendered in markup
I’ve confirmed that the classwpuf-repeat-fields-sortable-listis used in connectToSortable in these files:
- assets/js/wpuf-form-builder-components.js (lines 1647 & 1746)
- admin/form-builder/assets/js/components/form-fields/index.js (line 26)
- admin/form-builder/assets/js/components/form-fields-v4-1/index.js (line 44)
However, I couldn’t find any occurrence of
wpuf-repeat-fields-sortable-listin the HTML/PHP templates or JSX/Vue templates that render the repeat‐fields wrapper. Without this class on the container (e.g.<ul>or<div>for repeat fields), the new drag-and-drop target will never be matched.• Add
class="wpuf-repeat-fields-sortable-list"to the repeat‐fields container element in the component/template that renders repeat fields (e.g. in the PHP or JS template file responsible for the repeat‐field markup).
• Verify that the selector matches exactly so that jQuery UI’sconnectToSortablecan find it.
♻️ Duplicate comments (5)
wpuf-functions.php (2)
1856-1893: Back-compat conversion is valid but hard to follow; consider extracting to a helperThe in-place transformation for legacy repeat fields is correct but nested and hard to scan. Extract to a function to improve readability and testability, per earlier feedback on this PR.
1916-1918: Potential integer overflow and weak RNG in ID generator (repeat)The max value 9999000001 exceeds 32-bit PHP_INT_MAX and rand() is not ideal. Please adopt the previously suggested fix (PHP_INT_MAX-safe bounds and random_int/mt_rand fallback).
Apply this diff:
-function wpuf_form_field_id_generator( $min = 999999, $max = 9999000001 ) { - return rand( $min, $max ); +function wpuf_form_field_id_generator( $min = 999999, $max = null ) { + if ( null === $max ) { + $max = min( 2147483647, PHP_INT_MAX ); // Ensure 32-bit compatibility + } + + $min = max( 0, (int) $min ); + $max = min( (int) $max, PHP_INT_MAX ); + + if ( $min >= $max ) { + $min = 0; + } + + if ( function_exists( 'random_int' ) ) { + return random_int( $min, $max ); + } + + return mt_rand( $min, $max ); }assets/js/wpuf-form-builder.js (1)
138-146: Replace hasOwnProperty with Object.hasOwn for safer prototype-agnostic checks.Use Object.hasOwn to avoid noPrototypeBuiltins warnings and edge cases.
- for (const columnFields in innerColumnFields) { - if (innerColumnFields.hasOwnProperty(columnFields)) { + for (const columnFields in innerColumnFields) { + if (Object.hasOwn(innerColumnFields, columnFields)) { var columnFieldIndex = 0; ... - if (payload.field_name === 'name' && ! innerColumnFields[columnFields][columnFieldIndex].hasOwnProperty('is_new') ) { + if (payload.field_name === 'name' && ! Object.hasOwn(innerColumnFields[columnFields][columnFieldIndex], 'is_new') ) { columnFieldIndex++; continue; }includes/Admin/Posting.php (2)
849-856: Call static method directly; remove unused variables.get_input_fields is static; no need to instantiate. Also, $post_vars and $tax_vars are unused here.
- list( $post_vars, $tax_vars, $meta_vars ) = ( new Posting )->get_input_fields( $wpuf_cf_form_id ); + $input_fields = self::get_input_fields( $wpuf_cf_form_id ); + $meta_vars = $input_fields[2];
560-576: initRepeatField: validate data attributes to prevent runtime issues.Add guards for missing data-field-name and log a single warning per container.
- var fieldName = $container.data('field-name'); + var fieldName = $container.data('field-name') || ''; var maxRepeats = parseInt($container.data('max-repeats')) || -1; + if (!fieldName) { + console.warn('wpuf-repeat-container missing data-field-name attribute', $container.get(0)); + return; + }
🧹 Nitpick comments (14)
assets/js/wpuf-form-builder.js (3)
444-452: Guard and initialize inner_fields consistently when adding to repeat fields.Current logic is fine. Consider also validating payload.toIndex bounds (e.g., clamp to length) to avoid surprising inserts when large indices are passed.
455-461: Deletion in repeat fields: add defensive checks.Add null checks for inner_fields and index bounds before splice to avoid runtime errors when state gets out of sync.
- state.form_fields[repeatFieldIndex].inner_fields.splice(payload.index, 1); + const arr = state.form_fields[repeatFieldIndex].inner_fields; + if (Array.isArray(arr) && payload.index > -1 && payload.index < arr.length) { + arr.splice(payload.index, 1); + }
464-477: Cloning repeat inner field: OK with a minor nit.Deep-clone and new id/name semantics look good. Consider also copying/normalizing any meta flags that should not carry over (e.g., default values for options arrays) to avoid leaking prior state into the clone.
assets/css/admin/form-builder.css (4)
846-871: Duplicate transition-property declarations; remove duplicates to reduce CSS bloat.This block repeats transition-property multiple times. Keep a single declaration to avoid confusion and unnecessary overrides.
- transition-property: color, background-color, border-color, text-decoration-color, fill, stroke, opacity, box-shadow, transform, filter, -webkit-backdrop-filter; - transition-property: color, background-color, border-color, text-decoration-color, fill, stroke, opacity, box-shadow, transform, filter, backdrop-filter; transition-property: color, background-color, border-color, text-decoration-color, fill, stroke, opacity, box-shadow, transform, filter, backdrop-filter, -webkit-backdrop-filter;
1795-1804: Duplicate transition-property in .wpuf-modal-box.Same duplication here; keep only the combined variant.
- transition-property: color, background-color, border-color, text-decoration-color, fill, stroke, opacity, box-shadow, transform, filter, -webkit-backdrop-filter; - transition-property: color, background-color, border-color, text-decoration-color, fill, stroke, opacity, box-shadow, transform, filter, backdrop-filter; transition-property: color, background-color, border-color, text-decoration-color, fill, stroke, opacity, box-shadow, transform, filter, backdrop-filter, -webkit-backdrop-filter;
1219-1254: Redundant hover rules repeated across sections (primary/secondary variants).Multiple identical hover rule blocks are duplicated back-to-back. Consider de-duplicating in the source (Tailwind/DaisyUI config or preprocessor) to shrink output and ease maintenance.
If you want, I can generate a PostCSS/CSSO config to merge duplicate rules during build.
Also applies to: 1255-1290, 1291-1302
1070-1079: Disabled state rules appear twice for .wpuf-btm-nav items.Consolidate to a single definition to avoid maintenance drift.
Also applies to: 1985-2003
includes/Admin/Posting.php (7)
68-69: Scope admin_enqueue_scripts to relevant screens to avoid global admin bloat.Hooking unconditionally will enqueue assets on every admin page. Gate by screen (post.php/post-new.php or your metabox screens).
- add_action( 'admin_enqueue_scripts', [ $this, 'enqueue_script'] ); + add_action( 'admin_enqueue_scripts', function($hook) { + $screen = function_exists('get_current_screen') ? get_current_screen() : null; + if ($screen && in_array($screen->base, ['post', 'post-new'], true)) { + $this->enqueue_script(); + } + } );
451-467: Remove debug console logs from admin metabox output.These logs will clutter admin console for all users.
- // Test if field initialization script is loaded - console.log('Admin metabox script loaded'); - console.log('WPUF_Field_Initializer available:', typeof WPUF_Field_Initializer !== 'undefined');
472-495: Repeat-field render fallback when Pro class is unavailable.If Field_Repeat Pro class is missing, repeat fields won’t render at all. Consider a graceful fallback (e.g., render inner fields sequentially) or show a notice so admins know why repeats aren’t visible.
578-633: Cloning repeat instances: ensure unique IDs and plugin-safe cloning.
- IDs often don’t contain brackets; current logic may duplicate IDs. Also update non-bracketed IDs (e.g., trailing numeric suffix).
- Cloning enhanced selects (Selectize) can clone plugin wrappers. Prefer cloning an inert template or destroy plugin instances before cloning, then re-init after append.
- if (originalId && originalId.includes('[')) { - var newId = originalId.replace(/\[\d+\]/, '[' + newInstanceIndex + ']'); - $element.attr('id', newId); - } + if (originalId) { + if (originalId.includes('[')) { + $element.attr('id', originalId.replace(/\[\d+\]/, '[' + newInstanceIndex + ']')); + } else { + // Replace trailing number or append index + $element.attr('id', originalId.replace(/\d+$/, '') + newInstanceIndex); + } + }Additionally, before cloning:
- Strip any Selectize/intlTelInput wrappers or re-create a clean node from a stored template to avoid nested plugin DOM.
- When clearing selects enhanced by Selectize, use selectize.clear(true) if available.
643-669: Reindexing: apply same ID-safe logic as addRepeatInstance.Mirror the non-bracketed ID update to prevent duplicated IDs after deletions/reorders.
691-699: Avoid double initialization of WPUF_Field_Initializer.You call init() inside addRepeatInstance and again after a 500ms timeout. If the initializer isn’t idempotent, this can create duplicate listeners. Consider gating with a flag or ensuring init() is safe to re-run.
470-470: Remove unused $atts.$atts is declared but not used.
- $atts = [];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
admin/form-builder/assets/js/components/builder-stage-v4-1/template.php(2 hunks)admin/form-builder/assets/js/form-builder.js(6 hunks)assets/css/admin/form-builder.css(35 hunks)assets/js-templates/form-components.php(2 hunks)assets/js/wpuf-form-builder-components.js(8 hunks)assets/js/wpuf-form-builder.js(6 hunks)includes/Admin/Posting.php(7 hunks)includes/Traits/FieldableTrait.php(1 hunks)wpuf-functions.php(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- assets/js-templates/form-components.php
- includes/Traits/FieldableTrait.php
- admin/form-builder/assets/js/form-builder.js
🧰 Additional context used
🧬 Code Graph Analysis (3)
assets/js/wpuf-form-builder-components.js (1)
admin/form-builder/assets/js/components/builder-stage-v4-1/index.js (2)
refName(182-182)comp(183-183)
includes/Admin/Posting.php (2)
wpuf.php (1)
wpuf(418-420)admin/posting.php (1)
get_input_fields(566-633)
wpuf-functions.php (2)
admin/form-builder/assets/js/form-builder.js (1)
inner_field(375-375)assets/js/wpuf-form-builder.js (1)
inner_field(375-375)
🪛 Biome (2.1.2)
assets/css/admin/form-builder.css
[error] 851-851: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
transition-property is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 1799-1799: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
transition-property is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
🪛 PHPMD (2.15.0)
includes/Admin/Posting.php
470-470: Avoid unused local variables such as '$atts'. (Unused Code Rules)
(UnusedLocalVariable)
849-849: Avoid unused local variables such as '$post_vars'. (Unused Code Rules)
(UnusedLocalVariable)
849-849: Avoid unused local variables such as '$tax_vars'. (Unused Code Rules)
(UnusedLocalVariable)
wpuf-functions.php
1173-1173: Avoid unused local variables such as '$i'. (Unused Code Rules)
(UnusedLocalVariable)
1175-1175: Avoid unused local variables such as '$newvalue'. (Unused Code Rules)
(UnusedLocalVariable)
🔇 Additional comments (11)
wpuf-functions.php (3)
1162-1171: Good: repeat meta retrieval and safe unserialize guardFetching the repeat data with get_post_meta and guarding with is_serialized/maybe_unserialize is correct and defensive.
1856-1886: Verify inner_fields shape for repeat back-compat aligns with frontend renderingBack-compat code builds inner_fields in a column-style structure. Your wpuf_show_custom_fields('repeat') renderer expects a flat array of inner fields. Either keep the frontend tolerant (flatten as suggested) or standardize here by emitting a flat array to reduce complexity across layers.
If you prefer standardizing here, replace the column keys with a flat array:
Apply this diff:
- $field['inner_fields']['column-1'] = [ $field ]; - $field['inner_fields']['column-2'] = []; - $field['inner_fields']['column-3'] = []; + $field['inner_fields'] = [ $field ]; + unset( $field['inner_columns_size'] ); + unset( $field['columns'], $field['min_column'], $field['max_column'], $field['column_space'] );Please confirm if other parts of the builder/UI expect the column-structured format for repeat fields. If so, keep the frontend flattening change instead.
1906-1908: Hooks for child category retrieval look goodRegistering both authenticated and unauthenticated AJAX actions for wpuf_get_child_cat is appropriate.
assets/js/wpuf-form-builder-components.js (5)
698-728: Robust lookup for nested editing_form_fieldNicely done. The computed guard handles both column_field (object of columns) and repeat_field (array) inner_fields, preventing undefined access when editing nested fields.
733-741: Safe fallback when no editing_form_field is selectedReturning an empty settings array prevents downstream errors in panels. Good defensive addition.
771-774: Safe title fallback for missing editing_form_fieldReturning an empty string is appropriate here. Avoids “Cannot read property 'template'” at render time.
1309-1317: Rename alignment and constraint check for inner column fieldsRenamed method isAllowedInColumnField is consistently used and the disallow list logic is clear. The user feedback via SweetAlert is user-friendly.
Also applies to: 1327-1341
367-377: Guarded ref access for repeat field picker is fine; add the matching ref in your templateThe
openRepeatFieldPickerlogic correctly handles Vue 2’s array-refs, but I couldn’t find anyref="repeatFieldComponent_<id>"or:ref="…"bindings in your builder templates. Without that,this.$refs[refName]will always be undefined at runtime.Please verify and update the templates where you render the repeat‐field component (e.g. in
builder-stage-v4-1/index.jsor your PHP/Vue template) to include something like:<repeat-field-component v-for="field in fields" :key="field.id" :field="field" :ref="`repeatFieldComponent_${field.id}`" />– ensures
this.$refs['repeatFieldComponent_' + fieldId]actually points to each component instance.admin/form-builder/assets/js/components/builder-stage-v4-1/template.php (1)
23-26: Correctly exclude repeat wrapper; aligns with column-style renderingTreating repeat similar to column_field by excluding the default wrapper is appropriate for containers that manage their own inner rendering.
assets/js/wpuf-form-builder.js (1)
987-989: LGTM: simple assignment for settings radio item.No issues.
includes/Admin/Posting.php (1)
756-813: Admin repeat-field styles: LGTM.Clear, contained styles for repeat containers and controls.
| v-if="is_template_available(field) && (field.input_type === 'column_field' || field.input_type === 'repeat')" | ||
| :is="'form-' + field.template" | ||
| :field="field"> | ||
| </component> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Render repeat container component alongside column_field; add a ref for programmatic picker
The component gating is correct. To enable openRepeatFieldPicker(fieldId), bind a ref for repeat fields so this.$refs['repeatFieldComponent_' + fieldId] resolves.
Apply this diff:
- <component
- v-if="is_template_available(field) && (field.input_type === 'column_field' || field.input_type === 'repeat')"
- :is="'form-' + field.template"
- :field="field">
+ <component
+ v-if="is_template_available(field) && (field.input_type === 'column_field' || field.input_type === 'repeat')"
+ :is="'form-' + field.template"
+ :ref="field.input_type === 'repeat' ? ('repeatFieldComponent_' + field.id) : null"
+ :field="field">
</component>If refs must be unique and you have multiple repeat components rendered in a v-for, note that Vue 2 will expose them as an array; your JS already handles that.
🏁 Script executed:
#!/bin/bash
set -e
# Locate the v-for rendering loop in the Vue template
rg -n "v-for" -C3 admin/form-builder/assets/js/components/builder-stage-v4-1/template.php || true
# Show the first 120 lines of the file to inspect the context and confirm position of lines 58-61
sed -n '1,120p' admin/form-builder/assets/js/components/builder-stage-v4-1/template.phpLength of output: 10191
🏁 Script executed:
#!/bin/bash
set -e
# Search for openRepeatFieldPicker usages
rg -n "openRepeatFieldPicker" -C3 admin/form-builder/assets/js/components/builder-stage-v4-1/template.php || true
# Show lines 120-300 to locate the script block and method definitions
sed -n '120,300p' admin/form-builder/assets/js/components/builder-stage-v4-1/template.phpLength of output: 4168
🏁 Script executed:
#!/bin/bash
set -e
# Search for openRepeatFieldPicker usage across the repo
rg -n "openRepeatFieldPicker" -C3 .
# Search for existing refs on repeatFieldComponent_ to see if any code already references it
rg -n "repeatFieldComponent_" -C3 .Length of output: 3132
Bind ref on repeat field component to support openRepeatFieldPicker
To enable openRepeatFieldPicker(fieldId) to locate the correct component instance via this.$refs, add a dynamic ref only when field.input_type === 'repeat'. Without this binding, the picker cannot find the component.
Apply this diff in admin/form-builder/assets/js/components/builder-stage-v4-1/template.php (around lines 58–61):
- <component
- v-if="is_template_available(field) && (field.input_type === 'column_field' || field.input_type === 'repeat')"
- :is="'form-' + field.template"
- :field="field">
+ <component
+ v-if="is_template_available(field) && (field.input_type === 'column_field' || field.input_type === 'repeat')"
+ :is="'form-' + field.template"
+ :ref="field.input_type === 'repeat' ? 'repeatFieldComponent_' + field.id : null"
+ :field="field">
</component>• Vue 2 will expose refs defined inside a v-for as arrays when multiple repeats exist; the existing openRepeatFieldPicker logic already handles this case.
📝 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.
| v-if="is_template_available(field) && (field.input_type === 'column_field' || field.input_type === 'repeat')" | |
| :is="'form-' + field.template" | |
| :field="field"> | |
| </component> | |
| <component | |
| v-if="is_template_available(field) && (field.input_type === 'column_field' || field.input_type === 'repeat')" | |
| :is="'form-' + field.template" | |
| :ref="field.input_type === 'repeat' ? 'repeatFieldComponent_' + field.id : null" | |
| :field="field"> | |
| </component> |
🤖 Prompt for AI Agents
In admin/form-builder/assets/js/components/builder-stage-v4-1/template.php
around lines 58 to 61, the repeat-field component isn't given a ref so
openRepeatFieldPicker(fieldId) cannot find the instance via this.$refs; update
the component tag to add a dynamic ref only when field.input_type === 'repeat'
(e.g. :ref="field.input_type === 'repeat' ? 'repeat_' + field.id : null") so Vue
exposes the ref (note Vue 2 will return arrays for refs inside v-for and
existing picker logic already handles that).
| // check if the editing field belongs to a repeat field | ||
| if (state.form_fields[i].template === 'repeat_field') { | ||
| var innerRepeatFields = state.form_fields[i].inner_fields; | ||
|
|
||
| if (Array.isArray(innerRepeatFields)) { | ||
| for (var repeatFieldIndex = 0; repeatFieldIndex < innerRepeatFields.length; repeatFieldIndex++) { | ||
| // don't modify existing meta key | ||
| if (payload.field_name === 'name' && !innerRepeatFields[repeatFieldIndex].hasOwnProperty('is_new')) { | ||
| continue; | ||
| } | ||
|
|
||
| if (innerRepeatFields[repeatFieldIndex].id === parseInt(payload.editing_field_id)) { | ||
| if ('read_only' === payload.field_name && payload.value) { | ||
| innerRepeatFields[repeatFieldIndex]['required'] = 'no'; | ||
| } | ||
|
|
||
| if ('required' === payload.field_name && 'yes' === payload.value) { | ||
| innerRepeatFields[repeatFieldIndex]['read_only'] = false; | ||
| } | ||
|
|
||
| innerRepeatFields[repeatFieldIndex][payload.field_name] = payload.value; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Repeat inner-field updates: fix hasOwnProperty and normalize required/read_only toggling.
- Replace hasOwnProperty with Object.hasOwn.
- Type handling for required/read_only is inconsistent with top-level logic. Normalize checks to accept both boolean and "yes"/"no" values to prevent mismatches.
- if (payload.field_name === 'name' && !innerRepeatFields[repeatFieldIndex].hasOwnProperty('is_new')) {
+ if (payload.field_name === 'name' && !Object.hasOwn(innerRepeatFields[repeatFieldIndex], 'is_new')) {
continue;
}
- if ('read_only' === payload.field_name && payload.value) {
- innerRepeatFields[repeatFieldIndex]['required'] = 'no';
+ if ('read_only' === payload.field_name && (payload.value === true || payload.value === 'yes')) {
+ innerRepeatFields[repeatFieldIndex].required = 'no';
}
- if ('required' === payload.field_name && 'yes' === payload.value) {
- innerRepeatFields[repeatFieldIndex]['read_only'] = false;
+ if ('required' === payload.field_name && (payload.value === true || payload.value === 'yes')) {
+ innerRepeatFields[repeatFieldIndex].read_only = false;
}Confirm the canonical schema for required/read_only across all field types (boolean vs "yes"/"no"). If booleans are expected everywhere, we should set required = false here instead of "no". If strings are canonical for repeat inners, we should align top-level to strings for consistency.
🏁 Script executed:
#!/bin/bash
set -e
# Locate the wpuf-form-builder.js file
file=$(fd -e js wpuf-form-builder.js | head -n1)
echo "Inspecting file: $file"
echo
# Show context around 'required' handling for top-level fields
echo "=== Context for 'required' ==="
rg -nP "'required' === payload.field_name" -C3 "$file" || true
echo
# Show context around 'read_only' handling for top-level fields
echo "=== Context for 'read_only' ==="
rg -nP "'read_only' === payload.field_name" -C3 "$file" || true
echo
# Show any default schema definitions for required/read_only in this file
echo "=== Default schema in this file ==="
rg -nP "(required|read_only)\s*:\s*" -C2 "$file" || trueLength of output: 2343
Repeat inner-field updates: use Object.hasOwn and normalize to boolean flags
To align inner-field logic with top-level behavior (which uses booleans for required/read_only), update the checks and assignments as follows:
• Replace hasOwnProperty with Object.hasOwn
• Treat payload.value as “enabled” only when true or "yes"
• Use boolean false for disabling required instead of "no"
Diff:
- if (payload.field_name === 'name' && !innerRepeatFields[repeatFieldIndex].hasOwnProperty('is_new')) {
+ if (payload.field_name === 'name' && !Object.hasOwn(innerRepeatFields[repeatFieldIndex], 'is_new')) {
continue;
}
- if ('read_only' === payload.field_name && payload.value) {
- innerRepeatFields[repeatFieldIndex]['required'] = 'no';
+ if ('read_only' === payload.field_name && (payload.value === true || payload.value === 'yes')) {
+ innerRepeatFields[repeatFieldIndex].required = false;
}
- if ('required' === payload.field_name && 'yes' === payload.value) {
- innerRepeatFields[repeatFieldIndex]['read_only'] = false;
+ if ('required' === payload.field_name && (payload.value === true || payload.value === 'yes')) {
+ innerRepeatFields[repeatFieldIndex].read_only = false;
}These changes ensure inner repeats mirror the boolean schema used by top-level fields.
📝 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.
| // check if the editing field belongs to a repeat field | |
| if (state.form_fields[i].template === 'repeat_field') { | |
| var innerRepeatFields = state.form_fields[i].inner_fields; | |
| if (Array.isArray(innerRepeatFields)) { | |
| for (var repeatFieldIndex = 0; repeatFieldIndex < innerRepeatFields.length; repeatFieldIndex++) { | |
| // don't modify existing meta key | |
| if (payload.field_name === 'name' && !innerRepeatFields[repeatFieldIndex].hasOwnProperty('is_new')) { | |
| continue; | |
| } | |
| if (innerRepeatFields[repeatFieldIndex].id === parseInt(payload.editing_field_id)) { | |
| if ('read_only' === payload.field_name && payload.value) { | |
| innerRepeatFields[repeatFieldIndex]['required'] = 'no'; | |
| } | |
| if ('required' === payload.field_name && 'yes' === payload.value) { | |
| innerRepeatFields[repeatFieldIndex]['read_only'] = false; | |
| } | |
| innerRepeatFields[repeatFieldIndex][payload.field_name] = payload.value; | |
| } | |
| } | |
| } | |
| // check if the editing field belongs to a repeat field | |
| if (state.form_fields[i].template === 'repeat_field') { | |
| var innerRepeatFields = state.form_fields[i].inner_fields; | |
| if (Array.isArray(innerRepeatFields)) { | |
| for (var repeatFieldIndex = 0; repeatFieldIndex < innerRepeatFields.length; repeatFieldIndex++) { | |
| // don't modify existing meta key | |
| if (payload.field_name === 'name' && !Object.hasOwn(innerRepeatFields[repeatFieldIndex], 'is_new')) { | |
| continue; | |
| } | |
| if (innerRepeatFields[repeatFieldIndex].id === parseInt(payload.editing_field_id)) { | |
| - if ('read_only' === payload.field_name && payload.value) { | |
| if ('read_only' === payload.field_name && (payload.value === true || payload.value === 'yes')) { | |
| innerRepeatFields[repeatFieldIndex].required = false; | |
| } | |
| - if ('required' === payload.field_name && 'yes' === payload.value) { | |
| if ('required' === payload.field_name && (payload.value === true || payload.value === 'yes')) { | |
| innerRepeatFields[repeatFieldIndex].read_only = false; | |
| } | |
| innerRepeatFields[repeatFieldIndex][payload.field_name] = payload.value; | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In assets/js/wpuf-form-builder.js around lines 158 to 181, update inner
repeat-field handling to mirror top-level boolean semantics: replace
innerRepeatFields[repeatFieldIndex].hasOwnProperty('is_new') with
Object.hasOwn(innerRepeatFields[repeatFieldIndex], 'is_new'); treat
payload.value as "enabled" only when payload.value === true || payload.value ===
'yes'; when handling 'read_only' set
innerRepeatFields[repeatFieldIndex].read_only = (payload.value === true ||
payload.value === 'yes') and if enabling read_only set required = false
(boolean); when handling 'required' set
innerRepeatFields[repeatFieldIndex].required = (payload.value === true ||
payload.value === 'yes') and if enabling required set read_only = false
(boolean); for other fields assign payload.value as-is.
| // open repeat field settings panel | ||
| open_repeat_field_settings: function (state, payload) { | ||
| var field = payload.repeat_field; | ||
|
|
||
| if ('field-options' === state.current_panel && field.id === state.editing_field_id) { | ||
| return; | ||
| } | ||
|
|
||
| if (field) { | ||
| state.editing_field_id = field.id; | ||
| state.current_panel = 'field-options'; | ||
| state.editing_field_type = 'repeat_field'; | ||
| state.editing_repeat_field_id = payload.field_id; | ||
| state.editing_inner_field_index = payload.index; | ||
|
|
||
| setTimeout(function () { | ||
| state.editing_field_id = field.id; | ||
| }, 400); | ||
| } | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Align open_repeat_field_settings with existing pattern to force re-render.
open_column_field_settings resets editing_field_id to 0 before setting it after a timeout. Do the same here to ensure watchers trigger reliably.
- if (field) {
- state.editing_field_id = field.id;
- state.current_panel = 'field-options';
+ if (field) {
+ state.editing_field_id = 0;
+ state.current_panel = 'field-options';
state.editing_field_type = 'repeat_field';
state.editing_repeat_field_id = payload.field_id;
state.editing_inner_field_index = payload.index;
setTimeout(function () {
state.editing_field_id = field.id;
}, 400);📝 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.
| // open repeat field settings panel | |
| open_repeat_field_settings: function (state, payload) { | |
| var field = payload.repeat_field; | |
| if ('field-options' === state.current_panel && field.id === state.editing_field_id) { | |
| return; | |
| } | |
| if (field) { | |
| state.editing_field_id = field.id; | |
| state.current_panel = 'field-options'; | |
| state.editing_field_type = 'repeat_field'; | |
| state.editing_repeat_field_id = payload.field_id; | |
| state.editing_inner_field_index = payload.index; | |
| setTimeout(function () { | |
| state.editing_field_id = field.id; | |
| }, 400); | |
| } | |
| }, | |
| // open repeat field settings panel | |
| open_repeat_field_settings: function (state, payload) { | |
| var field = payload.repeat_field; | |
| if ('field-options' === state.current_panel && field.id === state.editing_field_id) { | |
| return; | |
| } | |
| if (field) { | |
| state.editing_field_id = 0; | |
| state.current_panel = 'field-options'; | |
| state.editing_field_type = 'repeat_field'; | |
| state.editing_repeat_field_id = payload.field_id; | |
| state.editing_inner_field_index = payload.index; | |
| setTimeout(function () { | |
| state.editing_field_id = field.id; | |
| }, 400); | |
| } | |
| }, |
🤖 Prompt for AI Agents
In assets/js/wpuf-form-builder.js around lines 556 to 575,
open_repeat_field_settings currently sets editing_field_id directly which can
fail to trigger watchers; update it to first set state.editing_field_id = 0,
then set the other fields, and inside the setTimeout set state.editing_field_id
= field.id (same pattern as open_column_field_settings) so the change forces a
re-render and reliably triggers watchers.
| // Enqueue field initialization script for admin metabox | ||
|
|
||
| // Enqueue Selectize for country fields | ||
| wp_enqueue_style( 'wpuf-selectize' ); | ||
| wp_enqueue_script( 'wpuf-selectize' ); | ||
|
|
||
| // Enqueue international telephone input for phone fields | ||
| wp_enqueue_style( 'wpuf-intlTelInput' ); | ||
| wp_enqueue_script( 'wpuf-intlTelInput' ); | ||
|
|
||
| // Try to load the field initialization script using the registered handle | ||
| wp_enqueue_script( 'wpuf-field-initialization' ); | ||
|
|
||
| // Localize script with asset URI | ||
| wp_localize_script( 'wpuf-field-initialization', 'wpuf_field_initializer', [ | ||
| 'asset_uri' => defined( 'WPUF_PRO_ASSET_URI' ) ? WPUF_PRO_ASSET_URI : '', | ||
| ] ); | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify script/style handles are registered; localize after enqueue.
Ensure 'wpuf-selectize', 'wpuf-intlTelInput', and 'wpuf-field-initialization' are registered before enqueuing. Also, call wp_localize_script after or near the enqueue to guarantee the handle exists.
🏁 Script executed:
#!/bin/bash
# Verify handles are registered in the codebase
rg -n -C2 -iP "wp_register_script\(['\"]wpuf-(field-initialization|selectize|intlTelInput)['\"]" | sed -n '1,200p'
rg -n -C2 -iP "wp_enqueue_script\(['\"]wpuf-(field-initialization|selectize|intlTelInput)['\"]" | sed -n '1,200p'Length of output: 236
To cover both scripts and styles, let’s search for all registrations and enqueues of these handles:
🏁 Script executed:
#!/bin/bash
# Look for registrations of scripts/styles
rg -n -C2 -iP "wp_register_(script|style)\(['\"]wpuf-(selectize|intlTelInput|field-initialization)['\"]"
# Look for enqueues of scripts/styles
rg -n -C2 -iP "wp_enqueue_(script|style)\(['\"]wpuf-(selectize|intlTelInput|field-initialization)['\"]"Length of output: 220
Let’s locate where scripts/styles (and the WPUF_PRO_ASSET_URI constant) are registered and hooked:
🏁 Script executed:
#!/bin/bash
# Find all script registrations
rg -n -C3 "wp_register_script"
# Find all style registrations
rg -n -C3 "wp_register_style"
# Locate the definition of the assets URI constant
rg -n -C3 "define.*WPUF_PRO_ASSET_URI"
# Check enqueue hooks in admin/frontend
rg -n -C3 "add_action\(['\"](admin_enqueue_scripts|wp_enqueue_scripts)['\"],"Length of output: 5676
Here’s a deeper dive into includes/Assets.php to locate where scripts and styles (including our handles) are defined:
🏁 Script executed:
#!/bin/bash
# Locate the static arrays defining assets
rg -n "private static.*\$scripts" -C5 includes/Assets.php
rg -n "private static.*\$styles" -C5 includes/Assets.php
# Print the surrounding lines to see the array entries
rg -n "^\s*'selectize'" -C3 includes/Assets.php || true
rg -n "^\s*'intlTelInput'" -C3 includes/Assets.php || true
rg -n "^\s*'field-initialization'" -C3 includes/Assets.php || trueLength of output: 1050
Let’s inspect the asset definitions to confirm whether intlTelInput and field-initialization are registered:
🏁 Script executed:
#!/bin/bash
# Print the first 200 lines of includes/Assets.php to view the static $scripts and $styles arrays
sed -n '1,200p' includes/Assets.phpLength of output: 6823
To confirm whether intlTelInput and field‐initialization are defined in the scripts array, let’s dump the JS definitions from includes/Assets.php:
🏁 Script executed:
#!/bin/bash
# Print JS definitions in get_scripts()
sed -n '200,350p' includes/Assets.php
# Look specifically for our handles
rg -n -C3 "'intlTelInput'" includes/Assets.php || true
rg -n -C3 "'field-initialization'" includes/Assets.php || trueLength of output: 6836
Register missing handles and move localization before enqueue
It looks like only selectize is defined in includes/Assets.php; intlTelInput and field-initialization aren’t registered, so enqueuing them will fail and the localization won’t attach. Please:
• In your assets registration (e.g. includes/Assets.php or your PRO Assets class):
– Add entries for
• intlTelInput (style & script) pointing at WPUF_PRO_ASSET_URI . '/vendor/intl-tel-input/…'
• field-initialization (script) pointing at your field init JS, with appropriate deps/in_footer/version.
• In includes/Admin/Posting.php:
– Call wp_localize_script( 'wpuf-field-initialization', … ) immediately after registering (or before enqueue), then enqueue with
php wp_localize_script( 'wpuf-field-initialization', 'wpuf_field_initializer', […]); wp_enqueue_script( 'wpuf-field-initialization' );
• Verify the WPUF_PRO_ASSET_URI constant is defined at plugin bootstrap so asset_uri isn’t empty.
🤖 Prompt for AI Agents
In includes/Admin/Posting.php around lines 146 to 163, the code enqueues handles
wpuf-intlTelInput and wpuf-field-initialization that are not registered and
localizes the field initializer after enqueueing, so localization may not
attach; register the missing handles in your assets class (e.g.,
includes/Assets.php or PRO Assets) adding intlTelInput style & script entries
pointing to WPUF_PRO_ASSET_URI . '/vendor/intl-tel-input/…' and a
wpuf-field-initialization script entry with correct src, deps, in_footer and
version, ensure WPUF_PRO_ASSET_URI is defined at bootstrap, then move
wp_localize_script('wpuf-field-initialization', ...) to run immediately after
that script is registered (or before enqueue) and only then call
wp_enqueue_script('wpuf-field-initialization').
| foreach ( $value as $i => $str ) { | ||
| if ( preg_match( '/[^\|\s]/', $str ) ) { | ||
| if ( is_string( $str ) && preg_match( '/[^\|\s]/', $str ) ) { | ||
| $newvalue[] = $str; | ||
| } | ||
| } |
There was a problem hiding this comment.
Bug: undefined variable and unused accumulator in repeat rendering
This loop references $value, which is not defined in this scope, and accumulates into $newvalue that is never used. This will emit notices (Undefined variable: value) and is dead code.
Remove the loop.
Apply this diff:
- foreach ( $value as $i => $str ) {
- if ( is_string( $str ) && preg_match( '/[^\|\s]/', $str ) ) {
- $newvalue[] = $str;
- }
- }📝 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.
| foreach ( $value as $i => $str ) { | |
| if ( preg_match( '/[^\|\s]/', $str ) ) { | |
| if ( is_string( $str ) && preg_match( '/[^\|\s]/', $str ) ) { | |
| $newvalue[] = $str; | |
| } | |
| } |
🧰 Tools
🪛 PHPMD (2.15.0)
1173-1173: Avoid unused local variables such as '$i'. (Unused Code Rules)
(UnusedLocalVariable)
1175-1175: Avoid unused local variables such as '$newvalue'. (Unused Code Rules)
(UnusedLocalVariable)
🤖 Prompt for AI Agents
In wpuf-functions.php around lines 1173 to 1177 there is a stray foreach loop
that references an undefined $value and populates an unused $newvalue; remove
the entire foreach block (the four lines) so no undefined variable access or
dead accumulator remains, and then search for any remaining references to
$newvalue in the surrounding function to remove or adjust them accordingly; run
PHP lint/tests to confirm no notices remain.
| foreach ( $repeat_data as $repeat_entry ) { | ||
| $repeat_html .= '<li class="wpuf-repeat-entry">'; | ||
| $repeat_html .= '<ul class="wpuf-repeat-entry-fields">'; | ||
|
|
||
| foreach ( $attr['inner_fields'] as $inner_field ) { | ||
| $inner_field_name = $inner_field['name']; | ||
|
|
||
| if ( isset( $repeat_entry[ $inner_field_name ] ) ) { | ||
| $inner_field_value = $repeat_entry[ $inner_field_name ]; | ||
|
|
||
| if ( ! empty( $inner_field_value ) ) { | ||
| $repeat_html .= '<li>'; | ||
|
|
||
| if ( 'no' === $inner_field['hide_field_label'] ) { | ||
| $repeat_html .= '<label>' . $inner_field['label'] . ':</label> '; | ||
| } | ||
|
|
||
| // Handle different field types | ||
| if ( 'checkbox' === $inner_field['input_type'] && is_array( $inner_field_value ) ) { | ||
| // For checkbox fields, join multiple values | ||
| $repeat_html .= '<span>' . make_clickable( implode( ', ', $inner_field_value ) ) . '</span>'; | ||
| } elseif ( 'radio' === $inner_field['input_type'] || 'select' === $inner_field['input_type'] ) { | ||
| // For radio and select fields, display single value | ||
| $repeat_html .= '<span>' . make_clickable( $inner_field_value ) . '</span>'; | ||
| } else { | ||
| // For text and other fields | ||
| $repeat_html .= '<span>' . make_clickable( $inner_field_value ) . '</span>'; | ||
| } | ||
|
|
||
| $repeat_html .= '</li>'; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| $html .= make_clickable( $new ) . '</li>'; | ||
| $repeat_html .= '</ul>'; | ||
| $repeat_html .= '</li>'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle both legacy (column-structured) and new (flat array) inner_fields for repeat fields
The current code assumes $attr['inner_fields'] is a flat array of inner fields. However, wpuf_get_form_fields() can populate inner_fields in a column-style structure for backward compatibility (column-1, column-2, column-3). Iterating directly over $attr['inner_fields'] will break in that case.
Flatten inner_fields when column keys are present, and use a safe default for hide_field_label to avoid undefined index notices.
Apply this diff:
- foreach ( $attr['inner_fields'] as $inner_field ) {
+ // Support both flat array and legacy column-structured inner fields
+ $inner_fields = $attr['inner_fields'];
+ if ( is_array( $inner_fields ) && ( isset( $inner_fields['column-1'] ) || isset( $inner_fields['column-2'] ) || isset( $inner_fields['column-3'] ) ) ) {
+ $inner_fields = array_merge(
+ isset( $inner_fields['column-1'] ) ? (array) $inner_fields['column-1'] : [],
+ isset( $inner_fields['column-2'] ) ? (array) $inner_fields['column-2'] : [],
+ isset( $inner_fields['column-3'] ) ? (array) $inner_fields['column-3'] : []
+ );
+ }
+
+ if ( ! is_array( $inner_fields ) ) {
+ break;
+ }
+
+ foreach ( $inner_fields as $inner_field ) {
$inner_field_name = $inner_field['name'];
if ( isset( $repeat_entry[ $inner_field_name ] ) ) {
$inner_field_value = $repeat_entry[ $inner_field_name ];
if ( ! empty( $inner_field_value ) ) {
$repeat_html .= '<li>';
- if ( 'no' === $inner_field['hide_field_label'] ) {
+ $inner_hide_label = isset( $inner_field['hide_field_label'] ) ? $inner_field['hide_field_label'] : 'no';
+ if ( 'no' === $inner_hide_label ) {
$repeat_html .= '<label>' . $inner_field['label'] . ':</label> ';
}
// Handle different field types
if ( 'checkbox' === $inner_field['input_type'] && is_array( $inner_field_value ) ) {
// For checkbox fields, join multiple values
$repeat_html .= '<span>' . make_clickable( implode( ', ', $inner_field_value ) ) . '</span>';
} elseif ( 'radio' === $inner_field['input_type'] || 'select' === $inner_field['input_type'] ) {
// For radio and select fields, display single value
$repeat_html .= '<span>' . make_clickable( $inner_field_value ) . '</span>';
} else {
// For text and other fields
$repeat_html .= '<span>' . make_clickable( $inner_field_value ) . '</span>';
}
$repeat_html .= '</li>';
}
}
}
Rubaiyat-E-Mohammad
added
QA Approved
Rubaiyat-E-Mohammad
assigned
sapayth and unassigned
sapayth,
arifulhoque7 and
Rubaiyat-E-Mohammad
Currently, the repeating field is hardcoded as a text field. With this enhancement, we could make different type of fields into a repeating field. supported fields:
'text_field',
'textarea_field',
'dropdown_field',
'multiple_select',
'radio_field',
'checkbox_field',
'website_url',
'date_field',
'phone_field',
'numeric_text_field',
'email_address',
'country_list_field',
Related PR #531
resolves #93, resolves #1246, resolves #1240, resolves #1307
Summary by CodeRabbit
New Features
Bug Fixes
Style
Chores