? ? ? Pending

User tests: Successful: Unsuccessful:

avatar Hackwar
Hackwar
11 Mar 2020

Pull Request for Issue #23659.

Summary of Changes

We have the subform field and don't need the repeatable field anymore. This removes that field as requested in #23659 and previously done in continga@71f831d.

This PR removes the repeatable field and adds a method to the update script which converts the existing repeatable fields to the subform field type. For all repeatable fields it inserts new fields and links those to the subfields field. To not display them twice in the edit view, the category for the new fields is always set to "none". Please notice that a field in the repeatable field of type "number" can NOT be converted, since we don't have a field of type number. You can however change this later on to integer for example.

Testing Instructions

  1. Setup at least one repeatable field with at least one field.
  2. Copy /plugins/fields/repeatable to a temporary folder.
  3. Apply this PR and then copy back the above folder again.
  4. Call the uninstallRepeatableFieldsPlugin() method from /administrator/components/com_admin/script.php. This can be done for example by using this code:
    public function update()
    {
    	require_once JPATH_ADMINISTRATOR . '/components/com_admin/script.php';
    	\JoomlaInstallerScript::uninstallRepeatableFieldsPlugin();
    }
    Add this to for example /administrator/components/com_content/src/Controller/ArticleController.php in line 53 and then call /administrator/index.php?option=com_content&task=article.update Before you do that, you need to modify the the access modifier of that method to public. So edit /administrator/components/com_admin/script.php line 235 from protected to public.
  5. Go to the fields view of the component where you have the repeatable field and see that it has been converted to subfields.
avatar Hackwar Hackwar - open - 11 Mar 2020
avatar Hackwar Hackwar - change - 11 Mar 2020
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 11 Mar 2020
Category Administration com_admin Language & Strings SQL Installation Postgresql Front End Plugins
avatar Hackwar Hackwar - change - 11 Mar 2020
Labels Added: ? ?
avatar Hackwar Hackwar - change - 12 Mar 2020
Labels Added: ?
avatar wilsonge wilsonge - change - 12 Mar 2020
Priority Medium Urgent
avatar Hackwar Hackwar - change - 12 Mar 2020
Labels Added: ?
Removed: ?
avatar Hackwar Hackwar - change - 12 Mar 2020
The description was changed
avatar Hackwar Hackwar - edited - 12 Mar 2020
avatar Hackwar Hackwar - change - 12 Mar 2020
Labels Added: ?
Removed: ?
avatar AndySDH
AndySDH - comment - 12 Mar 2020

Thank you for picking this up! @Hackwar

avatar chmst
chmst - comment - 13 Mar 2020

The steps 2 and 3 of the testing instruction are only needed if the patch is applied via patchtester.

The PR works fine for me. But there is a small problem.
I had two repeatable fields, customer and supplier, both with name and address, some as text, some as textarea.

After running the update script I have customer and supplier as subfields. This is correct..
And I have 2 fields "name" and 2 fields "address". This also is correct. But confusing.

A possible solution could be to set the name of the repeatable as a prefix i.e. "customer-name".

There are some problems with UX, but not in scope of this PR.

avatar continga
continga - comment - 14 Mar 2020

Thank you for picking this up for J4. Due to circumstances, I was not able to do this yet. Thanks!

avatar drmenzelit
drmenzelit - comment - 18 Mar 2020

When I try to apply the patch through the patch tester I get an error:
The patch could not be applied because it conflicts with a previously applied patch: installation/sql/mysql/joomla.sql

But I have not other patches applied... what can I do?


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28319.

avatar brianteeman
brianteeman - comment - 18 Mar 2020

@drmenzelit reset patchtester

avatar drmenzelit
drmenzelit - comment - 18 Mar 2020

Thank you @brianteeman ! It worked now.

The PR works. But the same as @chmst wrote: the repeatable was converted to subfields, but two new text fields (the two I created inside the repeatable) appear outside the subfield. I don't know if this is an expected behavior...


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28319.
avatar Hackwar
Hackwar - comment - 18 Mar 2020

@drmenzelit yes, unfortunately that is expected behavior.

avatar chmst
chmst - comment - 18 Mar 2020

So I will mark your PR as good. But it is the contrary of UX or self explaining. I will try to identify the UX issues.

avatar chmst
chmst - comment - 18 Mar 2020

I have tested this item successfully on 7a90f96


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28319.

avatar chmst chmst - test_item - 18 Mar 2020 - Tested successfully
avatar drmenzelit
drmenzelit - comment - 18 Mar 2020

I have tested this item successfully on 7a90f96


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28319.

avatar drmenzelit drmenzelit - test_item - 18 Mar 2020 - Tested successfully
avatar AndySDH
AndySDH - comment - 18 Mar 2020

@drmenzelit yes, unfortunately that is expected behavior.

By the way that's not "unfortunately". That's a good thing. That's a good feature, it means you can re-use the same fields as part of different subfields :)

avatar chmst
chmst - comment - 18 Mar 2020

@AndySDH yes, it is a good feature. But the subfields feature is not self explaining. For example in the field list, you cannot see which field is sub field only or in which subfields it is used.
The raping of the categoy for indicating "this is subfield only" is not self explaining too. So there should be some further improvements for the subfields feature but this is not in scope of this PR

avatar AndySDH
AndySDH - comment - 18 Mar 2020

Yes, that's true. It could definitely be useful to mark in some better way which subfields are only meant to be used as subfields. For now what I did is put them in a dedicated fieldgroup.

But the good thing about this, is that you can even use a field in two ways at the same time: you can both use it as a normal field somewhere, and as a subfield somewhere else!

avatar wilsonge wilsonge - change - 18 Mar 2020
Labels Added: ?
Removed: ?
avatar wilsonge wilsonge - change - 18 Mar 2020
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2020-03-18 14:53:13
Closed_By wilsonge
avatar wilsonge wilsonge - close - 18 Mar 2020
avatar wilsonge wilsonge - merge - 18 Mar 2020
avatar wilsonge
wilsonge - comment - 18 Mar 2020

I'm merging this. I've created a new issue to track the UX of the field given two people independently are finding it confusing #28389 and marked it as a release blocker.

Add a Comment

Login with GitHub to post a comment