Language Change NPM Resource Changed PR-4.0-dev Updates Requested

Pending

User tests: Successful: Unsuccessful:

avatar continga
continga
23 Apr 2019

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

$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? :)

avatar continga continga - open - 23 Apr 2019
avatar continga continga - change - 23 Apr 2019
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 23 Apr 2019
Category SQL Administration com_admin Postgresql com_fields Language & Strings JavaScript Repository NPM Change Front End Installation Libraries Plugins
avatar continga continga - change - 24 Apr 2019
Labels Added: Language Change NPM Resource Changed PR-4.0-dev
avatar franz-wohlkoenig franz-wohlkoenig - change - 24 Apr 2019
Title
Fields subfields 4.0
[4.0] Fields subfields
avatar franz-wohlkoenig franz-wohlkoenig - change - 24 Apr 2019
Title
Fields subfields 4.0
[4.0] Fields subfields
avatar franz-wohlkoenig franz-wohlkoenig - edited - 24 Apr 2019
avatar ghazal
ghazal - comment - 24 Apr 2019

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.

avatar AndySDH
AndySDH - comment - 24 Apr 2019

Thanks a lot for your work!

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

image

A few notes about this...

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

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

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

avatar ghazal
ghazal - comment - 24 Apr 2019

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

avatar AndySDH
AndySDH - comment - 24 Apr 2019

Some more stuff:

  1. In article view, the layout doesn't seem to stack on mobile view
  2. 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.

  1. Looks like there is space output around the subfield values, such as:
    image

  2. 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:
    #23659

Don't see anything else for now! :)

avatar ghazal
ghazal - comment - 25 Apr 2019

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.

avatar ghazal
ghazal - comment - 25 Apr 2019

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.

avatar ghazal ghazal - test_item - 25 Apr 2019 - Tested successfully
avatar sanderpotjer
sanderpotjer - comment - 4 May 2019

@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

avatar HLeithner
HLeithner - comment - 13 May 2019

@continga could you please resolve the conflicts?

avatar AndySDH
AndySDH - comment - 19 Aug 2019

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?

avatar continga continga - change - 4 Sep 2019
Labels Added: Updates Requested
avatar continga
continga - comment - 4 Sep 2019

Hi all, I just pushed an update for this which contains merging against 4.0-dev and some smaller bugfixes. Can y'all take a look again and report any last stuff? I am pretty sure we can somehow get this merged soon then yet.

PS: I don't know why the drone build is failing, looks like a CI failure and not something I have done wrong?

Note for testing: Currently, the selection of the sub fields stretches wide to the right, effectively overlaying with the content box to the right, hence not being able to use the add/remove/reorder buttons of the subform, see this screenshot:

image

This is being caused by the description of the top-row being way too long, and until we decide on a solution for this (maybe somehow just hide the description there, or implement this hotfix) there is a hotfix available to be able to use the page: Right-click on the "Render values" table header -> "Inspect element", remove the CSS rule "white-space: nowrap;", see screenshot:

image

Then the layout looks better, not completely OK yet, but it is useable:

image

avatar continga
continga - comment - 4 Sep 2019

@AndySDH I am not sure whether I understand your concern incorrectly, or whether you're wrong.

$row_output is an array containing the output of the subfields for one single row. So $row_output is the output for a single row. That is exactly what the variable name says. It is not the output of a single subfield inside a row - that is just what an array member of $row_output is.

Same for $result: It contains the overall result for all rows, not the output of a row. I also here think the name is correct.

avatar AndySDH
AndySDH - comment - 8 Sep 2019

@AndySDH I am not sure whether I understand your concern incorrectly, or whether you're wrong.

$row_output is an array containing the output of the subfields for one single row. So $row_output is the output for a single row. That is exactly what the variable name says. It is not the output of a single subfield inside a row - that is just what an array member of $row_output is.

Same for $result: It contains the overall result for all rows, not the output of a row. I also here think the name is correct.

Yup, you are right. I was thinking individual elements of the array instead of the whole array. It's actually good as it is :)

avatar AndySDH
AndySDH - comment - 8 Sep 2019

So... I tested this with the latest J4 Alpha dev 12. The core backend template layout is still a complete mess so I'm not sure I'm able to judge this accurately when it comes to appearance.

Functionality wise:

  • Still an issue from before: 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 not be stored when empty.

  • The "Repeatable" option currently can be changed after a subfield already exists. We figured it should not be possible to change it due to it messing up the values when changed.

I'll list what I can when it comes to appearance:

  • As you already mentioned, it makes no sense for there being a nowrap CSS rule for th, it's only going to mess things up, so that should definitely be fixed in the Joomla template.

  • Still an issue from before: When creating a subfield, 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 once at the top in the thead.

  • Looks like now the Joomla template displays input fields side-by-side instead of stacking. In article edit form, the subfields type can't really coexist side by side with anything, as it's a table display in the first place, so its control-group should always be forced to a new line and width 100%. See below the current situation:

terrible

But again, the whole Joomla Backend template is pretty terrible right now so idk.

  • Still an issue from before: In article edit form, the layout doesn't stack on mobile view

Add a Comment

Login with GitHub to post a comment