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

@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

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

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

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.

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

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

I have tested this item ✅ successfully on f485db8

I have tested this item ✅ successfully on f485db8

Tested successfully with 3 level subforms like the example

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.

RTC after two successful tests.

Thanks for Hint, @continga

@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.

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

Exactly, this is a bug fix.

On 19 Dec 2017, 09:23 +0000, Franz Wohlkönig notifications@github.com, 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.

ah, thought its a new Feature (Label).

As much I would like to see this pr in 3.8.x, for me this is not a bug fix. Even when it is marked as bug fix, I would be hesitant to be added in a patch release just because of the amount of code changes. I would leave it for 3.9.

There is a 3.8.4 release planned already though.

Given documentation doesn’t say it should only be 1 level deep it should be multiple levels deep and as it is not it’s therefore a bug.

On 19 Dec 2017, 10:05 +0000, Allon Moritz notifications@github.com, wrote:

As much I would like to see this pr in 3.8.x, for me this is not a bug fix. Even when it is marked as bug fix, I would be hesitant to be added in a patch release just because of the amount of code changes. I would leave it for 3.9.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

I totally agree. It's definitely a bug fix.

The documentation does not state that Joomla cannot make coffee, therefore Joomla's failure to make coffee is a bug. I suggest that someone submits a PR to fix that in 3.8.4 as I could really use more coffee (or tea).

Maintenance releases such as 3.8.4 should always aim to increase stability. Due to the size of this change there is a chance that it may itself contain bugs and hence decrease stability. Therefore, imho, it belongs in the next minor release (ie. 3.9.0).

That’s just plain stupid terminoligy Chris. The documentation and function clearly allows for selections based and showon values. The fact is it DOES NOT WORK past 1 level and therefore it is a bug. But that it isn’t just documented. It’s the fact it DOES NOT WORK.

If we were adding this for the first time then yes it is a feature, we are fixing the previously implemented feature.

On 19 Dec 2017, 10:33 +0000, Chris Davenport notifications@github.com, wrote:

The documentation does not state that Joomla cannot make coffee, therefore Joomla's failure to make coffee is a bug. I suggest that someone submits a PR to fix that in 3.8.4 as I could really use more coffee (or tea).
Maintenance releases such as 3.8.4 should always aim to increase stability. Due to the size of this change there is a chance that it may itself contain bugs and hence decrease stability. Therefore, imho, it belongs in the next minor release (ie. 3.9.0).
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

Even the description you wrote says its a new feature to me

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

Just for reference, can you point to the documentation article where it is defined that the subform field should support nested subforms?

@laoneo again it's terminology. The nested subforms are an additional field into a form which loads more forms and you should be able to use the showon as you do everywhere else as per the showon documentation: https://docs.joomla.org/Form_field. This allows that to function, it doesn't explicitly state about subforms but the principle is the same.

To me again it's fixing a bug in the original implementation of the subforms.

But then the side effect of this pr is to fix the showon bug. It's main purpose is to add a new feature to support subforms in subforms.

@tonypartridge it not a bug fix at all, it a new and very complex feature,
with a complex logic behind, which possible still a buggy,
and as @laoneo already noted, there was no declaration that nested forms are supported

The nested subforms are an additional field into a form which loads more forms and you should be able to use the showon as you do everywhere else as per the showon

subform field it is not a simple field itself,
make it nested, not only lead to code complication but also to performance loss (depends on the complexity of the form),
so I would not compare it with other regular fields

I've tested the feature in the latest dev release (with two nested forms) but I found it a bit bugged...In particoular, cliccking the "more" on the external sbuform adds another copy of the innest subform instead of the one requested.
I can't add another copy of the external form.

Thank you

Since September 11th, Joomla 3.9 Beta is available for testing, so I updated to 3.9 Beta, but nested subform problem persists! It seems like this bug is still not fixed in 3.9!

It seems like this bug is still not fixed in 3.9!

Well as you can see the pull request is still open. And based on the last few comments, it would appear that there are a couple of issues that need to be addressed. Until all issues are addressed and the pull request is appropriately tested, this won't be merged.

Until all issues are addressed and the pull request is appropriately tested, this won't be merged.

So this means, nested subforms will not be added to 3.9.x because its classified as feature, not bug fix, and new features won't be added to an existing version? Do we have to wait until 3.10 or 4.0?

@dawe78 basically a Pull request needs minimal 2 successfully Tests, which isn't given in this Case.

@continga could you make the code style changes so we can get two tests?

