? ? ? Success

User tests: Successful: Unsuccessful:

avatar continga
continga
1 Oct 2018

Summary of Changes

This pull request adds the possibility to create custom fields of a new type subfields.

Before this PR, it was only possible to add "basic" types as custom fields to articles, contacts etc, whereas "basic" types would be "text", "media", "integer", etc.

This PR adds the possibility to add a subfields custom field to an article, contact etc., whereby the idea behind subfields is that a user can create a list of sub fields that his custom field should contain, and those sub fields will then be presented to the user in the backend as a possibly repeatable list.

Testing Instructions

Install the patch, go in the backend to e.g. Content -> Fields -> New, select Type=Subfields, set Title=test123, under Fields add a simple Text field with name and label test1, and another simple Text field with name and label test2. Save & close the custom field.

Go to Content -> Articles -> New, switch to tab Fields, and check whether the new subfields custom field is working as it should (so displaying a repeatable version of the configured fields). Insert an Article Title and something in the custom field, and save & close the article. Afterwards go to the frontend and check whether the article and its content is being displayed correctly.

Expected result

Custom field type subfields can be edited correctly, its content can be added correctly in the backend when editing/creating an article/contact/etc, and the custom field values will be correctly displayed in the frontend.

Documentation Changes Required

I have written a small documentation which mainly targets developers and describes the needed actions to make a layout override for the rendering of the subfields value. This documentation can be enhanced by everyone interested to describe the main functionality. https://docs.joomla.org/Custom_fields_type:_Subfields

Additional information

This PR is based on #17552

I created this PR against the 3.10-dev branch, also because I would like to have enough time to work on possible feedback coming in (not against staging, but directly against 3.10). But can we all please work together so this really ends up in 3.10, and also in 4.0? I am very willing to put much work into this, but I am dependent on you guys' feedback. So please, tell me! ?

avatar continga continga - open - 1 Oct 2018
avatar continga continga - change - 1 Oct 2018
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 1 Oct 2018
Category SQL Administration com_admin Postgresql MS SQL com_fields Language & Strings Installation Front End Plugins
avatar continga continga - change - 1 Oct 2018
Labels Added: ? ?
avatar laoneo
laoneo - comment - 3 Oct 2018

What is the difference to #20243? For me it looks like it has a similar functionality.

avatar stutteringp0et
stutteringp0et - comment - 11 Oct 2018

Any chance I could convince you guys to rename this? My subform field plugin has been in the JED since Apr 26 2018 and will not be installable when this is merged.

avatar brianteeman
brianteeman - comment - 11 Oct 2018

@stutteringp0et The concept of adding a subform goes back much further than this pr see #17552

avatar mbabker
mbabker - comment - 11 Oct 2018

I hate to sound like I'm coming across as a jerk here too, but "subform" is what we call it in core and as annoying as it has to be for you it'd be equally as awkward to have a custom fields plugin in core named something completely different to avoid the naming collision with your plugin.

avatar stutteringp0et
stutteringp0et - comment - 11 Oct 2018

The second guy to cross the finish line doesn't get the first place trophy. They talked about it in #17552, the difference is that I actually did it.

core-subform maybe? I worked on mine for a long time too and it's not like I can get the JED to rename my extension, I have to re-submit. My users will probably not want to give up features to use the core version.

avatar brianteeman
brianteeman - comment - 11 Oct 2018

Well that's what happens when you work on something for yourself instead of contributing to help everyone benefit - sorry you're not going to get any sympathy from me

avatar stutteringp0et
stutteringp0et - comment - 11 Oct 2018

56 extensions in the JED, more than 80% of them are free, but I'm not helping anyone.

Gotcha

avatar mbabker
mbabker - comment - 11 Oct 2018

The second guy to cross the finish line doesn't get the first place trophy. They talked about it in #17552, the difference is that I actually did it.

By this same logic, it could easily be argued that any feature implemented by an extension couldn't be added to core because it will conflict with or completely disable/destroy an existing extension. Yes, it sucks for those who have those extensions and come into those conflicts. No, I don't think it's appropriate to rename a core feature to some other name to deconflict with an extension that used the same name.

avatar stutteringp0et
stutteringp0et - comment - 11 Oct 2018

I hope you recognize what this attitude does to morale for us lowly 3rd party developers. This isn't the first time, it's the 5th (for me). I have a hard time being enthused about releasing any new extensions in this environment.

My comments on issue #11905 were deleted by the Joomla user, so it's not like there wasn't prior knowledge that my extension existed.

No sympathy? Yes, that's quite clear.

avatar brianteeman
brianteeman - comment - 11 Oct 2018

iirc i deleted the comment because they were an advert for your extension which is not appropriate here (but my memory might be failing) however that was in aug 2018

avatar mbabker
mbabker - comment - 11 Oct 2018

Well, I don't have those email notifications anymore, but if the comment was perceived as an advertisement for an extension then it would be rightfully deleted.

"Subform" is the form field type in core, therefore a plugin adding support for it to custom fields should carry the same name. We should not have to prefix everything with "core-foo" or "joomla-foo". Again, yes, it totally sucks if you are the person who put out an extension with that exact name. But I don't think it's worthy of us renaming things in core to create even more confusion (if someone's on a site with your "Fields - Subform" plugin and someone updates core to get a new "Fields - Core Subform" plugin, how is that not confusing to users?) or making everything in core follow a plg_fields_core_text naming convention.

avatar stutteringp0et
stutteringp0et - comment - 11 Oct 2018

My comment was deleted August 2018, but I made it sometime around March or April in reply to pepperstreet who mentioned me in a comment on 3/30 (I still have the emails).

How confusing is it going to be for a user who upgrades Joomla and finds their subforms no longer work because they've been overwritten by a new core field of the same name? That's the kind of support call I'm going to get.

I seriously doubt you would ever consider naming a core extension com_comprofiler. Beat is a big fish, and I'm just a small fish.

avatar mbabker
mbabker - comment - 11 Oct 2018

The extension name in that case is prefixed/namespaced/whatever-you-want-to-call-it, so using that would be just to spite them. If it were named com_profiler and someone proposed something similar for core, the same issue would arise.

avatar stutteringp0et
stutteringp0et - comment - 11 Oct 2018

By the way, the PR that makes custom field subform possible - I'm the person who did that.

#19025

avatar continga
continga - comment - 14 Oct 2018

Hoi, I pushed an update and included all feedback so far (codestyling & phrasing).

@laoneo Regarding the difference to #20243: #20243 just adds support for custom lists. This PR adds support for all kinds of types of custom fields. This PR is more powerful, you basically can add any arbitrary field type as custom field, of course including lists. So, yeah, the functionality from #20243 is already included in this PR, but not the other way around.

Regarding the naming-discussion: I really do not care how this plugin is named. I would not prefer something like core_subform though, as it really is not clean and can be misleading. But I maybe could find another name, if the community wants it, but it doesn't seem like. @stutteringp0et Can't you rename your plugin and push out an update of your plugin that updates all instances of fields so your plugin keeps working when this is merged? I think it should be possible, shouldn't it?

avatar stutteringp0et
stutteringp0et - comment - 15 Oct 2018

@contiga - I would have to rename the extension, re-submit it to the JED, and then take tech support calls from people who didn't install the new version (which will be most of them). The JED doesn't offer any method to rename an extension - resubmit is the only option.

If I continue to make extensions for submission to the JED, the only safe bet is to give random strings as the extension name. plg_system_64ef367018099de4d4183ffa3bc0848a has a ring to it.

avatar laoneo
laoneo - comment - 15 Oct 2018

@continga then better to extend the existing one instead of having two things which do the same.

@stutteringp0et you can rename an extension in JED, at least I did it once already.

avatar AndySDH
AndySDH - comment - 19 Oct 2018

Hoi, I pushed an update and included all feedback so far (codestyling & phrasing).

@laoneo Regarding the difference to #20243: #20243 just adds support for custom lists. This PR adds support for all kinds of types of custom fields. This PR is more powerful, you basically can add any arbitrary field type as custom field, of course including lists. So, yeah, the functionality from #20243 is already included in this PR, but not the other way around.

@continga All great, but we already have the repeatable field merged and worked on. Work on revamping/improving that, or even replace it completely with your code if you need to. But there's no point in creating a new one that inglobes functionally that repeatable already has, therefore making it obsolete before it even debuts (if you think about that Joomla 3.9 isn't even out yet).

Also see this: #22415 (see my comment below it)

@roland-d

avatar mbabker
mbabker - comment - 19 Oct 2018

I can't keep up anymore.

Wasn't the root "repeatable" form field deprecated in favor of the "subform" field in the first place because of some deep rooted limitations?

So now I have to ask, what exactly is the difference between the repeatable custom field plugin and the subform custom field plugin? Is the repeatable plugin based on the deprecated repeatable field? Or is it misnamed and really intended to be a subform thing?

avatar AndySDH
AndySDH - comment - 19 Oct 2018

I can't keep up anymore.

Wasn't the root "repeatable" form field deprecated in favor of the "subform" field in the first place because of some deep rooted limitations?

So now I have to ask, what exactly is the difference between the repeatable custom field plugin and the subform custom field plugin? Is the repeatable plugin based on the deprecated repeatable field? Or is it misnamed and really intended to be a subform thing?

