? Success

User tests: Successful: Unsuccessful:

avatar laoneo
laoneo
14 Nov 2016

Pull Request for Issue #12810.

Summary of Changes

Adds a callback functionality for form fields to prepare the value of a custom field before the value is passed to the form.

Testing Instructions

  • Create a custom field of the type checkboxes and assign two options, one of them with a 0 value. Eg:
    image.
  • Select only the 0 value ("No" in my example) and save.
  • Reopen the form and check the value.
  • Check the item in frontend.

Expected result

When editing the article the 0 checkbox should be selected. On the frontend "No" should be displayed.

Actual result

No checkbox is selected in the form and nothing is displayed when viewing an article.

avatar laoneo laoneo - open - 14 Nov 2016
avatar laoneo laoneo - change - 14 Nov 2016
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 14 Nov 2016
Category Administration com_fields Front End Libraries
avatar Bakual
Bakual - comment - 14 Nov 2016

So we need a prepareValue method to for this FormField?

avatar laoneo
laoneo - comment - 14 Nov 2016

??

avatar Bakual
Bakual - comment - 14 Nov 2016

This code basically is only needed for the checkboxes field, right? And only to be used by com_fields?
Looks a bit strange to me then.
Since the checkboxes field works fine with JForm, I'd expect a fix in com_fields, not in the JFormField.

avatar laoneo
laoneo - comment - 14 Nov 2016

It allows a field to influence the value. If some more advanced fields will be created in the future they can then influence how a value is set on the form.
But right now it is only used in custom fields. What I want to prevent is to use field specific code in com_fields itself.

avatar Bakual
Bakual - comment - 14 Nov 2016

they can then influence how a value is set on the form

Isn't that already done with getInput? ? The appearance in a form is in full control of the formfield already.
The issue is only in com_fields in how it tries to guess if it should show the field value or not (I think). But as written in the issue I haven't found where the difference comes from. My guess was it's related to "forceMultiple" being true in the checkboxes field.

What I want to prevent is to use field specific code in com_fields itself.

Good idea ?

avatar laoneo
laoneo - comment - 14 Nov 2016

The problem lies here https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_fields/models/field.php#L581 where no array is set when it is a single row value.

avatar Bakual
Bakual - comment - 14 Nov 2016

And what does JForm differently that we get an array even with only one value? That's what I haven't found out yet. Something must be different and I think it's the cause for this issue.

avatar laoneo
laoneo - comment - 14 Nov 2016

I'v tested it adding a checkboxs field to the attribs fieldset. There it is stored as array when it get's json encoded.

avatar Bakual
Bakual - comment - 14 Nov 2016

I see, if a field has the attribute "multiple" set (also list fields), then the value is stored as array in the JSON string.
Most fields however can process strings as well since the attribute is optional. In the case of the checkboxes it (rightfully) always expects an array since it has forceMultiple property set to true.

I don't think we can check the multiple property correct since it's set during runtime of the field based on the forceMultiple property. Or do you see a way for that? Then it would be simple.

If we have to adjust the checkboxes field anyway I'd rather just cast the value to array and be done.

avatar laoneo
laoneo - comment - 14 Nov 2016

I don't really understand what this forceMultiple property does. Just had a quick look on it.

avatar Bakual
Bakual - comment - 14 Nov 2016

It ensures $this->multiple is always true and thus the value is always saved as an array ?

avatar laoneo
laoneo - comment - 15 Nov 2016

forceMultiple did the trick

avatar Bakual
Bakual - comment - 15 Nov 2016

Looks much better this way. Thanks!

avatar laoneo laoneo - change - 15 Nov 2016
Title
[com_fields] Callback to prepare the value before being passed to form
[com_fields] Handling single 0 selections properly in checkboxes
avatar laoneo laoneo - change - 15 Nov 2016
Title
[com_fields] Callback to prepare the value before being passed to form
[com_fields] Handling single 0 selections properly in checkboxes
avatar laoneo laoneo - edited - 15 Nov 2016
avatar laoneo
laoneo - comment - 16 Nov 2016

@Bakual could you test it?

avatar Bakual Bakual - test_item - 16 Nov 2016 - Tested successfully
avatar Bakual
Bakual - comment - 16 Nov 2016

I have tested this item successfully on 94be45f


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12886.

avatar Bakual
Bakual - comment - 16 Nov 2016

Thanks for the reminder. I wanted to test it yesterday but forgot it ?

The issue #12812 still exists, but is probably unrelated to the issue here.

avatar laoneo
laoneo - comment - 16 Nov 2016

Yes, that's different

avatar brianteeman
brianteeman - comment - 6 Dec 2016

I have tested this item successfully on 94be45f


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12886.

avatar brianteeman brianteeman - test_item - 6 Dec 2016 - Tested successfully
avatar jeckodevelopment jeckodevelopment - change - 6 Dec 2016
Status Pending Ready to Commit
avatar jeckodevelopment
jeckodevelopment - comment - 6 Dec 2016

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12886.

avatar jeckodevelopment jeckodevelopment - change - 6 Dec 2016
Milestone Added:
avatar rdeutz
rdeutz - comment - 6 Dec 2016

@laoneo could you check the conflicts, thanks

avatar laoneo
laoneo - comment - 7 Dec 2016

Done

avatar rdeutz rdeutz - change - 7 Dec 2016
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2016-12-07 08:30:14
Closed_By rdeutz
avatar rdeutz rdeutz - close - 7 Dec 2016
avatar rdeutz rdeutz - merge - 7 Dec 2016
avatar rdeutz rdeutz - reference | c3dd70f - 7 Dec 16
avatar rdeutz rdeutz - merge - 7 Dec 2016
avatar rdeutz rdeutz - close - 7 Dec 2016
avatar laoneo
laoneo - comment - 7 Dec 2016

Thanks.

avatar laoneo laoneo - head_ref_deleted - 7 Dec 2016

Add a Comment

Login with GitHub to post a comment