Nested subform compatibility #362

New Issue

Closed

opened 2019-01-12 12:15:22 +00:00 by AlexDings · 7 comments

AlexDings commented

2019-01-12 12:15:22 +00:00

(Migrated from github.com)

It is currently not possible to nest subforms within JCB.

Subforms that are selected within the 'fields' property of the parent subform get ignored when the field is generated.

Joomla version:

Joomla! 3.9.1 Stable.

JCB version:

JCB 2.9.8 (up to date at moment of writing)

Steps taken:

Created a subform & added it to an admin view
selected a textfield and another subform as its 'fields'
generated the admin view

Epected result:

A subform with a textfield and a nested subform

Result:

A subform with a textfield and no nested subform

Possible issue:

JCB is possibly not programmed to generate nested subforms. When examining the admin view xml under models>forms it is clear that the nested subform is not correctly generated.
When manually manipulating this xml found under models>forms to have nested subforms in the admin view the admin view seems to save this as expected.

Possible solution:

Most of the components to make this work are already there, on the user site.
A user can create multiple subforms like they normally would and then add the subform 'id' to the 'fields' property of the subform that should contain the subform.

The solution is in the way JCB generates XML. Currently, a subform (1) field inside a subform (0) field gets ignored by the JCB compiler. JCB could generate this nested subform (1) just like it would a normal subform (0). JCB should repeat this nesting until a subform ID is referenced that is already in the traversed path, or there are no subforms to nest anymore.
JCB could also just implement a hard depth limit, which would take the risk out of possible loops.

There is nothing special about the XML itself, it is pasting the same generated subforms into other subforms. Here a anonymised example:

<!-- Main Subform Field. Type: Subform. (joomla) -->
<field
  type="subform"
  name="level0"
  label="Level 0 Subform"
  layout="joomla.form.field.subform.repeatable-table"
  multiple="true"
  description="Level 0 Subform"
  default=""
  icon="list"
  maximum="100000">
  <form hidden="true" name="list_test" repeat="true">
    <!-- Evegrp Field. Type: Text. (joomla) -->
    <field
        type="text"
        name="level1title"
        label="Level 1 Title"
        size="100"
        default=""
        description="Level 1 Title"
        class="text_area"
        readonly="false"
        disabled="false"
        required="false"
        filter="STRING"
        message="Message"
        autocomplete="on"
    />
    <!-- Subgroup Field. Type: Subform. (joomla) -->
    <field
      type="subform"
      name="level1"
      label="Level 1 Subform"
      layout="joomla.form.field.subform.repeatable-table"
      multiple="true"
      description="Level 1 Subform"
      default=""
      icon="list"
      maximum="100000">
      <form hidden="true" name="list_test" repeat="true">
               
          Etc.
          
        </form>
    </field>
  </form>
</field>

It is currently not possible to nest subforms within JCB. Subforms that are selected within the 'fields' property of the parent subform get ignored when the field is generated. ### Joomla version: Joomla! 3.9.1 Stable. ### JCB version: JCB 2.9.8 (up to date at moment of writing) ### Steps taken: 1. Created a subform & added it to an admin view 2. selected a textfield and another subform as its 'fields' 3. generated the admin view ### Epected result: A subform with a textfield and a nested subform ### Result: A subform with a textfield and no nested subform ### Possible issue: JCB is possibly not programmed to generate nested subforms. When examining the admin view xml under models>forms it is clear that the nested subform is not correctly generated. When manually manipulating this xml found under models>forms to have nested subforms in the admin view the admin view seems to save this as expected. ### Possible solution: Most of the components to make this work are already there, on the user site. A user can create multiple subforms like they normally would and then add the subform 'id' to the 'fields' property of the subform that should contain the subform. The solution is in the way JCB generates XML. Currently, a subform (1) field inside a subform (0) field gets ignored by the JCB compiler. JCB could generate this nested subform (1) just like it would a normal subform (0). JCB should repeat this nesting until a subform ID is referenced that is already in the traversed path, or there are no subforms to nest anymore. JCB could also just implement a hard depth limit, which would take the risk out of possible loops. There is nothing special about the XML itself, it is pasting the same generated subforms into other subforms. Here a anonymised example: ~~~~  <field type="subform" name="level0" label="Level 0 Subform" layout="joomla.form.field.subform.repeatable-table" multiple="true" description="Level 0 Subform" default="" icon="list" maximum="100000"> <form hidden="true" name="list_test" repeat="true">  <field type="text" name="level1title" label="Level 1 Title" size="100" default="" description="Level 1 Title" class="text_area" readonly="false" disabled="false" required="false" filter="STRING" message="Message" autocomplete="on" />  <field type="subform" name="level1" label="Level 1 Subform" layout="joomla.form.field.subform.repeatable-table" multiple="true" description="Level 1 Subform" default="" icon="list" maximum="100000"> <form hidden="true" name="list_test" repeat="true"> Etc. </form> </field> </form> </field> ~~~~

