Create possibility for nested subforms #17552

continga · 2017-08-15T18:19:20Z

Pull Request for Issue #11551

Summary of Changes

This PR adds the possibility to nest subforms into subforms, adding the possibility to stack them as many times as a user would like to.

Testing Instructions

Create a form with a subform inside of a subform and test whether it works as expected (with multiple=true or false, layout=repeatable or repeatable-table, different min/max values, etc...).

A simple testcase is to testwise edit e.g. plugins/fields/checkboxes/params/checkboxes.xml, adding e.g. the following after line 27:

<field
	name="subformlevel2"
	type="subform"
	label="PLG_FIELDS_CHECKBOXES_PARAMS_OPTIONS_LABEL"
	description="PLG_FIELDS_CHECKBOXES_PARAMS_OPTIONS_DESC"
	layout="joomla.form.field.subform.repeatable-table"
	icon="list"
	multiple="true"
>
	<form hidden="true" name="list_templates_modal" repeat="true">
		<field
			name="name_lvl2"
			type="text"
			label="PLG_FIELDS_CHECKBOXES_PARAMS_OPTIONS_NAME_LABEL"
			size="30"
		/>
	</form>
</field>

After that has been added, navigate to Content -> Fields -> New in the Administrator Backend, select "Checkboxes" as Type, and play around with the nested subforms.

To do some additional tests, play around with the XML definition (adding 3rd level subforms, changing the multiple/non-multiple stuff, required status, etc...) and take a look whether everything works as expected.

Expected result

Something like in the following screenshot, where I have stacked 3 subforms inside of each other with just some basic textfields:

Additional information

This is a backwards-compatible change and hence does not require anything to be worried about (imho) - please correct me if I am wrong, maybe we can find a solution then.

I am aware that the main merge window of Joomla! 3.8 already has passed, so I guess this PR probably will target version 3.9 or 4.0, but I open this PR now because it is my first slightly bigger PR for Joomla, and I expect some feedback-cycles before we reach a merge-ready state.

I already have a question to begin with: I only committed the uncompressed JS file, is it my job to do the minification, or will some deploy-script take care of that?

Thanks in advance!

brianteeman · 2017-08-15T18:26:38Z

I already have a question to begin with: I only committed the uncompressed JS file, is it my job to do the minification, or will some deploy-script take care of that?

For Joomla 3.x then yes please. You can use any minifier

brianteeman · 2017-08-15T19:06:13Z

@continga there are several codestyle errors that have been reported here http://213.160.72.75/joomla/joomla-cms/7879 that it would be great if you could resolve - thanks

laoneo · 2017-08-16T19:11:31Z

Cool feature, was waiting for it!! Does that also work with the default layout?

Fedik · 2017-08-17T16:37:07Z

libraries/joomla/form/fields/subform.php

+		// separate unique subform id present to could distinguish the eventhandlers
+		// regarding adding/moving/removing rows from nested subforms from their parents.
+		static $unique_subform_id = 0;
+		$data['unique_subform_id'] = ('sr-' . $unique_subform_id++);


name confusing, technically it is counter,
each field already have an ID $this->id, which is unique
just IMHO 😉

but seems not a bad idea to resolve "buttons" conflicts 😉

Fedik · 2017-08-17T16:43:05Z

layouts/joomla/form/field/subform/repeatable.php

+			<?php /** Do a rawurlencode to not tamper with HTML elements, especially with
+			        * nested subforms (subform in subform), this might contain a
+			        * </script> tag, which else blows up our markup. */ ?>
+			<?php echo rawurlencode(trim($this->sublayout(


I do not like it, and I would not accept it with rawurlencode. why? 😉

The problem is correct, we cannot have nested <script> tag.
But solution not the best.

I had idea to use <template> tag, wich can be nested. That would be a correct solution.
But need to search/test for <template> polyfill to make it work in IE.

Big thumbs up for template, in J4 we have a polyfill already!
I think another good idea is to move all the field specific scripts to custom elements. The advantage is pretty obvious, we define a class one and use it as many times as we want and also less dom (slow) queries...

Fedik · 2017-08-17T16:54:44Z

media/system/js/subform-repeatable-uncompressed.js

+		}
+		// Recursively replace our basename + old group with basename + new group
+		// inside of nested subform template elements.
+		this.recursiveReplaceNested($row, search, replace);


hm, I have not tested but it looks not safe.
Each instance of subformRepeatable should work only with "own" fields, and should not care about nested/parent etc.

Additionally, you have fixed only the input name,
but it also require to fix an input id and label for.

This is one of complex issue with nested subform 😉

hm, I see, you fixed only nested template,
but the inputs attributes of this template will be calculated depend from data-base-name, data-group, later, when subform add new row

Fedik · 2017-08-17T17:10:25Z

@continga thanks for doing it, it is often requested feature 😉

laoneo · 2017-09-05T15:03:15Z

Is this ready for testing? Because I tested it and it worked, but the minified script is missing. I really would like to get that in.

continga · 2017-09-05T15:41:39Z

I had some other things to do the last 2 weeks, but will continue to work on this soon to include the feedback from above and fix the CI fails

continga · 2017-09-12T22:20:26Z

I have now included all your feedback (I at least hope so ;-)). Would be nice if you guys now could take another look. Thanks!

laoneo · 2017-09-13T11:45:48Z

I have tested this item ✅ successfully on f485db8

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17552.}