I believe it's "misnamed" and intended to be a subform thing. It's essentially a subform.

avatar continga
continga - comment - 19 Oct 2018

@mbabker Regarding the difference between this plugin and the repeatable custom field plugin, see #22446 (comment) - I must admit, the word choice ("subform", "repeatable") can be kinda confusing, as it means different things if we talk about custom field plugins, or generally...

However, as @laoneo already pointed out earlier, now that the repeatable custom field plugin is in core, I should re-do this PR and replace/improve that plugin with my code. I will work on it this weekend ☺️

avatar AndySDH
AndySDH - comment - 19 Oct 2018

@continga Awesome!

I personally also think that the "repeatable" name is also wrong. Because the main feature is not that it's repeatable. The main feature is that you can create a field with subfields in it.

The fact that you can repeat it, is the equivalent of the "Multiple" YES/NO in List fields for example.

My suggestion is naming it: "subfields".

And have the option: "Multiple" YES/NO" (like list/radio/checkboxes).
If this is set to NO, you can only enter one "row" of values.
If it's set to YES, you can enter multiple rows.

avatar continga
continga - comment - 19 Oct 2018

image
That is included here ?

Good idea for the naming, sounds good

avatar mbabker
mbabker - comment - 19 Oct 2018

If anything's getting renamed we need to be pretty dang quick about it because once stable ships, that's it.

avatar AndySDH
AndySDH - comment - 19 Oct 2018

@continga Awesome! Looks like you had the same idea. You could rename the "Repeatable" option into "Multiple", to be consistent with all the other Joomla Field Types plugins.

Also you need to style it the same. So a dropdown instead of a radio:

image

@mbabker Yep you are right! I think subfields name could be great, so we stay away from the subform name issue.

Thanks so much for your fantastic work by the way @continga!

avatar AndySDH
AndySDH - comment - 19 Oct 2018

As we were talking about in #22415, a good idea could be to select directly from existing Custom Fields instead of creating new ones as part of this "repeatable/subform/subfields" field type.

  • So first create an actual separate custom field with whatever field type, settings and display options.
  • Then create a subform/subfield and load that into it.

Would make it more easy and also can save some time. Say you need to re-use a custom field in multiple subforms. You create it only once and then you can load it into different subforms/subfields.
What do you think?

avatar Fedik
Fedik - comment - 19 Oct 2018

repeatable/subform/subfields

please stop it ?
do not introduce another confusion, please be kind to future contributors ?

avatar AndySDH
AndySDH - comment - 19 Oct 2018

repeatable/subform/subfields

please stop it ?
do not introduce another confusion, please be kind to future contributors ?

Haha, needed to make it clear what I was referring to until we decide on the final name.

avatar roland-d
roland-d - comment - 19 Oct 2018

Yes, I meant that we select any created custom fields from the custom field list. That way we don't have to rebuild the fields.

avatar AndySDH
AndySDH - comment - 20 Oct 2018

This needs to be changed to 3.9 Milestone by the way, it's urgent because it's about the repeatable field which is going in 3.9.

avatar continga
continga - comment - 21 Oct 2018

Ok guys, I am a bit unsure how to continue with this. Any suggestions are really appreciated.

First of all, I think being able to select from existing custom fields (as suggested by @AndySDH ) would really be nice, but that is definitely not in the scope of this PR. The idea of this PR was to add the possibility to create subform custom fields, nothing more. Being able to select existing custom fields to fill that subform can maybe be an addition in the future, but I do not think this should be part of this PR. Keep it simple for the beginning.

Besides that, as result of the discussions regarding the merged repeatable custom field type, which is currently in staging and will be part of 3.9, and the difference to this PR, I want to state the following: This PR is not going to make it into the 3.9 release. The timeframe is over, we are not going to make at in any way (re @AndySDH suggesting to make this part of 3.9).

So, as this PR is clearly not going to be a part of 3.9, and as we want this to replace the repeatable custom field type in 3.10 then, we need to decide on a plan for that. My suggestion:

  1. Rename the repeatable custom field type plugin in staging to subfields. This could be done before the release of 3.9 already. It is just a naming change, and as @mbabker said, once 3.9 is shipped, changing the name of a plugin later on will be difficult.

  2. Re-do this PR on the base of the updated staging, basically replacing the subfields of 3.9 with this version. This will not only require just to replace it, it will most likely also require some kind of upgrade-script to convert 3.9-subfield instances to 3.10-subfield instances.

What do you guys think? I am totally unsure how we should handle this. Any help would be appreciated.

TL;DR We should've had more communication in the community about this and the repeatable custom field type, and how to get all interests together.

avatar AndySDH
AndySDH - comment - 22 Oct 2018

Hey @continga, I read your reply via email notification, not sure why you then deleted it, but either way... I think we can make it for 3.9. Here is my take on it:

The feature of using existing custom fields can wait 3.10 if that's not possible to do in a timely manner, but I believe the current state of this PR can easily replace the 'repeatable' field already. This PR is essentially a more complete version of the 'repeatable' PR.

It's important that it makes it because as you said, if the current 'repeatable' makes it into 3.9, then it's going to be hard to replace it with a different code, would need a migration script etc, which would make things problematic. So here is what I would do:

  • Delete the 'repeatable' field altogether from 3.9. Eradicate it. Remove everything that PR added.
  • Rename this field in this PR from 'subform' to 'subfields'.
  • Fix that small think about the "Multiple" label I mentioned earlier.
  • Merge this into 3.9.
  • Let's test it in a timely fashion. I'll be testing it thoroughly myself.

Boom we're done!

Then for 3.10 we can look into adding existing fields to it. What do you think?

avatar continga
continga - comment - 22 Oct 2018

What is @mbabker saying to this?

avatar mbabker
mbabker - comment - 24 Oct 2018

I honestly don't use the custom fields stuff, or subform/repeatable, so I honestly have no idea what the issues and concerns are with where things are at right now other than the whole naming quirk or what needs to be done before a stable release.

avatar laoneo
laoneo - comment - 24 Oct 2018

I'v deleted the duplicate comments from @continga which have been created by the outage of Github recently.

avatar laoneo
laoneo - comment - 24 Oct 2018

@AndySDH I guess your proposal can't be implemented in 3.9.0. Because such big changes should not be done in the release candidate phase. Especially as we are close to delivery the stable release.

Probably the best way would be to rename the existing plugin before 3.9.0 goes out to whatever you guys prefer. Then start either redoing this one on top of the existing one or replace the existing one with a migration plan for a next minor release.

avatar Sandra97
Sandra97 - comment - 24 Oct 2018

As a side note, if ever it gets renamed for 3.9.0, all the marketing elements contain 'repeatable custom field' and it's too late to change it on these elements (which are already translated into 25 languages).

avatar pepperstreet
pepperstreet - comment - 26 Oct 2018

Sorry, I am confused. Is this going to replace the 3.9 Repeatable field?
If so, I really wonder why that "limited" field has been merged and advertised so quick and early?!?
It's 4 days until 3.9 is released ;)

My first thought was, o.k. the Repeatable field has a simpler feature set with basic inputs, and the ability to add/remove sub-fields via simple +/- buttons. No additional parameters, nor configuration. No full subforms, due to easier setup. Actually, I was surprised that it appeared so soon in the 3.9 alpha/beta/rc news. Without any real tests or concerns about the obvious "limitations". That's why I tried to report some related issues 1 month ago. But no one seemed to be interested.

Now, I have found some comments and discussion about the "Subform/Subfields" field. Actually basic things that should have been discussed earlier. Not to mention the naming issue with @stutteringp0et Michael Richey's custom field ;) Yes, probably he had to think about a "prefixed name" beforehand... but I don't understand the strange reactions and comments on him. As far as I know, he had also done some mentionable preliminary work on that matter...

I really hope we can bring it to a satisfactory conclusion.
Best regards, Maik.

#22411
#22412

avatar Sandra97
Sandra97 - comment - 26 Oct 2018

This "limited" field has not been merged and advertised so quickly.
This feature has been discussed since last April and added to the 3.9 milestone in May. So IMO there was plenty of time.
It has been 'advertised' as of the Beta release, as a Beta is considered as feature complete according to our development strategy, reason why we don't start promoting any feature before Beta.
Maybe we should just stop doing any marketing during the pre-release cycle? And start working on marketing just 4 days before a stable? Like that, all we will have as promotion would be a blog post, no translated landing page, no imagery for each main features, no press release, no translated video, no help screens ready. At least I would have far less to do for a release ;)

Note that my comment was just to inform that it won't be possible to change all the marketing elements. Nothing else. It's not about saying the change proposal would be good or not, should be applied or not, this is beyond my skills.

avatar joomla-cms-bot joomla-cms-bot - change - 11 Nov 2018
Category SQL Administration com_admin Postgresql MS SQL com_fields Language & Strings Installation Front End Plugins Administration com_admin SQL Postgresql MS SQL com_fields Language & Strings Installation Libraries Front End Plugins
avatar continga
continga - comment - 11 Nov 2018

Just pushed an update, this now contains a first version of replacing the repeatable custom field type with this one.

I still got the renaming on the plate (and some small review stuff) of which I will take care in the next days, but the main functionality is done now.

avatar AndySDH
AndySDH - comment - 11 Nov 2018

