[4.0] Fields subfields by continga · Pull Request #24711 · joomla/joomla-cms

continga · 2019-04-23T23:55:44Z

Summary of Changes

This is a new version of PR #22446 . We had a long discussion in #22446 about the feature of a subfields custom field, and after some feedback-cycles we came up with a good solution.

Sadly, that PR couldn't make it into the 3.x-branch, hence we need to rebase the whole PR and create a new one against 4.0-dev. This is the purpose of this PR. It will implement the functionality from #22446 into the 4.0-dev branch.

Testing Instructions

See #22446 - basically what needs to be tested is the new "Subfields" custom field type, which can contain other custom field instances. It is basically a collection of other custom fields.

We need to check both the correct behaviour in the backend (entering custom field data, editing custom fields), as well as the default-display in the frontend (outputting all kind of different subfields+fields values, etc). See also further down at the docs-link.

Expected result

See #22446

Documentation Changes Required

Yes. I have started to work on it a bit, see https://docs.joomla.org/Custom_fields_type:_Subfields

I will update that docs-page over the coming weeks, it is currently not reflecting the very latest changes. But it is already giving a good quick overview about the feature, and possible cases to test.

Additional information

I am very open for general code feedback, improvement ideas, test cases etc. I am not completely sure whether I followed all J4 coding-guidelines, so I would be glad to get some feedback and correct something - in case it's needed.

Generally, IMHO the feature is already very complex at the current state, so we should take distance from making it even more powerful. It makes more sense to get the current functionality merged, and then working on more advanced matters afterwards.

Also, it seems like choosing a category for a custom field currently seems broken in 4.0-dev generally, not only in this branch? Seems to be caused by an exception being thrown at

joomla-cms/administrator/components/com_fields/Model/FieldModel.php

Line 989 in e2abc09

$cat = $componentObject->getCategory([], $section ?: '');

This needs to be investigated, but it ain't part of this PR. Just dropping this note here, because I think testers will stumble accross not being able to set a category for a custom field.

Summoning @regularlabs @AndySDH - you guys did a great job at testing the previous PR, maybe you could take a look at this one? :)

…, squashed. This already contains adjustments for Joomla! 4, e.g. changed class files / paths, etc. This does not yet contain any tests made or bugfixes against the 4.0 branch. This will be done after this commit and possible bugfixes will be made in standalone commits. So this basically only contains the code from joomla#22446, adjusted to the 4.0 directory structure.

…r our last commit.

…-4.0

…t variant.

administrator/components/com_fields/tmpl/fields/default.php

ghazal · 2019-04-24T10:54:48Z

Tested but "Sub field *" dropdown doesn't show.

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

AndySDH · 2019-04-24T11:30:36Z

Thanks a lot for your work!

I'll post things as I find them... :)

A few notes about this...

Looks like Joomla 4 switched the true/false stuff to a toggle? See the "Required" option.
So I'm guessing to keep it consistent, all the Yes/No stuff you have ("Repeatable" option and "Render values" options) should be changed to the same way of doing things? Not sure.
Looks like the option descriptions for "Sub field" and "Render values get repeated for every row instead of just being at the top for some reason. So this unnecessary and should be probably removed and only displayed at the top.
As it can obviously be seen, those long options descriptions break the layout and they don't wrap to new lines. But this is probably a general issue that has nothing to do with this PR. (such as the one you already mentioned for not being able to select a Category)

ghazal · 2019-04-24T11:47:57Z

Sorry. After @AndySDH post, I understood my mistake.
Dropdown works allright.

plugins/fields/subfields/params/subfields.xml

AndySDH · 2019-04-24T13:41:41Z

Some more stuff:

In article view, the layout doesn't seem to stack on mobile view
This issue is back:

Issue with a calendar sub field: First filling (and saving) and then emptying a calendar sub field, makes it still store in the database value as "0000-00-00 00:00:00", and outputs it as "-0001-11-30". Instead if should obviously not be stored when empty.

Looks like there is space output around the subfield values, such as:
So as we know, Repeatable Field needs to be gone in Joomla 4.0. So just a reminder that something will have to be done with this:
[4.0] Remove plg_fields_repeatable #23659

Don't see anything else for now! :)

ghazal · 2019-04-25T09:27:17Z

I have tested this item ✅ successfully on 174a97a

I really hope this will be implemented.

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

sanderpotjer · 2019-05-04T12:23:48Z

@continga great work, thanks! Besides the feedback already shared by others i found an issue with the media field: The Media field is trowing JavaScript error if the article with field is not saved yet. Uncaught TypeError: this.querySelector(...).open is not a function, so the media model is not opening

