User tests: Successful: Unsuccessful:
This PR allows better management for forms created with the JForm class and fixes a bug when adding a field on the fly to a form object
I will try to add some more information on this because it is not so easy to understand what is the benefit or why this change is needed.
Firstly it is good to know how we can build XML form definitions, it has tree levels:
<fields> (level1 == groups)
<fieldsets> (level2)
<field> (level3)
you can skip level1 and then you build your form only with
<fieldsets> (level2)
<field> (level3)
the latter is how it is done in 99 % of all forms in core Joomla, we have one form where we use level1 and that is the backend article form. I will skip level1 for now it isn’t needed to explain the issue.
One of the cool things with JForm is that you can output forms like this
foreach ($this->form->getFieldsets() as $fieldset)
{
$fieldsInFieldset = $this->form->getFieldset($fieldset->name);
foreach ($fieldsInFieldset as $field)
{
echo $field->label
echo $field->input;
}
}
This is a very simplified version you need to do some markup around it to make it look nice but technically that would be enough.
So if you need one field more in your form, you only have to add it to the xml and you are done.
That’s fine but the real power is that you can manipulate the form one the fly, JModelForm calls a „preprocessForm“ function and allows an extension developer to do a lot with a form object. One thing is to remove fields easily and that works fine. You can try to add a fields but if you are using the code snipped above you can’t see the added field. So why is this? If you add a field it will be added to the form object but you can’t define the fieldset it should be added to. So the change I made with adding a fieldset param to „setField“ is the missing pice for doing it. You can also name it as a bugfix. Keep in mind this is level2, we could do it for level1 with the group param but this would also fail because it not gets added to the right fieldset.
The 2nd change I made here is to add a new public function „getFieldXml“ it gives you the SimpleXml Object of a field. The reason for this is that sometime you will rearrange fields, think about a situation you will show field1, field2, field3 and under other circumstances field2, field3, field1. There isn’t a move or reorder function, what you have to do is: remove fields and add fields in the right order. To add a field you need a SimpleXml Object and there is no way (besides reflection) of getting it out of the form object, so you have to build the object by analysing the field properties. That’s all doable but painful and you can make mistakes easily.
Not easy to understand and how this can be tested, is not easy either.
We can do JFormObject manipulations in the template, this is easier for testing. So in a vanilla Joomla installation:
Go to the template folder of protostar and create two directories html/com_users/remind and html/com_users/reset
Copy the default.php files form components/com_users/views/remind/tmpl/ and components/com_users/views/reset/tmpl/ to the corresponding directories.
<?php
// create and add a field
$form = $this->form;
$field = new SimpleXMLElement('<field></field>');
$field->addAttribute('name', 'a_textfield');
$field->addAttribute('type', 'text');
$form->setField($field);
// get the internals of the object
echo "#<div style='text-align:left;font_size:1.2em;'><pre>";
print_r($form);
echo "</pre></div>#";
?>
Without the patch you can see in the object dump that the field is added to the object but that there isn't a form output.
Apply the patch and try it again, the different is that the field is added in the context of the fieldset in the object and that you can see a text field now in the form.
This is the main part and proving that the bug is fixed.
<?xml version="1.0" encoding="utf-8"?>
<form>
<fields name="group1">
<fieldset name="default" label="g1">
<field
name="email"
type="text"
class="validate-username"
description="COM_USERS_FIELD_PASSWORD_RESET_DESC"
filter="email"
label="COM_USERS_FIELD_PASSWORD_RESET_LABEL"
required="true"
size="30"
/>
<field
name="captcha"
type="captcha"
label="COM_USERS_CAPTCHA_LABEL"
description="COM_USERS_CAPTCHA_DESC"
validate="captcha"
/>
</fieldset>
</fields>
<fields name="group2">
<fieldset name="default2" label="g2">
<field
name="whatever"
type="text"
/>
</fieldset>
</fields>
</form>
You should see a form like this:
<?php
// create and add a field
$form = $this->form;
$field = new SimpleXMLElement('<field></field>');
$field->addAttribute('name', 'a_textfield');
$field->addAttribute('type', 'text');
$form->setField($field);
// get the internals of the object
echo "#<div style='text-align:left;font_size:1.2em;'><pre>";
print_r($form);
echo "</pre></div>#";
?>
Without the patch you can see in the objects dump that the field is added to the object but that there isn't a form output.
Do some more tests with changing the line "$form->setField($field);":
a) $form->setField($field, 'group1');
b) $form->setField($field, 'group2', true,'default2');
All these should fail, now apply the patch and you should see the output and the field added at the right spot.
You should have seen that you now are able to add a fields to a form and that this field is shown in the html output. You can address where it will be added and it even works with "complicated" 3 level forms. Before someone asked, if you have default fieldset in more than one group and you didn't say to which one it should be added it will be added to the first one. It also works now for replacing a field.
This shouldn't be a problem, there are only small chances that this patch will have an impact on existing forms.
zero
Status | New | ⇒ | Pending |
Category | ⇒ | Libraries |
At this point I'd consider that repo pretty much busted and borderline unusable to be honest. Don't waste your time.
Title |
|
Title |
|
Labels |
Added:
?
|
I have tested this item
While the first part of this PR works as described. The second: "it depends".
If I use the provided code: $form->setField($field), the field will be appended to group1 (default). The field is visible in the form. The same goes for using the group paramater like: $form->setField($field, 'group1');
Now, after changing the group to $form->setField($field, 'group2'); the field is no longer visible in the form and got appended at the very end in the xml structure, outside all existing groups. I guess that is expected behavior, since the second group does not have a fieldset named default and the code responsible for rendering the fields does not care about "orphans" at this point / in this context, correct?
Looking at the methods signature here, the example code sets the $replace variable to the name of the fieldset. Adding the third parameter like this: $form->setField($field, 'group2', true, 'default2'); will add the field to group2 / default2 and will make it appear back in the form at the expected place.
@matrikular you are right the test instructions are wrong, updated, thanks for testing
I have tested this item
looks good to me and works as per instruction. Thanks @rdeutz
But RTC should be onhold until @laoneo and @rdeutz are fine on the inline comment.
I guess it is this PR @laoneo ? #12813
Yes this pr.
Status | Pending | ⇒ | Ready to Commit |
RTC after two successful testes.
@franz-wohlkoenig please hold of the RTC state until @rdeutz and @laoneo are ok after checking that PR ;)
Status | Ready to Commit | ⇒ | Pending |
forgotten
np just set this back to pendig please.
done
Status | Pending | ⇒ | Ready to Commit |
Labels |
Added:
?
|
RTC. Thanks @rdeutz!
Status | Ready to Commit | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2017-04-05 16:43:39 |
Closed_By | ⇒ | Bakual | |
Labels |
Thanks to all who helped to get this merged, this bug is soooo long on my I have to fix it list, so happy that I remove it from my list :-)
I assume this also applies to the framework and needs to be fixed there too.
https://github.com/joomla-framework/form