Great stuff @continga! Thanks for your work! I will be testing this and report back :)

Also remember this change to make it consistent with the implementation of the "Multiple" option in other Joomla fields.

This:

image

Should be changed in:

You could rename the "Repeatable" option into "Multiple", to be consistent with all the other Joomla Field Types plugins.

Also you need to style it the same. So a dropdown instead of a radio:

image

avatar AndySDH
AndySDH - comment - 11 Nov 2018

I think for the best usability, Render Options should be added for each individual subfield, just like you can add them when creating a normal field.

So the "Render Values YES/NO" option you have, should be moved from being a global setting, to being controlled in each individual subfield.

Clicking on YES on that option, will expand the Render Options for that subfield.

image

This way, for each Subfield you can control:

  • Whether you want to show the Label or not
  • Whether you want to give a CSS class to its rendering
  • Whether you want to use an alternate layout for it, using the built-in Layout option.
avatar Quy
Quy - comment - 12 Nov 2018

For Calendar type, both options are not selected by default for Show Time.

In the source code, the following is repeated for each entry.

<style>
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
.subform-table-sublayout-section .controls { margin-left: 0px }
	</style>
avatar continga
continga - comment - 13 Nov 2018

I just pushed an update which fixes the bugs mentioned in #22446 (comment) and #22446 (comment) and applied the feedback from @Quy

I will think about the feature-proposals from @AndySDH tomorrow

avatar Quy
Quy - comment - 14 Nov 2018

With Checkboxes type on the front end:
Warning: trim() expects parameter 1 to be string, array given in \plugins\fields\subform\subform.php on line 233

avatar continga
continga - comment - 30 Nov 2018

Sorry again for the delay, had some other stuff to do. I just pushed an update containing the rename to plg_fields_subfields and with a bugfix for #22446 (comment)

I thought about the additional feature requests, and as much as I agree with some of them (e.g. controlling the rendering options per individual subfield, or being able to select a subfield based on existing custom fields), I do not think they should be part of this PR. My suggestion is that we merge this PR better sooner than later into 3.10-dev, and afterwards we can continue to work on those additional features in separate issues. The reason is pretty simple, I think this PR already is kind of complex and we should keep it as simple as possible for the beginning. Doing it that way also makes it possible for different people to work on stuff they would like to have in based on this PR.

So, to sum this whole PR up a bit, as far as I know the only point missing is the following:

We need a proper documentation for this. I am currently planning to create a basic article on docs.joomla.org about this PR and plan to do that in the next 7 days. The first version of that article will not be too sophisticated, but I feel like I need to explain some things. I will then also remove the commented code here #22446 (comment) . Afterwards other people could work on improving that wiki article, but that would be outside of the scope of this PR.

As far as I see currently, that is the only thing missing (except that this ofc needs to tested, and if further bugs are being found, those should be fixed... ;-)).

@AndySDH I thought about your suggestion regarding renaming the repeatable attribute of this custom field type to multiple, but I think that repeatable is more correct. This custom field type renders it's subfields as a subform in the backend, and the subform form field type is also talking about "whether the subform fields are repeatable or not."[1], although the relevant attribute in the form XML is called multiple ... Confusing... I don't know. I feel like repeatable is more correct in this case.

[1] = https://docs.joomla.org/Subform_form_field_type

avatar continga continga - change - 30 Nov 2018
Title
Add a 'subform' custom field type
Add 'subfields' custom field type
avatar continga continga - edited - 30 Nov 2018
avatar AndySDH
AndySDH - comment - 30 Nov 2018

@continga Ok, makes sense, I agree, let's get this merged first, and then you can tackle the improvements in additional PRs. I will test now to see if I find any other issues. Thanks for your work!

avatar AndySDH
AndySDH - comment - 30 Nov 2018

Seems like applying patch doesn't uninstall/replace the Repeatable Field, but not sure if the patch can do that.

avatar regularlabs
regularlabs - comment - 30 Nov 2018

There is (still) an issue with how this adds prefixes to field names.
It does not take into account field names starting with a @, like notes might have.
Currently a field with a name="@foobar" will be handled as a normal field and get a name like _type_whatever_@foobar. This also breaks the output.

avatar regularlabs
regularlabs - comment - 30 Nov 2018

Maybe good to test what happens with a less straightforward custom field like:
https://www.regularlabs.com/extensions/articlesfield

avatar mbabker
mbabker - comment - 30 Nov 2018

All I'm going to say is this needs to be tested and reviewed very carefully. Never before has there been an attempt at uninstalling a core extension as part of a release's upgrade, and if the steps added in the update script don't execute correctly this could really mess up a site potentially.

Seems like applying patch doesn't uninstall/replace the Repeatable Field, but not sure if the patch can do that.

Just applying the patch will not automagically do it, to test that you have to perform a core upgrade with a package containing these changes.

avatar AndySDH
AndySDH - comment - 30 Nov 2018

Just a styling suggestion. If you put lots of fields (columns), the styling becomes really difficult to read for obvious reasons.
image

So maybe, you can set it that if there are more than x fields (more than 4? 5?), then default to the mobile layout, which stacks the fields:

image

Which is better. However, looking at the mobile view, I see it has issues as well. There is way too much space between Labels and Values.
Right now the label side is 50% of the width, which is not needed. The admin template for labels only uses 160px so it should be reduced accordingly. Also the layout for checkboxes is kinda bad.
I know it's probably outside the scope of this PR but while you're at it...

avatar AndySDH
AndySDH - comment - 30 Nov 2018

Found another bug: if Render Values is set to "NO", multiple values (list, checkboxes, etc..) are returned as the word "Array" being displayed instead of displaying the values

avatar AndySDH
AndySDH - comment - 22 Dec 2018

@continga No disrespect intended, I know it's all volunteer work... but at this rate we will never get there...

avatar continga
continga - comment - 23 Dec 2018

@AndySDH yes, you are absolutely right. To be honest, I am myself also
very unhappy with the recent progress. I will push this way more after
the holidays, in the new year. Have some nice holidays everybody!

avatar AndySDH
AndySDH - comment - 9 Jan 2019

All I'm going to say is this needs to be tested and reviewed very carefully. Never before has there been an attempt at uninstalling a core extension as part of a release's upgrade, and if the steps added in the update script don't execute correctly this could really mess up a site potentially.

Question regarding this by the way.
Is this a concern just because we're renaming the plugin? Would keeping the plugin being called "repeatable" avoid the problem of having to uninstall it? Therefore this being just an update/revamp of the code of it.
If so, and if it creates less concerns, the repeatable name can easily stay.
Yeah, 'subfields' is 'prettier' and more logic, but 'repeatable' as a name is fine enough too, and isn't (and shouldn't) be a bother. Also maybe worth keeping that name as people have already become familiar with it by now, since it has been out for a few months now. @continga

avatar continga
continga - comment - 14 Jan 2019

Hi, I pushed an update which addresses the last remarks.

@regularlabs can you maybe explain what you mean with the prefix with @ thingie, so provide a short example what the problem is, and how it should be instead? Never heard about prefixing fields with an @ - whats the use of that?

I also created a wiki page with a short documentation, especially regarding layout overrides as I think this will concern many people, see https://docs.joomla.org/Custom_fields_type:_Subfields
Unfortunately, my newly created wiki account continga got blacklisted or something by a spam bot, because I tried to do small edits of that page (fixing typos, creating a note that it is a feature which is not merged yet, etc) and the bot thought I would spam or something. How can I get my account unbanned and on a whitelist for further edits, who do I need to contact?

Regarding the "very careful"-naming thing: I don't know, and I also have no meaning about that. Well, at least deleting the old repeatable plugin is not a problem from my point of view, the problem is the transitioning-code which transforms old repeatable-fields into new subfields-fields. And that code will stay, regardless whether we name this plugin repeatable or subfields. So the problem will also stay. So I think we can leave the name, we really just need to make sure the transitioning is working smoothly in all cases, maybe even by testing what happens when the db-connection fails during the upgrade-process etc... Opinions?

avatar Sandra97
Sandra97 - comment - 14 Jan 2019

@continga I've unblocked you. You should be able now to edit the page(s).

avatar regularlabs
regularlabs - comment - 14 Jan 2019

@continga Regarding the @ prefix on field names. I don't see it used in Joomla core fields anywhere anymore, but it is something it definitely supports.
You can use it when you have (custom) field types that output a field but you don't want the value sent along to get stored in any way.
Joomla will not send the @-prefix fields along with the form data.

avatar AndySDH
AndySDH - comment - 14 Jan 2019

@continga To see the issue above in action, please see what happens when creating a subfield with this field type: https://www.regularlabs.com/extensions/articlesfield

Nice work with the documentation.

As far as the name goes - I see, it's not much about the name but the the tranformation of the fields into the new way of doing things. Then yeah, if the name makes no difference, we can keep subfields :)

avatar AndySDH
AndySDH - comment - 14 Jan 2019

PS: I don't know if you care enough for the styling (maybe not seeing you haven't addressed my previous post on the styling layout), but another thing I noticed is that it has no consistent widths for the columns:

image

Maybe could be good to uniform them. So if 3 columns, always have them 33.33%.
If 2 columns, 50%. If 4 columns, 25%. If 5+ columns, probably drop everything to a new line and stack the subfields (see my post above about the layout).