HLeithner · 2019-05-13T07:39:45Z

@continga could you please resolve the conflicts?

…-4.0

AndySDH · 2019-08-19T16:54:20Z

Any update @continga? Just making sure we don't forget this for Joomla 4! If @continga is not available, can someone else fix the conflicts?

…-4.0

continga · 2019-10-11T11:18:29Z

Ok, how do we continue with this PR? I feel like the collective effort to get this merged is not very high, which I think is kind of sad after having received so much positive feedback ealier and after having spent so much work into this.

What we should do, from my point of view: We merge this into 4.0-dev now. The codebase for this PR is over one year old (see #22446), and no major issues have been found for a long time, and we had many different tests from different people in that timeframe. There are maybe 2 minor issues present at this time, and I will very gladly fix them after this got merged, because then I know my work is not being a pure waste of time. Additionally, we then get enough time to test this together with the whole community and react on possible problems.

The other possibility would be to just close this PR, as we would never get this merged otherwise. Joomla 4 Beta is about to sail away this month, and if we don't get this into beta we can just close it already.

What do we do?

HLeithner · 2019-10-11T15:14:11Z

iirc #23659 is missing for j4 incl. migration of old fields. ymmv

continga · 2019-10-11T15:23:53Z

It is not "missing" in the sense of "it is required to be done" - it just makes sense to remove the repeatable type when this PR got merged, because the new type can do the same as the (then obsolete) repeatable type.

I will happily work on #23659 incl. a migration when we got this PR merged.

AndySDH · 2019-10-12T10:05:21Z

Yes, this absolutely needs to be merged asap! @wilsonge

wilsonge · 2019-10-13T11:09:03Z

Can we get a test of this done fully please. And I'm happy to get this merged before we tag the next alpha on Thursday and I'll rely on you keeping to your word on the upgrade script (which is essential)

AndySDH · 2019-10-13T11:14:12Z

Test was fully done, only minor things left to address are in my previous comment on September 8.

wilsonge · 2019-10-13T11:15:57Z

OK. Then I'm happy to commit. @continga please ensure you at least sort the migration script and the calendar null date issue

richard67 · 2019-10-13T14:43:32Z

@wilsonge

@continga please ensure you at least sort the migration script and the calendar null date issue

If you mean the null date usage in the insert statement for the extensions table in the joomla.sql files and the update sql files: This will be fixed with my PR #26491 . I've updated it so it contains the changes from this PR here and then corrects the nulldates for the checked_out_time columns to real null values like for all other extensions.

Quy · 2019-10-18T03:15:26Z

@richard67 Null date check here: https://github.com/joomla/joomla-cms/pull/24711/files#diff-60b1fc5d2dc0bc1d69d7c3caa257e881R1058

richard67 · 2019-10-18T12:54:52Z

@Quy Could be maybe ok because on js side, will possibily be set to the right value when writing to db in php. I will check later today or tomorrow.

Quy · 2019-10-18T22:27:35Z

@continga Invalid argument supplied for foreach() in \libraries\src\Form\FormField.php on line 1124

To reproduce:

Create a subform field
Edit an article (leave subform field empty)
Save the article
Check PHP error log

AndySDH · 2019-11-11T22:08:50Z

@continga Woud be good if you could add an option for Repeatable Subfields to select a maximum number of rows that can be added (how many times it can be repeated).

AndySDH · 2019-11-30T20:54:35Z

@continga Remember to look after any possible tweak, fixes, and the implementation of the migration script happening. The PR has been merged like you requested, now please don't just forget about it :P

micker · 2019-12-04T22:04:43Z

hello 2 questions about migration of repetable field to subfield
did you have some information about better migration ?
did you think we can have subfield for j3 to prepare a smother migration ?

AndySDH · 2019-12-05T09:01:24Z

Well, a PR was ready for 3.9, but it got asked to be rebased for 4.0
See: #22446

Rene Pasing added 5 commits April 15, 2019 19:55

Fixed immediate errors which preventing Joomla!4 to be installed afte…

e3edd88

…r our last commit.

Fix some syntax errors / deprecated stuff after switching to J4.

a6ec299

Merge remote-tracking branch 'upstream/4.0-dev' into fields-subfields…

9e22218

…-4.0

Adapt installation of plg_fields_subfields plugin to new autoincremen…

df69dac

…t variant.

joomla-cms-bot added Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Apr 23, 2019

continga mentioned this pull request Apr 23, 2019

Add 'subfields' custom field type #22446

Closed

Make phpcs happy.

174a97a

Quy reviewed Apr 24, 2019

View reviewed changes

administrator/components/com_fields/tmpl/fields/default.php Outdated Show resolved Hide resolved

ghost changed the title ~~Fields subfields 4.0~~ [4.0] Fields subfields Apr 24, 2019

Quy reviewed Apr 24, 2019

View reviewed changes

plugins/fields/subfields/params/subfields.xml Outdated Show resolved Hide resolved

sanderpotjer mentioned this pull request May 4, 2019

[4.0][com_fields] Category select is missing/broken for fields #24782

Closed

HLeithner mentioned this pull request May 13, 2019

Make squeezebox initialize itself when a new subform row is added #20232

Merged

Rene Pasing added 4 commits May 15, 2019 01:13

Merge remote-tracking branch 'upstream/4.0-dev' into fields-subfields…

5fc39fa

…-4.0

Add formatting feedback from PR

c90d8e2

Remove unneeded descriptions for fields

3a09074

Change radio buttons to use class switcher

71b98b2

This was referenced May 28, 2019

(Subform)Repeatable editor blank after drag&drop reorder #24343

Closed

[4.0] Styling the subform repeatable field #20720

Closed

Quy added the Updates Requested Indicates that this pull request needs an update from the author and should not be tested. label Jul 14, 2019

Merge remote-tracking branch 'upstream/4.0-dev' into fields-subfields…

502d650

…-4.0

Merge branch '4.0-dev' into fields-subfields-4.0

7535c94

wilsonge merged commit 87f590c into joomla:4.0-dev Oct 13, 2019

wilsonge added this to the Joomla 4.0 milestone Oct 13, 2019

wilsonge added Documentation Required and removed Updates Requested Indicates that this pull request needs an update from the author and should not be tested. labels Oct 13, 2019

Quy mentioned this pull request Oct 15, 2019

[4.0] create/edit subfields size does not fit #26602

Closed

AndySDH mentioned this pull request Oct 18, 2019

[4.0] Subfields - refactor for accessibility etc #26614

Merged

wilsonge mentioned this pull request Nov 11, 2019

[4.0] Migration of repeatable field custom field to subform #27047

Closed

AndySDH mentioned this pull request Nov 30, 2019

Add Url fieldtype to Repeatable Field #27167

Closed

Fedik mentioned this pull request Feb 1, 2020

[4.0] Subfield - Param repeat should be readonly #27751

Closed

AndySDH mentioned this pull request May 10, 2020

[3.9.1] [com_fields] Make sure disabled fields are not added to the request at all #22923

Merged

This was referenced Sep 27, 2021

[4.0] Subforms - How to get right data-base-name #35677

Closed

Subform: fix incorrect data-base-name, do not set empty fieldname #35685

Merged

bembelimen mentioned this pull request Jan 1, 2023

[4.3] Remove renaming of fieldname in subform #39536

Closed

4 tasks

Uh oh!

Conversation

continga commented Apr 23, 2019

Summary of Changes

Testing Instructions

Expected result

Documentation Changes Required

Additional information

Uh oh!

Uh oh!

ghazal commented Apr 24, 2019

Uh oh!

AndySDH commented Apr 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghazal commented Apr 24, 2019

Uh oh!

Uh oh!

AndySDH commented Apr 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghazal commented Apr 25, 2019

Uh oh!

sanderpotjer commented May 4, 2019

Uh oh!

HLeithner commented May 13, 2019

Uh oh!

AndySDH commented Aug 19, 2019

Uh oh!

continga commented Oct 11, 2019

Uh oh!

HLeithner commented Oct 11, 2019

Uh oh!

continga commented Oct 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AndySDH commented Oct 12, 2019

Uh oh!

wilsonge commented Oct 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AndySDH commented Oct 13, 2019

Uh oh!

wilsonge commented Oct 13, 2019

Uh oh!

richard67 commented Oct 13, 2019

Uh oh!

Quy commented Oct 18, 2019

Uh oh!

richard67 commented Oct 18, 2019

Uh oh!

Quy commented Oct 18, 2019

Uh oh!

AndySDH commented Nov 11, 2019

Uh oh!

AndySDH commented Nov 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

micker commented Dec 4, 2019

Uh oh!

AndySDH commented Dec 5, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

AndySDH commented Apr 24, 2019 •

edited

Loading

AndySDH commented Apr 24, 2019 •

edited

Loading

continga commented Oct 11, 2019 •

edited

Loading

wilsonge commented Oct 13, 2019 •

edited

Loading

AndySDH commented Nov 30, 2019 •

edited

Loading