drmenzelit · 2017-10-05T10:48:05Z

I have tested this item ✅ successfully on f485db8

Tested successfully with 3 level subforms like the example

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17552.}

continga · 2017-10-12T09:14:05Z

Is there anything we can do for merging this PR. If it's merged we like to go on with more PRs to add repeatable subforms to custom fields.

ghost · 2017-10-12T09:16:54Z

RTC after two successful tests.

Thanks for Hint, @continga

houndci-bot · 2017-11-07T01:04:41Z

media/system/js/subform-repeatable-uncompressed.js

+		 */
+		var nestedTemplates = $row.find(this.options.rowTemplateSelector);
+		// If we found it, iterate over the found ones (might be more than one!)
+		for (var i = 0; i < nestedTemplates.length; i++) {


'i' is already defined.

houndci-bot · 2017-11-07T01:04:41Z

media/system/js/subform-repeatable-uncompressed.js

+	) {
+		var group = (typeof group === 'undefined' ? $row.attr('data-group') : group),
+			basename = (typeof basename === 'undefined' ? $row.attr('data-base-name') : basename),
+			count    = (typeof count === 'undefined' ? 0 : count),


'count' is already defined.

houndci-bot · 2017-11-07T01:04:41Z

media/system/js/subform-repeatable-uncompressed.js

+		basename // group base name, without count, e.g. 'options'
+	) {
+		var group = (typeof group === 'undefined' ? $row.attr('data-group') : group),
+			basename = (typeof basename === 'undefined' ? $row.attr('data-base-name') : basename),


'basename' is already defined.

houndci-bot · 2017-11-07T01:04:41Z

media/system/js/subform-repeatable-uncompressed.js

+		group, // current group name, e.g. 'optionsX'
+		basename // group base name, without count, e.g. 'options'
+	) {
+		var group = (typeof group === 'undefined' ? $row.attr('data-group') : group),


'group' is already defined.

…forms options for the different types.

tonypartridge · 2017-12-19T08:58:40Z

@mbabker why the 3.9 release? Can we get this in 3.8.4 please since it’s a big fix as there is nothing to say that it can only be one level deep? It should be multiple and therefore a bug.

ghost · 2017-12-19T09:22:56Z

@tonypartridge as far as i know: new Features are shipped with new Release like 3.9., 3.8.* are for Fixes.

tonypartridge · 2017-12-19T09:23:50Z

Exactly, this is a bug fix.

…

On 19 Dec 2017, 09:23 +0000, Franz Wohlkönig ***@***.***>, wrote: @tonypartridge as far as i know: new Features are shipped with new Release like 3.9., 3.8.* are for Fixes. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

ghost · 2017-12-19T09:37:48Z

ah, thought its a new Feature (Label).

This is the 1st commit message: First WIP version of supporting nested subforms. This is the commit message joomla#2: Fix nested subforms not getting correct name attribute on input fields. This is the commit message joomla#3: Fix subform rows having invalid index, fix small typo. This is the commit message joomla#4: Replace the unique subform id via random bytes by just an increasing integer in the fields type rendering process. This is the commit message joomla#5: Implement feedback from PR at Joomla; - Use a <template> HTML element for the template of the subform rows, not a url encoded string inside of a <script> element. - Fix code style errors reported by phpcs. - Make the fixing of the unique attributes (name, id, etc) of input elements of nested subform rows more errorprone, using the same method as the main subform row. - Manually add a minified version of the javascript file. This is the commit message joomla#6: Fix failing javascript tests due to changed structure of subform repeatable template container. This is the commit message joomla#7: Change subform repeatable javascript test to correctly check on 0-indexed rows, previously they have been 1-indexed. This is the commit message joomla#8: Fix a problem where multi-level subforms on the same level doesnt trigger their template correctly. Additionally added a note why the fixScripts() method is broken and how it could get better. This is the commit message joomla#9: First commit which integrates the subform custom field type. This is the commit message joomla#10: Rename manifest file for plg_fields_subform This is the commit message joomla#11: plg_fields_subform: Improve code comments and made the subform field type optionally non-repeatable. This is the commit message joomla#12: Fix not being able to change the type of a subfield. This is the commit message joomla#13: Implement a small in-memory renderCache for subform custom fields This is the commit message joomla#14: Prepare to load the subfields into a subform custom field directly via their own xml definition. This is the commit message joomla#15: After merge with PR joomla#17552 we can now successfully show the subforms options for the different types. This is the commit message joomla#16: The subform custom field type doesnt need a default value. This is the commit message joomla#17: Implement an option for a subform customfield to disable rendering the field values. This is the commit message joomla#18: Remove unused code and set hidden fields to not-required. This is the commit message joomla#19: Reset for dev

This is the 1st commit message: First commit which integrates the subform custom field type. This is the commit message joomla#2: Rename manifest file for plg_fields_subform This is the commit message joomla#3: plg_fields_subform: Improve code comments and made the subform field type optionally non-repeatable. This is the commit message joomla#4: Fix not being able to change the type of a subfield. This is the commit message joomla#5: Implement a small in-memory renderCache for subform custom fields This is the commit message joomla#6: Prepare to load the subfields into a subform custom field directly via their own xml definition. This is the commit message joomla#7: After merge with PR joomla#17552 we can now successfully show the subforms options for the different types. This is the commit message joomla#8: The subform custom field type doesnt need a default value. This is the commit message joomla#9: Implement an option for a subform customfield to disable rendering the field values. This is the commit message joomla#10: Remove unused code and set hidden fields to not-required.

First commit which integrates the subform custom field type. This is the commit message joomla#2: Rename manifest file for plg_fields_subform This is the commit message joomla#3: plg_fields_subform: Improve code comments and made the subform field type optionally non-repeatable. This is the commit message joomla#4: Fix not being able to change the type of a subfield. This is the commit message joomla#5: Implement a small in-memory renderCache for subform custom fields This is the commit message joomla#6: Prepare to load the subfields into a subform custom field directly via their own xml definition. This is the commit message joomla#7: After merge with PR joomla#17552 we can now successfully show the subforms options for the different types. This is the commit message joomla#8: The subform custom field type doesnt need a default value. This is the commit message joomla#9: Implement an option for a subform customfield to disable rendering the field values. This is the commit message joomla#10: Remove unused code and set hidden fields to not-required. Signed-off-by: Rene Pasing <r.pasing@continga.de>

dawe78 · 2018-10-01T08:01:30Z

First of all, thanks for developing, testing and adding nested subforms to beta3!!!

I updated to 3.9.0 beta3 on saturday and startet testing my component using nested subform, but I got an issue...

Steps:

First subform name "order_articles"
inside subform order_articles, nested subform "order_addons"

Click add-row for new article: row created width id "jform_order_articles__order_articles0__[nameofFields]"
Click add-row for new addon inside article: row created "jform_order_articles__order_articles1__order_addons__order_addons0__addon"

As you can see, nested subform count up article row num; subform should be jform_order_articles__order_articles0_..., but it creates jform_order_articles__order_articles1_...

I tested this behaviour only with my own component form until now, but I will do this test with a blank test form as soon as possible and report the results. Maybe someone has the same issue?

dawe78 · 2018-10-01T08:45:27Z

Okay, tested on blank form, same issue... Nested subform starts counting on parent subform row num, eg row "order_articles2" subform starts at "order_articles3__order_addons__order_addons0".

dawe78 · 2018-10-01T08:50:59Z

Additional information: after saving form (only parent subform rows saved) adding nested subform rows results in correct row count. So this issue occurs only on new added subform rows, after saving subform row adding nested subform rows works fine.

zero-24 · 2018-10-01T09:28:23Z

Can you please open an new issue mention this here? As this is a closed / merged issue.

dawe78 · 2018-10-01T10:16:42Z

Okay, sorry...

First commit which integrates the subform custom field type. This is the commit message joomla#2: Rename manifest file for plg_fields_subform This is the commit message joomla#3: plg_fields_subform: Improve code comments and made the subform field type optionally non-repeatable. This is the commit message joomla#4: Fix not being able to change the type of a subfield. This is the commit message joomla#5: Implement a small in-memory renderCache for subform custom fields This is the commit message joomla#6: Prepare to load the subfields into a subform custom field directly via their own xml definition. This is the commit message joomla#7: After merge with PR joomla#17552 we can now successfully show the subforms options for the different types. This is the commit message joomla#8: The subform custom field type doesnt need a default value. This is the commit message joomla#9: Implement an option for a subform customfield to disable rendering the field values. This is the commit message joomla#10: Remove unused code and set hidden fields to not-required. Signed-off-by: Rene Pasing <r.pasing@continga.de>

dawe78 · 2019-08-01T11:57:42Z

Hey guys, I got an issue with nested subforms in IE 11. While nested subforms are working on FF, Chrome, Opera... , IE 11 crashed the subforms (see images below). Does anyone else got the same issue? Joomla Version is 3.9.10 (latest).

Expected result (FF):
screen shot 2019-08-01 at 11 56 52!

IE 11 Result:
screen shot 2019-08-01 at 11 56 53

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17552.}