avatar continga
continga - comment - 15 Jan 2019

@Sandra97 thanks!

@regularlabs OK I tested your plugin with this, and ofc something was broken there. The first thing I noticed was that your field definitions for e.g. the rl_showon field type was not loaded. I just pushed an update which fixes this and makes sure that additional field paths of external custom field types are correctly being loaded. Now at least it doesn't look completely broken anymore. ? I am not sure about that @-prefix thing yet (whether we really need to take action there) - but before checking that, I noticed another thing: Your rl_showon field type seems to be not working anymore, as it now always is shown in the sub fields selection, no matter which custom field type has been chosen. For me this looks like something that you have to fix in your extension, so can you please take a look at this and tell me if you need me to change anything in plg_fields_subfields for this, or whether this is something you can fix?

@AndySDH Yeah, I do not care too much about the styling, but you have a point ? I just pushed an update which uses the repeatable subform layout instead of the repeatable-table layout, when there are 5 or more sub fields present. This should fix the "squeezed together" problem. I must admit I have no idea how to fix the other thing, the "not equal width"-thing. I am using the standard subform field type of Joomla! to display that table, and as far as I know it does not support any kind of possibility to influence the width of the columns, please correct me if I am wrong. Do you have an idea how to fix that?

avatar continga continga - change - 15 Jan 2019
The description was changed
avatar continga continga - edited - 15 Jan 2019
avatar regularlabs
regularlabs - comment - 15 Jan 2019

Your rl_showon field type seems to be not working anymore, as it now always is shown in the sub fields selection, no matter which custom field type has been chosen.

Ah, that will be be because of the changed field names too!

The rl_showon stuff uses the Joomla core showon functionality.
This shows/hides elements based on the value of other fields.
But as you are changing the field names (by prefixing them), stuff goes haywire, as the showon stuff is still looking for the original name.

You would also face this issue with simply using the Joomla core showon="" attributes on fields.

No idea how you could get around that.

I am not sure about that @-prefix thing yet (whether we really need to take action there)

Well, yes, you would need to take action here, as you need to comply with how Joomla handles xml form fields. Adding a @-prefix is a built-in core feature that helps you prevent including that field in the sent form data. If you do not support that in the subform fields, then that is not compliant with the Joomla core handling of xml form fields everywhere else.

avatar AndySDH
AndySDH - comment - 15 Jan 2019

@AndySDH Yeah, I do not care too much about the styling, but you have a point ? I just pushed an update which uses the repeatable subform layout instead of the repeatable-table layout, when there are 5 or more sub fields present. This should fix the "squeezed together" problem. I must admit I have no idea how to fix the other thing, the "not equal width"-thing. I am using the standard subform field type of Joomla! to display that table, and as far as I know it does not support any kind of possibility to influence the width of the columns, please correct me if I am wrong. Do you have an idea how to fix that?

Cool about the 5+ subfields thing ?

As far as the equal widths, I am not sure. One way would be giving a class to the repeatable table based on the amount of subfields present. And then style the columns of that table based on the class that the table gets. That's one way I can come up with.
If that's not possible either, I'm guessing that would be something that should be addressed in the standard Joomla! subform field type itself, so probably in a standalone PR.

avatar regularlabs
regularlabs - comment - 15 Jan 2019

"So, Peter, what is your opinion on all of this", I hear no-one ask.
Giving it anyway :)

Sorry to have to say this, as I really don't want to get involved (energy-sucking stuff),
But I think the whole approach here to the the subfields is flawed.

There are so many issues that are being thrown up because of weird (read:wrong) decisions from the start. (Yes, I know, great way to start a message and get sympathy :P )

I think it would solve so many issues to just not have entire custom fields (forms) being loaded inside the subfields field.
A different approach would be:

  • Create new custom fields like you would normally, and optionally state somewhere that it is to be used for the subfields.
  • Create a subfields in which the + button simply gives you the option to select one of these earlier created custom fields (not field types).

You could add an edit button that could open that field in a modal to edit the custom field properties. But no inline (inside subfield) editing.

This approach would solve a lot of stuff from the get-go, resulting in:

  • No more crappy UI issues, like having to scroll through a forest of fields and settings to see the newly added fields (+ button at top, new field 3 miles under it).
  • No more renaming of form names, which cause a plethora of issues (including the showon stuff)
  • No more loading ALL extra fields for every field (which is what is being done now)
  • Way faster/lighter pages
  • Third party devs no longer being limited in development, as stuff that works in normal custom fields now doesn't work inside subfields.
  • Being able to use the same custom field (including all settings) multiple times in different subfields fields.

So again, I think it would be a big mistake to continue on the path this subfields is currently going.
You end up with a bug-ridden piece of... code. That will need all sorts of dirty bandages and patches to try to cover up underlying issues.

avatar regularlabs
regularlabs - comment - 15 Jan 2019

@continga I realise my last reply might come across aggressive. But it really isn't meant to wipe away any of the hard work and effort you have done. It is meant to make my concerns clear about where this thing is heading and the issues it causes.

avatar AndySDH
AndySDH - comment - 15 Jan 2019

I personally agree with @regularlabs.

I was one of the people that suggested from the get-go to use the approach of first creating the fields and then simply selecting them.

image

As we can see now, that approach would solve a pletheora of issues too.
Also I might add to all the advantages raised by Peter:

  • This would also allow every field to also have their own Form Options and Render Options (so options for showing the Label or not, CSS classes, choosing an alternate layout, etc.). While those are completely missing as well now.

I get the idea of trying to start small, but the current approach probably causes more issues than it solves :(

avatar continga
continga - comment - 15 Jan 2019

Well, I do not see a "plethora of issues" with the current approach. I have to say that.

But. I do agree, that the current approach makes things unneeded complicated and requires some error-prone "bandages", while your proposal highly would easy the complexity whilst still being equally powerful.

So, yeah, I will start to work on implementing it that way.

avatar AndySDH
AndySDH - comment - 15 Jan 2019

Awesome man.
Of course, not to discount everything you did so far (and the idea to do this in the first place!), which is super appreciated! Just trying to find all together the best approach to improve it and make this as solid and powerful as it can be :)

avatar continga
continga - comment - 16 Jan 2019

I just pushed an update which changes the implementation like discussed. This is the first version of that, so happy testing and I will be glad to react on feedback from you guys.

One thing I would like to discuss directly is the replacement of plg_fields_repeatable which is currently done. I don't longer think this is a good idea, I think we should just leave the repeatable plugin in core, but mark it is deprecated and inform users they should switch to the subfields plugin. Reason: Please take a look at the method in the update script which is responsible for transforming repeatable- into subfield-fields: https://github.com/continga/joomla-cms/blob/4309e77a43105f9b9f2a57a8ab98405d1549b8c0/administrator/components/com_admin/script.php#L385 It is pretty complex and I had to do all kind of non-obvious stuff (like renaming field names in case of a collision) which potentially can break existing setups (if people rely on the field names). Instead of taking that risk I think we should just leave the repeatable plugin, but mark it is deprecated and suggest users to use the subfields plugin instead.

avatar regularlabs
regularlabs - comment - 16 Jan 2019

Another option would be to:

  • Not include the Repeatable Field plugin in new packages (so it doesn't get installed on new setups/updates).
  • Check if there are any uses of the Repeatable Field, if not: uninstall it.
  • If the Repeatable Field is used, change/update code so a warning message shows in the field when using it.

So this does not require going through any hoops to try and port data from one to the other.

avatar AndySDH
AndySDH - comment - 16 Jan 2019

Sooo... I went through a deep, long and tortuous session of testing :D
First of all, great job! I tested this thoroughly and there are lot of things done really well, so -> awesome! This approach is a lot more clean and streamlined, makes the interface very easy to use.

As far as the repeatable field, I think @regularlabs brought up good ideas. I would follow his advice :) So not install it for new setups/updates. And uninstall it only if there are no instances of the repeatable field used. If it is used, then show the warning message that suggests to "switch to the subfields plugin and uninstall the repeatable field plugin".

