search

terminal42 / contao-conditionalformfields

Display form fields conditionally in Contao Open Source CMS
MIT License
18 stars 12 forks source link

disable Widget if field is disabled #18

Closed fritzmg closed 7 years ago

fritzmg commented 7 years ago

See #17 and https://github.com/contao/core/pull/8707#issuecomment-297922914

If a form field has been disabled in the front end by conditionalformfields, the actual Widget also needs to be set to disabled - otherwise old session data might be processed in processFormData.

aschempp commented 7 years ago

I actually think this should be done in the prepareFormField hook? That way we don't even need to unset any errors etc.

fritzmg commented 7 years ago

I thought so too, but that is not the case. Contao still validates the field, regardless of whether it is set to disabled or not.

Setting it to disabled only has an effect on the front end attributes and on the return value of the \Widget::submitInput function, nothing else.

aschempp commented 7 years ago

What about the submitInput property?

fritzmg commented 7 years ago

\Widget::submitInput will return false, if the attribute disabled or readonly is set to true: Widget.php#L303-L305 (note: all form fields whose input should be submitted set self::$blnSubmitInput to true by default, e.g. FormTextField.php#L37).

Only when \Widget::submitInput returns true, the value of the Widget will be added to the $arrSubmitted array: Form.php#L218 - and only the data from $arrSubmitted will be processed in processFormData.

All this however has no effect on validation.

fritzmg commented 7 years ago

Could this be merged please or do you have another suggestion? We would like to use this extension in a few other projects as well.

aschempp commented 7 years ago

The problem I see is that the field will be disabled on the subsequent run (e.g. if another field was not successfully submitted), therefore the output in the frontend will be different.

How about removing the data in the prepareFormData hook instead?

fritzmg commented 7 years ago

The problem I see is that the field will be disabled on the subsequent run (e.g. if another field was not successfully submitted), therefore the output in the frontend will be different.

Only the Widget object is set to disabled. So on the subsequent run, the Widget would be enabled again - and only set to disabled again, if the conditional form field disabled it.

How about removing the data in the prepareFormData hook instead?

I am not sure this will work properly for all cases (including multipage forms). The logic of Contao's form class is, that the data is only saved in the session and processed, if

$objWidget->submitInput()

returns true (Form.php#L216).

aschempp commented 7 years ago

Only the Widget object is set to disabled. So on the subsequent run, the Widget would be enabled again - and only set to disabled again, if the conditional form field disabled it.

If another form field has errors, there is no subsequent run. The fields are rendered while they are set to disabled…

fritzmg commented 7 years ago

Yes, but the field is disabled in the front end by conditionalformfields anyway. And if you change the form so that the field is enabled by conditionalformfields again, the field is simply not disabled anymore. And if you then submit the form, the field will processed by Contao regularly.

I have tested this particular case many times, I never encountered any weird side effect.

aschempp commented 7 years ago

You're right. Merged in 24d768156c76b91efb602c901d5b419d3892107b