October: Add "no events" flag to Form widget for Repeater implementation

[enniel]

Repeater displayed fields recursively

...
        Users::extendFormFields(function($form, $model, $context){
            if($this->isUsers($form)) {
                $form->addTabFields([
                    'phones' => [
                        'label'      => 'phones',
                        'type'       => 'repeater',
                        'defaultTab' => 'backend::lang.user.account',
                        'form'       => [
                            'fields' => [
                                'phone' => [
                                    'label' => 'phone',
                                    'type' => 'text',
                                ]
                            ],
                        ],
                    ],
                ]);
            }
        });
...

[daftspunk]

This is happening because the repeater field itself is a form widget. So you are extending the form widget that renders the user, then you are extending the repeater to add another repeater. In short, the extendFormFields affects all forms rendered using that controller.

A suggestion perhaps would be to allow $this->isUsers to execute and return true once and once only.

[tschallacka]

Using a function static might help with this to make sure it only runs once per php execution call.

Users::extendFormFields(function($form, $model, $context){ static $hasRun; if(is_null($hasRun) && $this->isUsers($form)) { $hasRun = true; $form->addTabFields([ 'phones' => [ 'label' => 'phones', 'type' => 'repeater', 'defaultTab' => 'backend::lang.user.account', 'form' => [ 'fields' => [ 'phone' => [ 'label' => 'phone', 'type' => 'text', ] ], ], ], ]); } });```

[daftspunk]

I've been thinking about this. It seems to be a common issue. We should add a special flag to the Form widget called 'noEvents' or something like this, that disables the Form events when implemented by a repeater. It will prevent this issue from occuring.

[LukeTowers]

@daftspunk, any progress on this? I'm running into this issue myself just now. As a side note, are there Form events that would be useful for the repeater to continue to implement?

Is there a way of passing the FormWidget object being extended to that event so that we could just check to make sure we're not extending repeater form widgets instead? Something like:

Users::extendFormFields(function($form, $model, $context, $formWidget){
    if ($formWidget instanceof Backend\FormWidgets\Repeater) { return; }

    $form->addTabFields([
        'phones' => [
            'label'      => 'phones',
            'type'       => 'repeater',
            'defaultTab' => 'backend::lang.user.account',
            'form'       => [
                'fields' => [
                    'phone' => [
                        'label' => 'phone',
                        'type' => 'text',
                    ]
                ],
            ],
        ],
    ]);
});

I feel like that could potentially be a better solution than just flat out disabling events for specific form widgets; what are your thoughts?

[daftspunk]

Bumped priority to get it looked at sooner. @LukeTowers the solution proposed will not work since the widget is an instance of a Backend\Widgets\Form in both cases. A known solution is to store a flag that tells the logic to only execute once, however it is a bit dirty. Possibly the best approach is the noEvents flag, since it is very rare to hook events for a repeater.

[mbaxter91288]

Just come across this issue too. Spoken to @LukeTowers and provided me with a workaround.

[LukeTowers]

Just came across a situation where @tschallacka's solution of static variables doesn't help.

If you have a relation controller listing records, and you wish to extend the fields presented in the popup from clicking on a record within the relation controller, then the solution of static variables will prevent the field extension code from running on any record other than the first. The relation controller appears to be running the extension code over every single record visible even when just clicking one record.

To summarize, if you are using the static $hasRun; workaround, and you have a relation controller that you wish to extend with repeater fields, you're screwed.

@daftspunk Would my solution work if get_class($formWidget) === 'Backend\FormWidgets\Repeater' was substituted for $formWidget instanceof Backend\FormWidgets\Repeater?

[LukeTowers]

This even dirtier workaround can be used in the above case:

// Run only once per field model
static $processedIds = [];
if (!array_key_exists($model->id, $processedIds)) {
    $processedIds[$model->id] = true;
    $form->addTabFields($model->type->fields);
}

although personally I'd much rather be able to check the class name of the FormWidget being extended if possible instead of use these workarounds or have to put in a flag for no events.

[daftspunk]

Need to consider if filterFields is considered an event and should also be blocked. It would make sense based on https://github.com/rainlab/blog-plugin/pull/247

[daftspunk]

Went down a simpler path than the original proposal. The isNested property of the form widget will be set to true when a repeater is using it. Use this value to halt any form field extensions.

/**
 * @var bool Used to flag that this form is being rendered as part of another form,
 * a good indicator to expect that the form model and dataset values will differ.
 */
public $isNested = false;

This will be available in Build 379+