Below I'm gonna list the issues that I found in my testings and that should be addressed :) I've put the list of things in numbers so it will be easy for you to address each point:

  1. First thing first, we need to implement a way for a custom field to optionally be made available only to be used inside subfields. There is a big chance that you might want to have certain fields to be ONLY used inside subfields and not be displayed normally.
    So we need to find an idea/solution that allows people to mark a sub field to be "exclusively available for subfields" and not otherwise visible normally. Maybe we can add a checkmark or an option to the field options. Or other ideas are welcome.

  2. When displaying the Sub field Select dropdown, also display the field type along with the Field Name. So something like:

  • Company (list)
  • Gender (radio)
  • Platform (checkboxes)
    I think that would be very useful visually to know what field you're selecting. But if you want you can also keep the field alias. So display both things there (field alias and field type).
  1. Right now it's possible to select the same field more than once inside the same subfields (Then in the article form the duplicates are not displayed). So this should be addressed. You probably shouldn't be able to use the same field twice inside the same subfields.

  2. If the form is set to be repeatable, have the first row already expanded by default in the article form. Right now you have to click on the "+" to make the first row even appear. The first row should already be there.

  3. If the field is set to "Required", that is not respected when leaving every subfield empty. If you leave every subfield empty in every row (literally nothing is compiled, not even one value at all in any cell), then the field should not save if it's set to required, and should return the "required" warning. (see more comments below by @regularlabs)

  4. Each sub field should inherit their form options (Options -> Form Options) from its settings. This way each sub field inside a subfields can have its own Placeholder, its own Field Class, and Label Class being respeced. Sorry, it looks like this is already supported! Ignore n.6, WELL DONE! :)

  5. Each sub field should inherit their Render Options (Options -> Render Options) from its settings. When "Render values" is set to "Yes", the render options of each sub field should be inherited for rendering each sub field. These are the Render Class, the Show Label Yes/No, the Label Class and the Alternate Layout in each sub field.
    Consequently, when displaying the field in the frontend, the Label of the field will be shown in front of the value, instead the "name" of the field.
    To do this, simply take inspiration from "components/com_fields/layouts/fields/render.php"
    This is the layout for field groups. It calls for the correct render layout of each field.
    Only difference will be that instead of using <dl> as a field container, you would be using a <li>. And instead of using a <dd> to display each field, they would simply be <span> separated via a comma.

  6. By doing this, and since we are using existing fields, you also don't need to store the name of the field in the database anymore in the field values. We should instead use the field ID here. While the field name is unique as well, it's possible to change a field name. So if you change the name of a sub field, there could be problems there. So it would be more stable to use the ID of the sub fields.

  7. Bug:

  • Set the field to not be repeatable.
  • Compile values in each subfield in the form and save.
  • Now set the field to be repeatable.
  • Now in the form the values you put earlier are lost, and the form will automatically get as many (empty) rows as the number of subfields.
  • The expected behaviour should instead be that once you set to repeatable a previously non-repeatable field, the values are kept and stored in the first row of the repeatable form, and only that first row will be shown in the form.
  • The reason for this is because you're saving a non-repeatable field without it being a row. It should be saved as the row0.
  1. Like @regularlabs suggested earlier, maybe in the subfields creation form you could add an edit button that opens a modal to edit the sub field properties. This is by no means necessary or vital as much as the other things mentioned, but would be a cool extra usability feature. (see more comments on this below)

  2. It shoudn't be possible to nest a repeatable field type as one of the sub fields. The idea is that the repeatable field type is outdated/deprecated so there is no need to allow it to be selected inside a sub field.

avatar Quy
Quy - comment - 16 Jan 2019

When creating a new custom field and switching to type Subfields:
Notice: Trying to get property 'context' of non-object in \plugins\fields\subfields\subfields.php on line 79

avatar regularlabs
regularlabs - comment - 16 Jan 2019
  1. sub fields (values and connections) should definitely be saved in the db using their ID. As the names are not fixed.
avatar regularlabs
regularlabs - comment - 16 Jan 2019
  1. The required setting might not even be useful inside the subform field itself.
    I think it might be best to let it rely on the required setting inside the individual sub fields.
    Meaning one field might be required, but another isn't (this already works, I believe).

If there is going to be a required setting inside the subform field itself, I guess that would overrule any underlying individual required settings and make all sub fields required. But that might be confusing. So I think it would be better to just not have a required setting in the subform field.

avatar AndySDH
AndySDH - comment - 17 Jan 2019
  1. Like @regularlabs suggested earlier, maybe in the subfields creation form you could add an edit button that opens a modal to edit the sub field properties. This is by no means necessary or vital as much as the other things mentioned, but would be a cool extra usability feature.

Idea regarding this. We could change the whole select form to be done like for Joomla article/category menu items. So instead of being a dropdown, it could be something like this:

image
(Of course with "Select a Custom Field" instead of "Select an Article")

Where it then expands a modal with a list of Fields looking exactly the same as the Field List view, along with filters on top as well. So exactly like a menu item for an article is created. This would also solve issue n.2, as the field types would be displayed along the field names in the field list.

And you also have the "Create" button there to create a new one.

After inserting it, the form would turn into this:

image
(Again of course with "Select a Custom Field" instead of "Select an Article")

With the ability to edit the field or clear it to select another one.

avatar AndySDH
AndySDH - comment - 17 Jan 2019
  1. First thing first, we need to implement a way for a custom field to optionally be made available only to be used inside subfields. There is a big chance that you might want to have certain fields to be ONLY used inside subfields and not be displayed normally.

Regarding this, probably the easiest way to do this (without adding any extra option), is the ability to leave the category assignment empty:

image

However, right now, when leaving that empty and saving, it will automatically be filled with "All".

image

Simply disabling this auto-fill behavior - thus allowing to assign a field to 0 categories - would make that field not visible in any article form normally. (therefore making it possible to only use it as a sub field)

avatar AndySDH
AndySDH - comment - 17 Jan 2019
  1. Another issue

This seems to work only with PHP 7.2 right now.

With PHP 5.6, 7.0 and 7.1, when you try to create a subfields field you get:
Fatal error: Call to undefined method DOMNodeList::count() in plugins/fields/subfields/subfields.php on line 72

avatar continga
continga - comment - 17 Jan 2019

Hi all, I just pushed an update.

@Quy thanks, fixed.

@AndySDH & @regularlabs thanks for your feedback. Here my response:

  1. Good idea, but how should we do it exactly? Just adding a checkbox in the "Options" tab seems a bit overkill, not sure whether this really is good UX. Leaving the categories empty like suggested by you is also kind of counter-intuitive I think. Maybe somebody has a better idea?

  2. Done.

  3. Done, will now be deduplicated when the subfields is being saved.

  4. This is the default behaviour of the subform field type. Not sure whether we really want to change that. That means every time we edit an article, an "empty row" will get added when there were no values, and we get an empty row saved then too, if not removed manually. I don't think this is a good idea?

  5. Not fixed yet. I have to think about it.

  6. This is already the case.

  7. Done.

  8. This is already the case.

  9. This is again the default behaviour of the subform field type. Not sure whether we can fix this. I need to think about it.

  10. Tbh, this is really a nice idea, especially doing it like the "Select an article" thing, but I don't think I will implement that in this PR. It is not vital, it is not important, it is something nice-to-have. We can maybe work on that when this PR finally got merged sometime, and then work on that as a follow-up PR.

  11. The repeatable type should not be existant at all when using this codebase, hence also not appear there. Also, see next paragraph.

  12. Fixed.

Regarding how to continue with the repeatable plugin: Can one of you guys maybe implement it like discussed in a fork, and I will merge it back? Or, maybe we should even create a new issue to handle this. I think it would be a good idea to split these two things. Then I could just undo my changes to the repeatable plugin here and we handle it in another issue. A PR for that issue can then base on the code from this PR. This will also allow us the split the discussion, as this starts to get a bit messy.

TL;DR All fixed but point 1, 5 and 9.

avatar AndySDH
AndySDH - comment - 17 Jan 2019

Wow, @continga, great work! Didn't expect it to be addressed all so quickly :)

  1. I agree, I'm not entirely convinced about the empty category method myself either. At the same time, I'm thinking that if a field is set to be used only as child field, then the category selection doesn't make sense for it anymore. Maybe it could be be something like...

image

It's set to "No" as default. If you set it to "Yes", the Category input disappears (hides). And if it's set to Yes, it won't display in any form in any category, but will only be available to be inserted inside a subfields. What do you think?

2, 3, 7, 12. Awesome!!! Looks great. Well done :)

  1. Uuh I see what you mean... I understand. Yeah, not ideal to save empty rows. Unless there is a way to check whether all values are empty, and if they are, to not store the row. Is it possible somehow?

  2. As @regularlabs suggested, I think a good idea is to get rid of the 'required' option altogether, and make the subfields rely on the inherited 'required' options of the child fields. What do you think?

  3. Yep, noticed that later!

  1. I'm afraid that's not already the case, currently the field values are stored in the database with the field names and not with the field IDs. See:

cattura

As you can see, both the parent field names and child field names are used in the database value storing. For the child items it's best to use the ID, while as far as the parent, we don't need an identifier at all as we're already inside a value of the correct field_ID column, so I think each row could be simply stored as generic row0, row1, row2, and so on for all fields. (without the name of the parent). As the names of the fields (both parent and child) can always be changed and this would cause issues.

  1. I see. However it's pretty bad usability if you can't change a field from repeatable to non-repetable (or viceversa) without breaking it and losing all of the values. So I think it should be addressd.

  2. Sure, that sounds good ?

  3. Right, but in cases that some users still have it installed for whatever reason... Even if they do, it should not be presented to them as an available sub field, so I guess you should exclude it from the selectable fields? Or it's not worth it?


  1. [new] Another thing I noticed:

If a field have no values filled in the form in any of the cells, the field is still displayed in the frontend like:
, , ,
(just commas)
This also happens when the field is set to NOT be repeatable.
Do you have a way to completely prevent the display of the field if all values are empty?

Thanks again for your great work and effort!

avatar AndySDH
AndySDH - comment - 17 Jan 2019

Still a PHP issue now in 5.6 only: Parse error: syntax error, unexpected '->' (T_OBJECT_OPERATOR) in /plugins/fields/subfields/subfields.php on line 79

avatar regularlabs
regularlabs - comment - 17 Jan 2019
  1. You could force-add the empty row if any of the sub fields are required.
    If not required, then you need to manually add the row...
avatar AndySDH
AndySDH - comment - 17 Jan 2019

