? Pending

User tests: Successful: Unsuccessful:

avatar okonomiyaki3000
okonomiyaki3000
19 Jan 2016

The 'repeatable' field type has a problem when used in a config form. Because the repeatable type has an extra 'fieldset' inside the the main 'field' tag and because JForm::getFieldsets does not exclude these specially nested 'fieldsets' and because this particular form's template doesn't use the joomla.edit.params layout (which includes a special exemption for fieldset tags with the repeatable attribute set to 'true'), when you place a 'repeatable' in a config form, you will get an extra tab that you didn't want. You can test this by inserting a 'repeatable' field in any config form. For example:

<field name="test" type="repeatable" label="Test" default="{'test':['']}">
    <fieldset name="test_modal" hidden="true" repeat="true">
        <field name="test" type="text" label="Test" />
    </fieldset>
</field>

paste this code in some appropriate location inside of any component's config.xml and then go to the administrator and look at the form. There should be an extra tab with nothing in it. This is the tab for the 'test_modal' fieldset. Apply this patch and it will go away.

This patch is kind of a quick fix for this issue. It solves the problem and shouldn't have any side-effects. In the longer term (because other things may be affected) more changes would be nice, including:

  • The config form should be changed to make use of the joomla.edit.params layout.
  • JForm::getFieldsets should (at least optionally) omit 'fieldset' tags nested within 'field' tags. This breaks BC but it was a kind of break of BC when 'fieldsets' began to be nested within 'field' tags in the first place because that was certainly not the expectation when JForm::getFieldsets was written.
  • Possibly rethink the 'repeatable' field type. Although it serves a valuable purpose, almost everything about it is a mess. The xml to set it up is not so easy to remember, the layout of the fields on the rendered form is a bit limiting, the fact that you must 'save' the modal and then remember to 'save' the actual form can be confusing, using the stored values of the sub-fields is not as easy as it should be... Well, it's probably too late to do anything about any of that anyway.
avatar okonomiyaki3000 okonomiyaki3000 - open - 19 Jan 2016
avatar okonomiyaki3000 okonomiyaki3000 - change - 19 Jan 2016
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 19 Jan 2016
Labels Added: ?
avatar photodude
photodude - comment - 19 Jan 2016

Anything done with repeatable field to fix it's problems is likely to break BC. The repeatable field type is totally broken for anything more than very simple fields like a text box. Once JS logic comes into play all bets are off.
(I'm still hoping for repeatable field type to be fixed some day) #6882

avatar okonomiyaki3000
okonomiyaki3000 - comment - 19 Jan 2016

@photodude I agree. It might be best start over with a completely different approach to repeatable fields. Come up with a new type that works better, gradually migrate to it, and deprecate repeatable at some point in the far future. But, for now, I'd be satisfied with a fix to this little problem.

avatar photodude
photodude - comment - 19 Jan 2016

There is a PR attempting to do Just that and start over, #7829

avatar okonomiyaki3000
okonomiyaki3000 - comment - 19 Jan 2016

Good, I hope it turns out well. Really though, the whole issue illustrates the mess of doing this stuff the 'old fashioned' way. I'd really like to see the whole admin (and site too for that matter) remade with all rendering done on the front end using Angular, React, or something similar. Doing complex forms (and just about everything else) that way is laughably trivial compared with server-side rendering.

avatar brianteeman brianteeman - change - 10 Mar 2016
Category Fields
avatar okonomiyaki3000
okonomiyaki3000 - comment - 10 May 2016

There's actually no conflict with this. It can surely be merged.

avatar okonomiyaki3000
okonomiyaki3000 - comment - 5 Jul 2016

This seems to be solved already.

avatar okonomiyaki3000 okonomiyaki3000 - change - 5 Jul 2016
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2016-07-05 09:22:33
Closed_By okonomiyaki3000
avatar okonomiyaki3000 okonomiyaki3000 - close - 5 Jul 2016

Add a Comment

Login with GitHub to post a comment