It wasn't just the codestyle comments, #17552 (comment) seems to imply there's a bug with this PR too.

I’m going to have a crack at this tonight and see if I can make any progress, else I’ll have to code my own implementation which is not as a great a long term solution.

Sent from my iPhone

On Sep 13, 2018, at 10:07 AM, Michael Babker notifications@github.com wrote:

It wasn't just the codestyle comments, #17552 (comment) seems to imply there's a bug with this PR too.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

I will continue to work in this PR on this weekend and check all mentioned errors, the bug report etc. I hope I can give you a testable version latest on sunday.

To be honest, for me it seemed like there was no interest from the community in this PR, why I de-prioritized working on it. I am pretty glad to hear this is not the case, so I will gladly continue on this. I will report back here latest on monday with results.

No we absolutely want this PR :-). It's just we all have busy lives and there are many PR's. Looking forward to the update.

On Sep 13 2018, at 10:59 pm, continga notifications@github.com wrote:

I will continue to work in this PR on this weekend and check all mentioned errors, the bug report etc. I hope I can give you a testable version latest on sunday.
To be honest, for me it seemed like there was no interest from the community in this PR, why I de-prioritized working on it. I am pretty glad to hear this is not the case, so I will gladly continue on this. I will report back here latest on monday with results.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub (https://link.getmailspring.com/link/1536876006.local-6215dab3-455f-v1.4.2-f587b7b7@getmailspring.com/0?redirect=https%3A%2F%2Fgithub.com%2Fjoomla%2Fjoomla-cms%2Fpull%2F17552%23issuecomment-421166764&recipient=cmVwbHkrMDAxNTYwOTY5NmJjNzU3ZjgyNTg4Y2NhNDIxNGRiZGFiZTQzYzVlY2FjOTZmMzNkOTJjZjAwMDAwMDAxMTdiMjlmY2Y5MmExNjljZTBlZWNiOGI3QHJlcGx5LmdpdGh1Yi5jb20%3D), or mute the thread (https://link.getmailspring.com/link/1536876006.local-6215dab3-455f-v1.4.2-f587b7b7@getmailspring.com/1?redirect=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FABVgluTMAxvyht97GXWlDa8gIOiOOT9Iks5uatVPgaJpZM4O37bl&recipient=cmVwbHkrMDAxNTYwOTY5NmJjNzU3ZjgyNTg4Y2NhNDIxNGRiZGFiZTQzYzVlY2FjOTZmMzNkOTJjZjAwMDAwMDAxMTdiMjlmY2Y5MmExNjljZTBlZWNiOGI3QHJlcGx5LmdpdGh1Yi5jb20%3D).

I will continue to work in this PR on this weekend and check all mentioned errors, the bug report etc. I hope I can give you a testable version latest on sunday.

Great, thank you so much! Let me know if I can help you, e.g testing!

@dawe78 if this PR is ready for Test it will be written here so you get automatically informed.

for note: this already implemented for Joomla! 4 #19184

Hi, I updated the PR to include all feedback so far. I would be happy if some of you guys could test this so I could work on possible feedback

I was not able to reproduce any issue you mentioned, @Polm90 - can you maybe be more specific about that, so what did you do, how was the result, what did you expect?

Additionally, I don't understand why the Drone Build was failing, it doesn't look to be related to any changes I made. Maybe the build had a temporary error, can I somehow restart that?

@SniperSister Please take a look on this dron failure as this is related to rips.

Remove RTC if you want new tests, please. I've never looked into this pr because of the RTC label.
And it's definitely 3.10 now we have to test with? Because the pr says "wants to merge 2 commits into joomla:staging" (=3.9 atm)
Thaaank you!

And it's definitely 3.10 now we have to test with? Because the pr says "wants to merge 2 commits into joomla:staging" (=3.9 atm)

Testing against staging is fine unless this needs code that's merged only to the 3.10 branch. Otherwise, someone can deal with rebasing branches when merging.

@wojsmol fixed

Well, the RTC label was added after two successful tests last year in October... Afterwards, additional reviewing was done (with some feedback regarding styling/accessibility), and a rebase was needed due to conflicts. I think this is why the milestone then was changed to 3.10 in May (as those points haven't been taken care of), and it's the reason why I would like people to re-test this.

Is there any chance that we still could get this into 3.9?

If not, we should remove RTC label and start the whole procedure from scratch again.

If yes, I think we should give this high priority and merge it into staging soon, as the base-code has 2 successful tests (the base-code has not been changed since, it was only minor styling/formatting stuff), and as the PR works and has no major bugs or regressions; at least from my point of view everything works.

If folks will test it out this week I've got no issue merging it (technically beta is supposed to mean feature freeze but I've always given a little flexibility to that if something is "close" (arbitrary personal measurement)). But let's have it merged by the time beta 3 goes out next week if that's going to happen.

Thanks @mbabker I’ll get it tested tonight

On Sep 17, 2018 at 2:59 pm, <Michael Babker (mailto:notifications@github.com)> wrote:

If folks will test it out this week I've got no issue merging it (technically beta is supposed to mean feature freeze but I've always given a little flexibility to that if something is "close" (arbitrary personal measurement)). But let's have it merged by the time beta 3 goes out next week if that's going to happen.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub (#17552 (comment)), or mute the thread (https://github.com/notifications/unsubscribe-auth/ABVglmnb0PSdx9Wx6rSEO_nWgcj40jVHks5ub6qpgaJpZM4O37bl).

I have been using this patch for months using patch tester and it has been working fine for me (not beatiful design, but with no bugs). I'll test it tomorrow with the new changes.

I have tested this item 🔴 unsuccessfully on 2bb31c0

Reason will follow on GitHub #17552

I have tested this item 🔴 unsuccessfully on 2bb31c0

Reason will follow on GitHub #17552

Joomla 3.9 nightly.

I installed a module that has already a subform like this in module manifest:

<fieldset name="addImg" label="FIELDSET_IMAGES" description="IMAGES_DESC">

	<field name="images" type="subform" min="1" max="100" multiple="true"
		formsource="modules/mod_flexsliderghsvs/myforms/subform.xml" 
		layout="joomla.form.field.subform.repeatable" groupByFieldset="false"
		label="Add images"
		description="" />

</fieldset>

simplified formsource modules/mod_flexsliderghsvs/myforms/subform.xml

<?xml version="1.0" encoding="UTF-8"?>
<form>
 <field name="foto" type="media" label="Photo" description="" default="" preview="true" />
</form>

Module works as expected. Form looks OK.

Then I installed the patch 17552 via PatchTester. Nothing more!!

Afterwards my old form has 2 Add buttons. The bigger one is OK but doesn't work. The smaller one comes from patch and works.

When I change
layout="joomla.form.field.subform.repeatable-table"
to
layout="joomla.form.field.subform.repeatable"
of example code of Testing Instructions that form has 2 buttons, too.

Remove RTC

Remove RTC

@ReLater Thank you for testing, bug is fixed now!

I have tested this item ✅ successfully on cdbe0c9

Working well

I modified a module, added a form to /forms/ then a subsub form to forms/

Included and run before patch, repeating field goes crazy.

Include and run after patch, forms are respectable, + and x toggles work as expected for each form.

Tested saving, saves as expected and reloads with the data passed into the form.

@ReLater can you re-test this? If you give your GO we have two successful test 🙂 😊

I have tested this item ✅ successfully on cdbe0c9

I have successfully tested this patch in Joomla! 3.8.12 and 3.9 beta with Patch tester.
I have successfully tested this path directly from the commit cbe0c9 code.

Test performed:

I have added this field to one of my components configuration:

<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"
		/>
	<field
		name="subformlevel3"
		type="subform"
		label="PLG_FIELDS_CHECKBOXES_PARAMS_OPTIONS_LABEL"
		description="PLG_FIELDS_CHECKBOXES_PARAMS_OPTIONS_DESC"
		layout="joomla.form.field.subform.repeatable"
		icon="list"
		multiple="true"
	>
	<form hidden="true" name="third_level" repeat="true">
	<field
		name="name_lvl3"
		type="text"
		label="PLG_FIELDS_CHECKBOXES_PARAMS_OPTIONS_NAME_LABEL"
		size="30"
		/>
	</form>
	</field>
    </form>
</field>

and in all scenarios the patch has worked with no issues.

Ready to Commit after two successful tests.

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

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...

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

Resolution ie needs a poly fill for template

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

On Thu, 1 Aug 2019 at 13:01, dawe78 notifications@github.com 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
https://github.com/joomla/joomla-cms/pull/17552?email_source=notifications&email_token=AAKWBFUREHHNLS65EYBCEA3QCLGAPA5CNFSM4DW7W3S2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3KLCAQ#issuecomment-517255426,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAKWBFSPVXUJNPWIXHMILL3QCLGAPANCNFSM4DW7W3SQ
.

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...

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...

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

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

@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?

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???

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???

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)

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

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

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:

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
}