Llewellyn commented

2019-01-12 16:42:19 +00:00

I like this... interesting really. Yes all the tools are there, and I do block the option to include subforms in subforms as at the time it did not work, or so I thought.

Okay so lets see... The code that does this are found at two places, here on line 1453 when using string manipulation (our preference) and here on line 1784 when using the simpleXMLElement builder approach.

Well this is from where we need to follow the rabbit 👍

So lets focus just on one aria and see how this works out (both are actually doing the same thing).. I will use my preference area so between line 1482 to 1516 we are are allowing three types of fields, option plain and custom and so any other type of fields just get ignored. But in reality the method call $this->setField is a recall of the whole processes at work. So it causes recursion to take place in a controlled way, adding subforms in at this level... is what you are suggesting right.

Okay so let just add the following lines at line 1496 as a test, and let me know what happened at your end

elseif ($r_typeName === 'subform') // how many to allow hmmm
{
	// we need more tabs every time around (TODO)
	$r_taber = $this->_t(3);
	// now add to the field set
	$field .= $this->setField('special', $r_fieldValues, $r_name, $r_typeName, $langView, $view_name_single, $view_name_list, $placeholders, $r_optionArray, null, $r_taber);
}

This should just work... I have not tested it, but you are welcome to try it out and let me know. O yes remember this is in the string manipulation implementation, so if you are using the simpleXMLElement option, the lines go in at line 1865 and look like this

elseif ($r_typeName === 'subform') // how many to allow hmmm
{
	// now add to the field set
	ComponentbuilderHelper::xmlAppend($form, $this->setField('special', $r_fieldValues, $r_name, $r_typeName, $langView, $view_name_single, $view_name_list, $placeholders, $r_optionArray));
}

If that just works, it will be stunning!