dawe78 · 2019-08-01T12:00:54Z

Forgot: I got this issue when I click to add a new row (main item). Existing subforms (loaded form data), including nested subforms, are displayed as expected...

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17552.}

dgrammatiko · 2019-08-01T12:05:08Z

Check the IE console, I guess the error is something like template is not defined.

Resolution ie needs a poly fill for template

tonypartridge · 2019-08-01T12:05:52Z

Sound like unsupported JS In IE 11. Does the console say anything?

…

On Thu, 1 Aug 2019 at 13:01, dawe78 ***@***.***> wrote: Forgot: I got this issue when I click to add a new row (main item). Existing subforms (loaded form data), including nested subforms, are displayed as expected... ------------------------------ This comment was created with the J!Tracker Application <https://github.com/joomla/jissues> at issues.joomla.org/tracker/joomla-cms/17552. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#17552?email_source=notifications&email_token=AAKWBFUREHHNLS65EYBCEA3QCLGAPA5CNFSM4DW7W3S2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3KLCAQ#issuecomment-517255426>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAKWBFSPVXUJNPWIXHMILL3QCLGAPANCNFSM4DW7W3SQ> .

dawe78 · 2019-08-01T12:16:00Z

Hi dgrammatiko, thanks for your reply.

Debugging with the console is the first thing I always do to find out whats going on. But there is no error or warning output...

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17552.}

