[4.1] Fix repeatable fields initialization #3143
Conversation
|
The inspection completed: No new issues |
|
|
||
| // If it's not a repeatable container we exclude all the repeatable fields from the initialization | ||
| if (!selector.hasClass('repeatable-element')) { | ||
| $elements = $elements.not(".repeatable-field-input"); |
There was a problem hiding this comment.
@pxpm this would set a bad precedent - I'd rather avoid having hardcoded stuff about one field inside form_content.blade.php. This file is general, for all fields, so it should not do things differently depending on field type. If we hardcode stuff about repeatable today, we'll hardcode stuff about another field tomorrow, and soon enough it's all a jumbled mess and the form depends a bunch of fields.
Plus, what would have happened if repeatable was an addon, and it didn't have the possibility to modify form_content at all?
What other solution can we find that keeps the changes in repeatable.blade.php alone?
There was a problem hiding this comment.
Hello @tabacitu atm I could not think of any other solution, but maybe this is a wording problem that we can fix.
First of all, why the change in form_content: there are three places in the current code where we call initializeFieldsWithJavascript.
The ones we are looking for are the self form_content that initializes the whole form, and the repeatable call that would initialize the repeatable elements.
What is needed here is a way to NOT initialize some fields per developer instruction. Atm there is no functionality in backpack that allows this. It's here that I say this might be a wording problem.
If we changed the classe names from: repeatable-element to self-initialize-container and repeatable-field-input to self-initialize-field we are creating a feature that other developers could use to tell backpack that the fields inside that container should not be initialized by main form initialization script and that would make repeatable use that functionality instead of being "a repeatable functionality". That's what I would call the wording problem.
Let me know what you think.
Pedro
There was a problem hiding this comment.
That. Is fucking. AWESOME! Excellent solution @pxpm , and excellent wording for it.
Do we need to rename repeatable-element and repeatable-field-input or can we just add this feature on top? Aren't repeatable-element and repeatable-field-input used for other purposes, other than initialising the JS?
There was a problem hiding this comment.
repeatable-element is already used by repeatable functionality, we can add the self-initialize-container class along with. As for repeatable-field-input that's a class I created for this functionality, so it could just be renamed to self-initialize-field.
There was a problem hiding this comment.
@pxpm we decided in our conversation that we should try to move ALL subfields outside the main form, using @push('before_scripts') or something like that. If that happens we'll no longer have that problem.
Waiting for your alternative PR with that solution - I have high hopes for it 😄
|
closed in favor of: #3148 |
refs: #3141 , #3137
Adds a new class to identify the repeatable fields.
Using the
data-repeatable-input-namedoes not work because repeatable fields initialization is after we need it.Excludes the repeatable fields from initialization if the selector is not a
repeatable-elementcontainer.more info: #3141 (comment)