I like this... interesting really. Yes all the tools are there, and I do block the option to include subforms in subforms as at the time it did not work, or so I thought. Okay so lets see... The code that does this are found at two places, here on [line 1453](https://github.com/vdm-io/Joomla-Component-Builder/blob/staging/admin/helpers/compiler/c_Fields.php#L1453) when using string manipulation (our preference) and here on [line 1784](https://github.com/vdm-io/Joomla-Component-Builder/blob/staging/admin/helpers/compiler/c_Fields.php#L1784) when using the simpleXMLElement builder approach. Well this is from where we need to follow the rabbit :+1: So lets focus just on one aria and see how this works out (both are actually doing the same thing).. I will use my preference area so between line [1482](https://github.com/vdm-io/Joomla-Component-Builder/blob/staging/admin/helpers/compiler/c_Fields.php#L1482) to [1516](https://github.com/vdm-io/Joomla-Component-Builder/blob/staging/admin/helpers/compiler/c_Fields.php#L1516) we are are allowing three types of fields, `option` `plain` and `custom` and so any other type of fields just get ignored. But in reality the method call `$this->setField` is a recall of the whole [processes at work](https://github.com/vdm-io/Joomla-Component-Builder/blob/staging/admin/helpers/compiler/c_Fields.php#L1168). So it causes recursion to take place in a controlled way, adding subforms in at this level... is what you are suggesting right. Okay so let just add the following lines at [line 1496](https://github.com/vdm-io/Joomla-Component-Builder/blob/staging/admin/helpers/compiler/c_Fields.php#L1496) as a test, and let me know what happened at your end ``` elseif ($r_typeName === 'subform') // how many to allow hmmm { // we need more tabs every time around (TODO) $r_taber = $this->_t(3); // now add to the field set $field .= $this->setField('special', $r_fieldValues, $r_name, $r_typeName, $langView, $view_name_single, $view_name_list, $placeholders, $r_optionArray, null, $r_taber); } ``` This should just work... I have not tested it, but you are welcome to try it out and let me know. O yes remember this is in the string manipulation implementation, so if you are using the simpleXMLElement option, the lines go in at [line 1865](https://github.com/vdm-io/Joomla-Component-Builder/blob/staging/admin/helpers/compiler/c_Fields.php#L1865) and look like this ``` elseif ($r_typeName === 'subform') // how many to allow hmmm { // now add to the field set ComponentbuilderHelper::xmlAppend($form, $this->setField('special', $r_fieldValues, $r_name, $r_typeName, $langView, $view_name_single, $view_name_list, $placeholders, $r_optionArray)); } ``` If that just works, it will be stunning!

👍 2

ro-ot commented

2019-01-12 17:13:45 +00:00

(Migrated from github.com)

I just tested, and this works. But the tabs are not correct, and there needs to be a hard depth limit.

👍 1

Llewellyn commented

2019-01-12 17:38:34 +00:00

What will be a reasonable depth limit? four layers in? so that will allow one subform to have in one column three more subforms. That may not even look that good... but it will work.

Okay... that was fast, thanks! @AlexDings if you can also confirm that this change is stable. Then we will push it out. I will add some limitations and see what I can do to fix the statistics of the XML in the string manipulation area. What will be a reasonable depth limit? four layers in? so that will allow one subform to have in one column three more subforms. That may not even look that good... but it will work.

👍 1

AlexDings commented

2019-01-13 15:20:49 +00:00

(Migrated from github.com)

I tested it throughout the day today and it works like a charm.
My subform nesting situation has 4 layers currently. It is just for people to structure items so this is a very visual way of showing the groups. I would suggest 5 layers as a minimal maximum.

I tested it throughout the day today and it works like a charm. My subform nesting situation has 4 layers currently. It is just for people to structure items so this is a very visual way of showing the groups. I would suggest 5 layers as a minimal maximum.

👍 1

Llewellyn commented

2019-01-13 20:07:13 +00:00

But you do realize this means another tracker array in memory...

About the statistics of the XML in the string manipulation area, I think just leaving it as is should be okay, yes it does not look perfect, but any who read it will clearly see it is special, nested. Here the simpleXMLElement works better in statistics.

Okay at this point, the only kind of loop we want to stop is a subform included into itself by one of the nested subforms, as this will cause and endless loop, all the other will run to a halt, but that one will hit the infinite loop. So instead of adding a depth limit I am think, lets just prevent the above, and that way we will, or should be in the clear. But you do realize this means another tracker array in memory... About the statistics of the XML in the string manipulation area, I think just leaving it as is should be okay, yes it does not look perfect, but any who read it will clearly see it is special, nested. Here the simpleXMLElement works better in statistics.

AlexDings commented

2019-01-13 20:48:25 +00:00

(Migrated from github.com)

Either way works for me. A reasonable depth limit or a tracker array.
I agree that the tracker is the more elegant solution.

Either way works for me. A reasonable depth limit or a tracker array. I agree that the tracker is the more elegant solution.

Llewellyn commented

2019-01-14 15:37:04 +00:00

Okay this changes has been committed to staging branch, with a limit of 20 nested per subfrom. This should do the trick... let me know if you run into any issues.