#80(wpum custom fields) adding repeaters as subfields of a repeater should be allowed#410
Conversation
…as-subfields-of-a-Repeater-should-be-allowed
There was a problem hiding this comment.
Pull request overview
This pull request enables nested repeater fields functionality in the WPUM Custom Fields addon by allowing repeater fields to contain other repeater fields as subfields. The changes refactor both the frontend JavaScript handling and the backend Vue.js field editor to support this nested structure.
Changes:
- Removed Vue.js code that prevented adding repeaters as subfields of repeaters in the field editor
- Refactored JavaScript repeater handling to support recursive nested repeaters with proper field name generation
- Updated PHP template to delegate rendering to parent field types for proper nested structure
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/fields-editor/settings/field-type-repeater.vue | Removed code blocking repeater fields from being added as subfields |
| assets/js/src/wp-user-manager.js | Refactored repeater initialization and field management to support nested repeaters with recursive handling |
| templates/forms/form-registration-fields.php | Added early return for parent field types to delegate rendering |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Prepend parentBase if available and does not already have a parentBase | ||
| if (parentBase && !fieldName.includes(parentBase)) { |
There was a problem hiding this comment.
The check !fieldName.includes(parentBase) on line 157 could produce incorrect results if the field name naturally contains the parentBase string as a substring in an unrelated context. For example, if parentBase is "user" and fieldName is "username[0][field]", this would incorrectly match. Consider using a more precise check, such as verifying if fieldName starts with parentBase followed by a bracket.
| // Prepend parentBase if available and does not already have a parentBase | |
| if (parentBase && !fieldName.includes(parentBase)) { | |
| // Prepend parentBase if available and the fieldName is not already prefixed with it | |
| if (parentBase && !fieldName.startsWith(parentBase + '[')) { |
| var fieldSet = $(this).closest('fieldset'); | ||
| var currentParentBase = repeaterKey + '[' + i + ']'; | ||
|
|
||
| self.setupInstances(fieldSet, currentParentBase); |
There was a problem hiding this comment.
The recursive call to setupInstances on line 185 doesn't have any depth limit or circular reference check. While unlikely in normal usage, if there's a bug in the field configuration that creates circular parent-child relationships, this could cause infinite recursion and stack overflow. Consider adding a depth parameter or tracking visited fieldsets to prevent potential infinite loops.
| .find('> fieldset > .field :input') | ||
| // Exclude sub repeater fields | ||
| .not($(this).find('> fieldset > .fieldset-wpum_field_group :input')) |
There was a problem hiding this comment.
The selector $(this).find('> fieldset > .field :input') excludes inputs from nested repeaters using .not($(this).find('> fieldset > .fieldset-wpum_field_group :input')). However, this creates a new jQuery object inside .not() which could have performance implications if there are many nested repeaters. Consider caching the nested inputs selector or using a more efficient approach like filtering within a single find operation.
Resolves #https://github.com/WPUserManager/wpum-custom-fields/issues/80
Description
Testing Instructions
Pre-review Checklist