Another alternate to n.1 would be adding a "None" option to Categories:

administrator/components/com_fields/models/forms/field.xml
1547756561

And should probably also check when saving that if there is a -1 value in the cat list, that it removes any others. As you don’t want to be able to save it to None + others:

administrator/components/com_fields/models/field.php
1547756516

PS: @regularlabs might have had something to do with this idea and might have sent me the screenshots. But that's just a rumor.

Up to you which one you prefer between this and the toggle in earlier post :)

avatar continga
continga - comment - 19 Jan 2019

Hoi, I just pushed another update. Following things have been done:

4: Changed so that when at least one sub field is required, always one row will be shown (like discussed).

5: The 'required' option is now not available for the subfields custom field, and its value will be taken from the child fields, like discussed.

8: Ah, I thought you meant how the relation parentfield<->subfields is being saved in the database, not the values themselfes for items. However, should be fixed now!

9: I don't think we should do this in this PR. This would mean changing a lot of logic for the subform type, and I don't know how this would affect existing setups. So let us leave this like it is.

13: Fixed.

The additional php5.6-compatibility thing has been solved too.

Still TODO: Setting fields as available-for-subfields-only (maybe with the category-solution? I have not decided yet) and point 9 (maybe we should really just hide the repeatable type here, and handle the removal of the repeatable type in another issue, but have not decided finally).

Is it correct, that those currently are the only two missing points? I must admit I kind of lost overview... All those comments with feedback, it is getting a bit confusing... :-) But thanks guys, seems like we are finally getting there! :D

avatar Fedik
Fedik - comment - 19 Jan 2019
  1. Right now you have to click on the "+" to make the first row even appear . The first row should already be there.

This is correct behavior. That why you have a "+".
What if you do not need any row/value?
There should not be any Row if min attribute are not defined or less than 1

  1. Changed so that when at least one sub field is required, always one row will be shown (like discussed).

This is wrong.
There 2 different cases: Subform Field required, and One of the children field are required.
They both are valid, and should not be mixed.
Subform Field required: means that the subform should have at least one row (mainly useful for multiple)
One of the children field are required: means , IF there at least one row, then the required field should have a value.

  1. Bug

This not a bug.
This is correct behavior of the subform field and should not be changed. Important point, that the 'multiple' attribute should not be changed if field already has a stored value, because it will destroy your data.

avatar AndySDH
AndySDH - comment - 19 Jan 2019

A couple of more things that I noticed:

  • When the field is set to be repeatable, it looks like that when trying to save an article without compiling a required sub field, the page will generate an error at the top but without stating what the error is (empty error). It should be saying:
    "Invalid field: [name of the field]"

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

Everything else that you did today @continga seems to work as expected, fantastic job man!
Great stuff with the triple checks of empty values, empty rows and empty field altogether in the template file.
We're indeed getting there! :)

avatar AndySDH
AndySDH - comment - 19 Jan 2019

I found a super weird issue. Here's how to reproduce:

  • Create a list field (to be used as sub field), let's call it "Fruits" As options, add an option with a name "Apple" (on the left) and a option value of "1" (on the right)
  • Create another list field (to be used as sub field), let's call it "Animals". As options, add an option with a name "Cat" (on the left) and also an option value of "1" (on the right)
  • Create two separate subfields field types. One having "Fruits" field as its only sub field. And the other having "Animals" as its only sub field, - Edit: This also happens if both "Fruits" and "Animals" are sub field of the same subfields.
  • Now go in an Article Form. In the first subfield, fill-in the value "Apple". In the second subfield, fill in the value "Cat".
  • Visit the article on the frontend. Both fields will show value "Apple".

TL;DR: If the option value is the same for two field values in two different fields (in this example "1" for both), the value of the first field will show in both fields.

avatar Fedik
Fedik - comment - 20 Jan 2019

If one of the children is required, it means it has to have at least one value in one row. So one row should become inherently required as well.

Nope, it doesn't meant this.
In your app you can be no need a value from the subform field, but if it exist then one of its field must have a value. How you deal with that, if you always force to "required"?
As I said it is different cases, and should not be forced.

That very easy:
if you need at least one Row in the subform, then you set it to required="true" (optionally can add min=1);
if you need some Text in the subform to be filled, then you set required="true" to the text field;
if you need at least one row with some text to be filled, then you set both to required="true" ;

Well, it might not be a "bug", but if it's not a bug, it's a very bad design choice

It is correct design choice.
The field (not only subform but any other field) should not care about data structure, the field must receive the value already in correct structure.
The structure of the data should be handled by a app model.

avatar AndySDH
AndySDH - comment - 20 Jan 2019

That very easy:
if you need at least one Row in the subform, then you set it to required="true" (optionally can add min=1);
if you need some Text in the subform to be filled, then you set required="true" to the text field;
if you need at least one row with some text to be filled, then you set both to required="true" ;

Ok, sounds okay, I have no problem with this way of doing it.

avatar joomla-cms-bot joomla-cms-bot - change - 24 Jan 2019
Category SQL Administration com_admin Postgresql MS SQL com_fields Language & Strings Installation Front End Plugins Libraries SQL Administration com_admin Postgresql MS SQL com_fields Language & Strings Front End Installation Libraries JavaScript Plugins
avatar continga
continga - comment - 24 Jan 2019

Hi all, I just pushed another update.

I changed the "required"-handling for the subfields type again. Now the Yes/No checkbox for the subfields instance itself is back and decides whether at least one row of the sub fields is required. The "required" checkbox at the sub fields themselfes only decides whether this sub field is required inside of a row (this is like we discussed recently).

I added the "None" selection for the category when creating a custom field. Selecting "None" there will not display that custom field anywhere, hence making is usable only for a sub field by the subfields plugin without appearing anywhere else in an item, like discussed. I think this is a good solution.

I reverted the removal of the plg_fields_repeatable custom field type and created a new issue #23659 to handle this. All discussions regarding this topic should go into that issue and we can ignore that plugin in this PR for now.

Regarding the (empty error) thing when a required sub field is not being filled, this is a general problem with a subform containing required elements. The reason is, the javascript which is responsible for displaying the error message is searching for a label for the input element to display which field is containing the error:

label = jQuery(invalid[i]).data("label");
but inside of a subform, an element does not have a label, it only has a parent tr-element respectively a data-column attribut on itselfes td-element. So, yes, this is a bug, but we shouldn't fix it here but create a new issue for this (feel free to do so).

I fixed the calendar-thing with the "-0001-11-30" output.

The fruits/animals bug with the list sub field type is fixed too.

I also updated the documentation on docs.joomla.org to reflect the latest status of this PR.

So, as far as I know, I now fixed all bugs found so far and there are no open points anymore at the moment, please correct me if I am wrong. Please feel free to do some additional testing and let me know the results :-)

Thanks!

avatar AndySDH
AndySDH - comment - 24 Jan 2019

Great stuff @continga! Testing now.

Tweak to do. When selecting "None" in the category, it will still show as if it was set to "All" in the fields list:
image

avatar AndySDH
AndySDH - comment - 24 Jan 2019

Issue with the Calendar field still kinda happens. How to reproduce:

  • Fill the Calendar field with a value. Save the article.
  • Now empty the Calendar field. Save the article.
  • You can see now the issue happening in the frontend.
  • If you save the article again (with the calendar value already empty), without touching anything, the issue will be gone.

So it happens after immediately clearing a filled-in field. Probably not dealbreaking, but worth reporting

avatar AndySDH
AndySDH - comment - 24 Jan 2019

Since there is the issue that changing the subfields from repeatable to non-repeatable (or viceversa) will destroy the data, and it looks like the issue won't be fixed from what you guys said...

A backup idea would be to make the "Repeatable" option disabled after a field has been saved. Similar to how you cannot change a field type after creating the field, the repeatable option cannot be changed after creating a field.

Either that, or we need to place a huge warning when someone tries to change the 'repeatable' setting. Losing all their data when changing that setting is definitely not something anybody wants to go through.

Thanks @continga, everything else looks great! We're pretty much done :)

avatar continga
continga - comment - 24 Jan 2019

Issue with the Calendar field still kinda happens. How to reproduce:

* Fill the Calendar field with a value. Save the article.

* Now empty the Calendar field. Save the article.

* You can see now the issue happening in the frontend.

* If you save the article again (with the calendar value already empty), without touching anything, the issue will be gone.

So it happens after immediately clearing a filled-in field. Probably not dealbreaking, but worth reporting

Did you make sure to clear your browsercache? This was a javascript-issue and should be fixed already, but it might be that js-file still is in your browsercache.

avatar AndySDH
AndySDH - comment - 24 Jan 2019

Did you make sure to clear your browsercache? This was a javascript-issue and should be fixed already, but it might be that js-file still is in your browsercache.

Yep, happens with cleared cache too. Not happening for you?

avatar continga
continga - comment - 24 Jan 2019

Did you make sure to clear your browsercache? This was a javascript-issue and should be fixed already, but it might be that js-file still is in your browsercache.

Yep, happens with cleared cache too. Not happening for you?

Ah, I forgot to add the minified version of the js file. Should be fixed now

avatar AndySDH
AndySDH - comment - 24 Jan 2019

Ah, I forgot to add the minified version of the js file. Should be fixed now

Yup, indeed. Works now :)