dawe78 · 2019-08-01T12:18:37Z

Hi tonypartridge, thanks too! No, unfortunally there is no error or warning output... I expected a JS warning or error, but there is none...

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17552.}

dgrammatiko · 2019-08-01T12:25:09Z

@dawe78 try adding

<script src="https://unpkg.com/@webcomponents/template@1.4.0/template.js"></script>

at the beginning of the head section of your template (isis?). Does that fix the problem?

dawe78 · 2019-08-01T12:43:40Z

Yes, this JS-File fixes the issue. Is there any update for ISIS available which fixes the bug? Or do I need to add your script to all my joomla installations by myself???

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17552.}

dgrammatiko · 2019-08-01T12:47:06Z

Is there any update for ISIS available which fixes the bug? Or do I need to add your script to all my joomla installations by myself???

The process of adding the script to the template was purely for testing purposes, the actual solution requires some deferent steps (namely calling this script before the repeatable one)

dawe78 · 2019-08-01T14:42:00Z

Okay, is there a tutorial which describes the steps to do?

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17552.}

dgrammatiko · 2019-08-01T16:12:41Z

I don't think so but all is needed is adding

JHtml::_('script', 'system/template-polyfill.js', array('version' => 'auto', 'relative' => true));

before

	JHtml::_('script', 'system/subform-repeatable.js', array('version' => 'auto', 'relative' => true));

line:

joomla-cms/layouts/joomla/form/field/subform/repeatable.php

Line 33 in c372ec3

    
           JHtml::_('script', 'system/subform-repeatable.js', array('version' => 'auto', 'relative' => true));

Also the same in the other layout (table)