So only a couple of things to resolve I think and we're good to go!

avatar AndySDH
AndySDH - comment - 24 Jan 2019

There is an issue with 3rd party fields that use form fields that try to load javascript and/or css. Resources are not getting loaded in that case.

avatar roland-d
roland-d - comment - 30 Jan 2019

@continga I have tested this and it is starting to look good. Thank you for also writing some documentation already.

I was just wondering about this:
image

Wouldn't it be better to use the table layout here since we only have 2 fields? That makes the list clearer and easier to drag and drop if you have a higher number of fields.

avatar joomla-cms-bot joomla-cms-bot - change - 8 Feb 2019
Category SQL Administration com_admin Postgresql MS SQL com_fields Language & Strings Installation Front End Plugins Libraries JavaScript SQL Administration com_admin Postgresql MS SQL com_fields Language & Strings Templates (admin) Front End Installation Libraries JavaScript Plugins
avatar continga
continga - comment - 8 Feb 2019

Hey guys, I just pushed an update which addresses the latest remarks.

Fixed: Component showed 'All' instead of 'None' category
Feature: 'Repeatable' attribute of subfields instance now cannot be edited anymore.
Fixed: Formatting feedback form @Quy
Fixed: Use table layout as suggested by @roland-d

There is an issue with 3rd party fields that use form fields that try to load javascript and/or css. Resources are not getting loaded in that case.

@AndySDH do you have an example for that?

avatar AndySDH
AndySDH - comment - 8 Feb 2019

Try Articles Field: https://www.regularlabs.com/extensions/articlesfield

If you put an instance of Articles Field as part of a 'subfields', and then visit the article form, you will see that the Articles Field won't load its required javascript (so the select won't load).

If the article form already contains a standalone Articles Field, the javascript gets loaded normally and so the Articles Field is also displayed inside the subfields form. So the issue is that the field type associated javascript is not getting picked up when part of a subfields.

I'm thinking this might be the only issue left to fix, everything else I tested correctly and no other issues came up!

avatar regularlabs
regularlabs - comment - 8 Feb 2019

You can also try with the "Advanced Custom Fields" type "Time Picker" from @tassosm:
https://www.tassos.gr/releases/advanced-custom-fields/advanced-custom-fields-0-5-0

ezgif com-optimize

avatar regularlabs
regularlabs - comment - 8 Feb 2019
avatar continga
continga - comment - 8 Feb 2019

can you maybe give me a test-case where I do not need to create an account to download a plugin, or create an account to get an API-key? ;) I would really appreciate that. Thank you.

@regularlabs your free version of articlesfield throws a fatal error in Joomla! 3.10 when creating an article, so I also cannot test it with that: Call to undefined method PlgFieldsArticlesFilters::getCurrentArticleId() in plugins/fields/articles/fields/articles.php on line 80

avatar regularlabs
regularlabs - comment - 8 Feb 2019

The free version of Articles Field should work now. Just released a new version.
You don't need any account or API to test these things.

avatar continga
continga - comment - 8 Feb 2019

You don't need any account or API to test these things.

I meant the plg_fields_travel and advanced-custom-fields plugins you posted... For the first I need an API-key for the geocoding APIs obviously, for the second I need an account on tassos.gr to download the plugin ;) But nvm, will test with your free plugin now

avatar regularlabs
regularlabs - comment - 8 Feb 2019

Ah, yeh, forgot about the login needed for @tassosm
But you can try the one from @coolcat-creations, which you can simply download from github (downloading the whole thing and installing that zip works).
I think that Travel fields is probably a little less complex to try out.

avatar continga
continga - comment - 8 Feb 2019

Okay, I tested this with the plg_fields_travel plugin, and we need to talk about how we can fix this. What I am about to say is preliminary (have to go now, but wanted to share my preliminary results already now), so keep that in mind when answering and/or dissent with me ;) . I currently only have tested the plg_fields_travel thingie, and maybe for other plugins the problem is different etc...

So, what the plg_fields_travel plugin does, is it inserts some javascript on the page, that when the DOM is loaded calls the places.js API to render its textfield with functionality (autocomplete, suggestions, etc). Please take a look at the relevant code https://github.com/coolcat-creations/plg_fields_travel/blob/e11f1adadec5ca6a96bfcdca2fa1835af264b25e/travel/fields/travel.php#L24

Problem now: When we have that travel field inside of a subfields instance, and we add a new row of that subfields instance when creating/editing an article, that input-field of the travel plugin has no JS events bound to it, as the field was not existing when the DOM was loaded (it just gets created when the user clicks the "Add row" button). So the JS is not bound to that field when it gets added to the DOM after the page has been loaded, and hence the field is not working as expected.

One solution would be to change the code of the travel field type plugin to also could handle input fields with the class "address-input", when those are being added later to the DOM by e.g. the subform repeatable class. This could be done e.g. this way https://www.diffchecker.com/B7EjgmYE This is a short quick'n'dirty fix, there might be better ways to do this. With that diff being applied, the plugin works without problems inside of a subfields instance.

Now. Well. This is obviously one solution: Dump the work for fixing this to the plugin authors, letting them change their code to have in mind that their fields also can be added to the DOM after the page already has been loaded. I think this is the cleanest solution, as it is the plugins' authors responsibility to take care his field also works when it gets added to the DOM after the page has been loaded, as done by the subform type.

But, as I understand, this solution might not be very pretty or satisfying for you guys. First of all because we are introducing something which requires adaption of plugin-developers... And second, because we are breaking stuff for abandoned plugins... But yeah... We are implementing this for a new major version, and it is not breaking existing setups (not backwards incompatible), it just does not work with current plugins... So it should be justifiable?

What I currently don't know is whether this javascript-problem is the cause of the other bugs you guys mentioned here. I couldn't test this currently, would be nice of someone of you could take care of that. But I guess it is the same problem. And the solution should be the same, at least from my point of view: The plugin authors need to take care of that.

As I said, this is my preliminary opinion, and I would like to hear what you guys think of this. Go on :D

avatar regularlabs
regularlabs - comment - 8 Feb 2019

Ok, to not zoom in to specific javascript approach issues, the main issue in the subform that needs to be tackled is this:

Custom Fields can make use of different (custom) form fields.
These formfields may depend on js/css stuff to function.
These are loaded via the JFactory::getDocument() methods, like addScript(), addScriptDeclaration(), addStyleSheet(), addStyleSheetDeclaration() and anything else that triggers js/css to get loaded.

The fields inside the subforms are loaded via javascript and do not do anything with these js/css calls. So the dependant files are not loaded.

I do not have a direct solution for this. But it would already be a major step if the subform would already preload the fields (at least 1 set) up front, so that the JFactory::getDocument() methods actually get executed.

And throw some javascript event when new subfields are added, so that developers can re-trigger their js functions on newly added fields.

avatar Fedik
Fedik - comment - 8 Feb 2019

So the issue is that the field type associated javascript is not getting picked up when part of a subfields.

It not an issue here at all, it an issue of that fields that have a javascript.
Search for "Be aware" there https://docs.joomla.org/Subform_form_field_type

But it would already be a major step if the subform would already preload the fields (at least 1 set) up front, so that the JFactory::getDocument() methods actually get executed.

All scripts and styles (and etc) are preloaded.

And throw some javascript event when new subfields are added, so that developers can re-trigger their js functions on newly added fields.

Everything already there, since it was made.

avatar regularlabs
regularlabs - comment - 8 Feb 2019

Ok

avatar AndySDH
AndySDH - comment - 9 Feb 2019

@Fedik The issue is not the same as described in "Be aware". Or at least not only that. This is an issue also in single mode (not only multiple).

Instructions to reproduce:

  • Create an individual Articles Field. Set it to category "All".
  • Create a Subfields containing the above (or any other) Articles Field. Set it to category "All".
  • Visit the article form.

Now both instances of Articles Field work (the individual one and the one as part of the subfields):

image

And if you view the html source page, you can see that these two resources are correctly loaded:

media/regularlabs/js/form.min.js
media/regularlabs/css/form.min.css

All good so far.

Now, set the individual Articles Field to category "none". So it is no longer displayed in the article form as a standalone field.

Visit the article form.

Now, the instance of Articles Field as part of subfields doesn't work anymore.

image

The reason being, if you view the html source of the page, you can see that the two required resources are not getting loaded anymore:
media/regularlabs/js/form.min.js
media/regularlabs/css/form.min.css

So, the resources are only loaded if a standalone version of the field also exists on the page. Otherwise they are not loaded.

avatar Fedik
Fedik - comment - 9 Feb 2019

Now, set the individual Articles Field to category "none". So it is no longer displayed in the article form as a standalone field.
... the two required resources are not getting loaded anymore

It about preloading. Every JS/CSS resource are loaded, the same as any other "non subform" script.
It an issue with that field, you have to ask its author, why field do not load js/css at call of getInput.
I can guess the reason, but it not related to current pull request.

avatar AndySDH
AndySDH - comment - 9 Feb 2019

I have tested this item successfully on 0ced78f

I see. Well, in this case, I'll mark this PR as tested successfully. Nothing left to address/fix, RTC in my opinion.


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

avatar AndySDH AndySDH - test_item - 9 Feb 2019 - Tested successfully
avatar regularlabs regularlabs - test_item - 25 Feb 2019 - Tested successfully
avatar regularlabs
regularlabs - comment - 25 Feb 2019

I have tested this item successfully on 0ced78f

Seems to do as advertised.


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

avatar regularlabs
regularlabs - comment - 25 Feb 2019

I have tested this item successfully on 0ced78f

Seems to do as advertised.


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

avatar zero-24 zero-24 - change - 25 Feb 2019
Status Pending Ready to Commit
avatar zero-24
zero-24 - comment - 25 Feb 2019

RTC thanks.


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

avatar zero-24
zero-24 - comment - 25 Feb 2019

RTC thanks.


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

avatar HLeithner
HLeithner - comment - 25 Feb 2019

Merging this against 3.10 makes no sense can you change this or create a new PR against 3.9 please

avatar brianteeman
brianteeman - comment - 25 Feb 2019

@HLeithner ITs a new feature so it can only go into 3.10

avatar woluweb
woluweb - comment - 25 Feb 2019

Perfectly. As a new feature, it has to ship with the next subversion... being 3.10

But would it be possible to provide this Custom Field as an installable plugin (since Custom Fields are "just" plugins) ?
That way, people could already enjoy it (and we would also have more feedback)

avatar AndySDH
AndySDH - comment - 25 Feb 2019

There is way more done in this PR than just a new plugin.
As you can see by the "Files Changed" tab.

There are many modifications/tweaks done to other core files in order for this to be supported. So it's just not a new installable plugin.

However, anybody can download the changed files and overwrite them on their set up, if you need.

Or just apply the patch via the patch tester. And if you do, please test and confirm if it works fine for you, more tests are welcomed.

avatar HLeithner
HLeithner - comment - 25 Feb 2019

3.10 will not be a feature release and is only for compat to 4.0, in this case please make directly a PR against 4.0 and close this one.

avatar brianteeman
brianteeman - comment - 25 Feb 2019

and that is where the plan is fatally flawed. this great new feature will not be seen for a year

avatar HLeithner
HLeithner - comment - 25 Feb 2019

That's the decision and you reminded me, thx.

avatar continga
continga - comment - 25 Feb 2019

wait, you are saying this now, 4 months after this PR has been opened and several feedback-cycles have been made, and after we finally reached the RTC status? Feels bad, man.

I have to say, this really is a bad atmosphere for open source contributions. Why didn't this topic come of earlier? Why isn't everyone aware of this? But this probably shouldn't be discussed in this PR. No hard feelings, how should we now continue with this PR to not lot it rot multiple months?

avatar AndySDH
AndySDH - comment - 25 Feb 2019

+1 vote for exception to merge in 3.9.4.
Thumb up if you agree. Lol
[/youtube-comment-noob]

Jokes aside, you could easily move plans for the current 3.10 into a 3.11 instead, and allow features like this into an intermediary 3.10.

avatar mbabker
mbabker - comment - 25 Feb 2019

No offense meant to the work put in here, but this PR alone isn't enough to justify a new feature release and all of the effort that goes into one. Plus, the project's roadmap already looks like a pin-the-tail-on-the-donkey board with how many times it has been said "3.x LTS will be here, 4.0 release will be here" and sooner or later someone needs to either come up with a realistic timeline for both releases or admit the project has bitten off more than it can chew and downscale to do what it can support and accomplish (which, it seems like saying no more 3.x feature releases so effort can focus on shipping 4.0 this decade has started to do because clearly trying to put out "big" feature releases and concurrently work on a new major version isn't working).

avatar AndySDH
AndySDH - comment - 25 Feb 2019

So, from Joomla blog post:
https://developer.joomla.org/news/764-joomla-3-10-and-joomla-4-0.html

Where should new features go?
Because of the specific nature of this minor version, we have decided not to merge any new features to Joomla 3.10 if they are not related to the compatibility layer and upgrade process. We are focusing only on the backward compatibility for the benefit of our extension developers and users.

Thus, we are formally announcing that any proposed new features should be made for the Joomla 4.0 branch. The CMS Maintainers Team can assist you with any help you may need to retarget any existing feature pull requests to be used with the Joomla 4 branch.

So I guess this would need to be retargeted to 4. But what I'm afraid of is that 4 might take even more than a year for a reason or another, so this would have to wait forever.

I don't know. This PR alone might not be enough to justify a new release in-between, but I'm sure there are some other cool new features PR as well.

Consciously planning to not release any feature at all until Joomla 4 is ready (who knows when) is a scary thought.

avatar alikon
alikon - comment - 25 Feb 2019

and surely some more can be included if got more tests

avatar mbabker
mbabker - comment - 25 Feb 2019

Consciously planning to not release any feature at all until Joomla 4 is ready (who knows when) is a scary thought.

Consider the alternative, which is in effect the current status quo. 4.0 development has been ongoing in some form since the middle of 2015 and still hasn't reached a beta milestone. Since the first try at a 4.0 release, everything from 3.5 up has been shipped. That's 5 "big" feature releases concurrent with 4.0 being in development (in all of its incarnations, and every major change in direction that has slowed down the current iteration (i.e. the push for yet another admin redesign over a year after the initial template had been committed)). Clearly, continuing to ship "big" feature releases for 3.x while continuing to develop on 4.0 (which has consistently been 5-6 patch releases behind 3.x's stable releases; right now 3.9 still hasn't merged forward) is not working and there is in essence a split community with people working on one or the other and not that much overlap, and it's now getting to the point where bug fixes are going into 4.0 without consideration of whether those same bugs exist in 3.x. So either feature work needs to slow down massively and/or stop in full on 3.x, or 4.0 needs to be abandoned; the resources aren't there to work on features for both and keep both branches synchronized in a sane manner.

avatar alikon
alikon - comment - 25 Feb 2019

that's true, but we cannot see only 0 or 1, even with low resources,
we need to have one foot on two shoes anyway.... to survive

avatar regularlabs
regularlabs - comment - 25 Feb 2019

Which begs the question:
Why did (and does) nobody jump in when efforts on new features for J3 start
(like this one). And say:
"Hey, there is a feature freeze on Joomla 3. If you want to create a new
feature, do it for Joomla 4!
So no need to spend months of your life investing in this for nothing."

On Mon, Feb 25, 2019, 20:12 Nicola Galgano notifications@github.com wrote:

that's true, but we cannot see only 0 or 1, even with low resources,
we need to have one foot on two shoes anyway.... to survive


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

avatar HLeithner
HLeithner - comment - 25 Feb 2019

There is a blog post on https://developer.joomla.org/news/764-joomla-3-10-and-joomla-4-0.html about this.

To understand the problem why there should be no new features in < 4.0 is because merging a 3.x commit in to 4.0 could take up to 10 minutes, it heavily depends on the commit how long it really takes.
J4 branch is 1105 commits behind 3. At the moment only George is doing this work so every pr merged in 3.x delays j4 release.

avatar brianteeman
brianteeman - comment - 25 Feb 2019

@regularlabs I gave up asking for someone to review the existing pr and make that comment

avatar alikon
alikon - comment - 25 Feb 2019

@HLeithner that's from 24 January 2019 , there are more acient pr's to consume before 2019

avatar alikon
alikon - comment - 25 Feb 2019

just think some gsoc related pr's , for example ?

avatar continga
continga - comment - 26 Feb 2019

So, should I create a new PR against staging to get this into a patch-release of 3.9, or what? Does anybody object, is it worth the work? Or what do we do now?

The more general discussion regarding 4.0 vs 3.10 is very interesting, but this PR is not the place to discuss that. Are there other places we could do that?

avatar HLeithner
HLeithner - comment - 26 Feb 2019

The PR is a (great) new feature and as long as the production department doesn't say anything new it have to go into 4.0.

So please rebase the PR on 4.0 branch and we can merge it.

avatar franz-wohlkoenig franz-wohlkoenig - change - 11 Apr 2019
Category SQL Administration com_admin Postgresql MS SQL com_fields Language & Strings Installation Front End Plugins Libraries JavaScript Templates (admin) Administration com_admin com_fields Front End Installation JavaScript Libraries MS SQL Plugins Postgresql SQL Templates (admin)
avatar continga
continga - comment - 23 Apr 2019

I have created a new PR #24711 that is based on this PR, but targets 4.0-dev, as it seems like we will not get this feature into 3.x

Please everybody involved in this discussion: Please take a look at #24711 and test it and let us know the results in that new PR! If we all work together I think we will be able to finally get this feature into the Joomla! core soon ;)

Thank you all for your help in advance. I will close this PR. Please let us take all discussions to the new PR.

avatar continga continga - change - 23 Apr 2019
Status Ready to Commit Closed
Closed_Date 0000-00-00 00:00:00 2019-04-23 23:58:30
Closed_By continga
Labels Removed: J3 Issue
avatar continga continga - close - 23 Apr 2019
avatar joomla-cms-bot joomla-cms-bot - change - 23 Apr 2019
Category SQL Administration com_admin Postgresql MS SQL com_fields Installation Front End Plugins Libraries JavaScript Templates (admin) SQL Administration com_admin Postgresql MS SQL com_fields Language & Strings Templates (admin) Front End Installation Libraries JavaScript Plugins
avatar HLeithner
HLeithner - comment - 24 Apr 2019

@continga thx for your work

Add a Comment

Login with GitHub to post a comment