The only thing is that the js file I pointed before needs to be wrapped in an if statement, so it only executes on the browsers that they need it.
something like

  function needsTemplatePolyfill() {
    // no real <template> because no `content` property (IE and older browsers)
    var template = document.createElement('template');
    if (!('content' in template)) {
      return true;
    }
    // broken doc fragment (older Edge)
    if (!(template.content.cloneNode() instanceof DocumentFragment)) {
      return true;
    }
    // broken <template> cloning (Edge up to at least version 17)
    var template2 = document.createElement('template');
    template2.content.appendChild(document.createElement('div'));
    template.content.appendChild(template2);
    var clone = template.cloneNode(true);
    return (
      clone.content.childNodes.length === 0 ||
      clone.content.firstChild.content.childNodes.length === 0
    );
  }

if (needsTemplatePolyfill()) {

// The code of the polyfill goes here
}

joomla-cms-bot added the PR-staging label Aug 15, 2017

brianteeman added the Feature label Aug 15, 2017

Fedik reviewed Aug 17, 2017

View reviewed changes

continga force-pushed the issue-11551 branch from cb61832 to a26269c Compare August 31, 2017 21:21

laoneo mentioned this pull request Sep 5, 2017

Multi level subform #11551

Closed

continga force-pushed the issue-11551 branch from a26269c to c449f1f Compare September 12, 2017 22:19

joomla-cms-bot added the Unit/System Tests label Sep 12, 2017

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Oct 12, 2017

continga force-pushed the issue-11551 branch from f485db8 to 7c943d7 Compare November 7, 2017 01:04

houndci-bot reviewed Nov 7, 2017

View reviewed changes

continga pushed a commit to continga/joomla-cms that referenced this pull request Nov 7, 2017

After merge with PR joomla#17552 we can now successfully show the sub…

7efaeba

…forms options for the different types.

continga pushed a commit to continga/joomla-cms that referenced this pull request Nov 7, 2017

After merge with PR joomla#17552 we can now successfully show the sub…

306d316

…forms options for the different types.

mbabker added this to the Joomla 3.9.0 milestone Nov 24, 2017

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Sep 22, 2018

dawe78 mentioned this pull request Oct 1, 2018

Row count incorrect on nested subform #22441

Closed

continga mentioned this pull request Oct 1, 2018

Add 'subfields' custom field type #22446

Closed

Harmageddon mentioned this pull request Oct 3, 2018

Subforms: Implementing per-field filters and validation. #14189

Closed

gaelicwinter mentioned this pull request Oct 17, 2018

Nested subform improperly nesting fields #22690

Closed

SharkyKZ mentioned this pull request Oct 31, 2018

Remove required field option on config_domain.xml #22880

Closed

chrisdavenport mentioned this pull request Nov 28, 2018

Re-add generic CSS classes in subforms. #23190

Merged

wilsonge mentioned this pull request Apr 10, 2019

[4.0] Subform field conflicts when merging to J4 #23279

Closed

Create possibility for nested subforms #17552

Create possibility for nested subforms #17552

Conversation

continga commented Aug 15, 2017 • edited by joomla-cms-bot

Summary of Changes

Testing Instructions

Expected result

Additional information

brianteeman commented Aug 15, 2017

brianteeman commented Aug 15, 2017

laoneo commented Aug 16, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Fedik Aug 17, 2017 • edited

Choose a reason for hiding this comment

Fedik commented Aug 17, 2017

laoneo commented Sep 5, 2017

continga commented Sep 5, 2017

continga commented Sep 12, 2017

laoneo commented Sep 13, 2017

drmenzelit commented Oct 5, 2017

continga commented Oct 12, 2017

ghost commented Oct 12, 2017 • edited by ghost

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tonypartridge commented Dec 19, 2017

ghost commented Dec 19, 2017

tonypartridge commented Dec 19, 2017 via email

ghost commented Dec 19, 2017

dawe78 commented Oct 1, 2018

dawe78 commented Oct 1, 2018

dawe78 commented Oct 1, 2018

zero-24 commented Oct 1, 2018

dawe78 commented Oct 1, 2018

dawe78 commented Aug 1, 2019

dawe78 commented Aug 1, 2019

dgrammatiko commented Aug 1, 2019

tonypartridge commented Aug 1, 2019 via email

dawe78 commented Aug 1, 2019

dawe78 commented Aug 1, 2019

dgrammatiko commented Aug 1, 2019 • edited

dawe78 commented Aug 1, 2019

dgrammatiko commented Aug 1, 2019

dawe78 commented Aug 1, 2019

dgrammatiko commented Aug 1, 2019

continga commented Aug 15, 2017 •

edited by joomla-cms-bot

Fedik Aug 17, 2017 •

edited

ghost commented Oct 12, 2017 •

edited by ghost

dgrammatiko commented Aug 1, 2019 •

edited