User tests: Successful: Unsuccessful:
This is a bigger refactoring of com_fields to get rid of the use of com_categories for its field groups (fieldset) feature.
During the rewrite I found also several code pieces which are unused or were copy-pasted from com_categories but don't make sense in the context of fields.
I also replaced the useage of some deprecated methods.
So forgive me that it become a bit bigger than initially planned
If there already is documentation how to implement com_fields into an extension, that will need to be adjusted.
Status | New | ⇒ | Pending |
Category | ⇒ | SQL Administration com_admin Postgresql com_categories com_contact com_content com_fields |
Milestone |
Added: |
Looking closer
I see this dropdown everywhere now. Before you only had it on the two context in mail
That in itself is NOT a problem
the problem is that when you switch context to a different component you dont really notice as the only thing that is changing on the screen is the title which is ABOVE and to the left of the select. From a UI perspective this is outside of your field of vision and its confusing
Actually that is intented behaviour, but something which can be discussed if we want it or not.
The dropdown would allow to switch between the extensions that support custom fields. It takes the contexts from the extensions helper method. The sidebar automatically adjusts to the selected extension.
The URL doesn't need the context/extension anymore after it is initially set. So if you click the link in the sidebar, you see the extension/context in the URL. It will then set the userstate and from there on it's not needed anymore. If you use the dropdown, the extension/context will be sent in the form (POST) and not in the URL (GET). So that is fine.
the problem is that when you switch context to a different component you dont really notice as the only thing that is changing on the screen is the title which is ABOVE and to the left of the select. From a UI perspective this is outside of your field of vision and its confusing
The sidebar also changes, but I agree it may not be noticed.
It also prevents someone from direct linking to a field menu (unless you understand the secret urls)
Direct links should work imho. The sidebar and menu links doen't do anything else and they work fine as far as I tested.
Not going to argue about the confusing part. I'm not good with UIs anyway.
I just kind of liked the possibility to switch fast between the extension and thought I add it. It's simple to remove again if we don't want it. We just have to adjust the formfield.
It would also be simple to adjust the options eg to "Contact -> Mail" instead of just "Mail" if that helps.
Direct links should work imho.
they will if you copy the one that has the full context in. They wont if you use the url you see when you use the switcher as seen in the video
Yep, that's true. Not sure how important it is. It may be a bit tricky to do and likely involves JavaScript if you want to change the URL from a select. I think in current staging it's done using JS.
IMO it will be confusing for the end user to have all of the sudden an option to switch between components. In Joomla we don't have such functionality nowhere, so I would not introduce it. I would also do it the way that when there is only one context in the component, that the field will be hidden at all.
In Joomla we don't have such functionality nowhere, so I would not introduce it.
To be fair, the only place where we currently have a similar cross-extension extension is com_categories. That one doesn't have such a select, that's true.
But that's not really an argument to me since we're only talking about one possible place.
The Google project for translating/associating would be another such extension and that one could use such a selector.
You could look at it as a required option to filter fields. Like the search filters but not optional.
But if all think it is to confusing I have no problem removing it (or make it an option - which brings up another topic fast g)
I would remove it.
I can't find a use case where you are working on custom fields for articles and then you need to switch to the custom fields of users.
I can think of use cases when you're setting up a site.
But please this PR isn't about this single select (I already said I can remove it if all don't like it).
Much more important would be to test the function of the PR. Eg that fields and groups still work as expected and also ACL which should now work properly with inheritance from component -> groups -> fields.
Removed the switcher from groups view and only show it in fields view when there is more than one context.
Also wrong context com_content.article on attempting to create a field under com_users.
Yep, I am now able to create fields under com_users, and apart from the "field deleted" message, the rest appears to be fixed as well.
Fixed the message as well.
Also found another wrong redirect which is now fixed.
Looks like there is a permissions issue on the Field Groups views. When logged in as a Manager I get a "You are not authorised to view this resource," regardless of ACL settings.
Batch processing should work as well now.
Could someone explain to me the usage of the language field, both in fields and fieldgroup?
My test show here (simple text field for article) that, on a multilingual site, tagging the field to a specific language (example fr-FR) displays the field in the article edit form both when the article is tagged to the same language (fr-FR) and when the article language is set to "All".
I think the field will only display if the language of the field matches the site language. Not related to the language of the article.
But I haven't changed anything in regards to that and I honestly have no clue if it makes sense to have that feature or not.
I guess in fieldgroups it is not necessary. If the article you are editing belongs to a language, then the fields are only displayed which belong to the same language.
Then we do have a bug as it also displays when language is set to ALL for the article when field is set to fr-FR.
EDITED
Should be or not? If a field should shown on all languages and an article as language fr-FR then this field should also be loaded, I guess or I'm wrong?
THe case I describe is exactly the opposite
The field is set to a specific language and the article is set to ALL
Here is the line which defines it https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_fields/helpers/fields.php#L86. If the language is set to all on the article then all fields are loaded, which is right IMO. But you know better than me how the multilangual behavior should be.
Personally, I'd question the whole use of languages on the field and field groups.
Is there even a need for that or does it only complicate code and confuse users? It lookes like it even confuses me and JM already
After all, you can already filter fields by category. Eg they only show if the article belongs to a specific category. On a multilingual site chances are high the category is language specific as well.
On a multilingual site chances are high the category is language specific
as well.
High but not required. It depends on how you build the site
High but not required. It depends on how you build the site
True. do you think there is a need for a language field on the field (and the groups)? If so, what would be a possible use case?
Just so we know how the code should work, because imho it's not that clear. The language filter could either depend on the language of the item (eg article) or depend on site language. Currently it is depending on the item language (if I understood @laoneo correct). But that becomes interesting in the edit form since you can change the language of the item there.
True. do you think there is a need for a language field on the field (and
the groups)? If so, what would be a possible use case?
Scenario 1
French Music group - this group is only for french language content
Scenario 2
Music group - with a translated field Chanson (fr) Song (en) Canzone (it)
and all other fields are suitable for all
We can always have something like #11040 and overcome the client side changes...
Potentially reloading all custom fields when the language changes doesn't sound like good UX to me. But yes, it is likely the only solution.
Music group - with a translated field Chanson (fr) Song (en) Canzone (it)
I had that in mind as well, but imho it is already possible using a translated label.
Except if the field value would be different as well on each language, but then how are you going to enter all three translations? The fields would all have to be visible in the form then so you can enter the various values.
I think that's likely something which should be discussed in a separate Issue.
If the language is set to all on the article then all fields are loaded, which is right IMO.
I don't think so. Only fields set to ALL languages should display when an article is set to ALL. Fields set to ALL languages should also display for any article, whatever the language.
On a multilingual site chances are high the category is language specific as well.
High but not required. It depends on how you build the site
If one builds a site with items set to a specific language, then the hierarchy above that item (category, menu item) must OR be tagged to the same language OR to ALL languages.
One could do for example (but this is vicious and can lead to confusion...):
ROOT (set to ALL)
-Sub category 1 (set to ALL)
------> items set to ALL
------> items set to xx-XX
------> items set to yy-YY
--Sub-sub category1 (set to xx-XX)
-------> items set to ALL
-------> items set to xx-XX
--Sub-sub category2 (set to yy-YY)
-------> items set to ALL
------> items set to yy-YY
-Sub category 2 (set to yy-YY)
------> items set to yy-YY
------> items set to ALL
Etc.
In the case above, indeed, a field set to ALL and through the Sub category 1 (set to ALL)
should display in all items down the hierarchy.
If the field is set to xx-XX and to be displayed in the same Sub category 1 (set to ALL)
, it should display only when the items are set xx-XX.
In any case, we may have here indeed, as @Bakual says, an unecessary complexity.
I would also not discus the language set up in a separate issue to stay focused here on the group implementation.
Ah crap, didn't spot that. I'll see if I can add it somewhere without changing global JLayout files.
My pleasure. I'll take another look tonight.
I would also not discus the language set up in a separate issue to stay focused here on the group implementation.
Agree, but it has an impact though as batch functions use language as parameter and do not use Groups and /or Categories
When the user has the edit permission on the article but not the edit value permission on the field, I could change the value of the field.
When I tested the ACL this morning I used the editor group which by default has edit permissions on the articles. I was able to allow and restrict the ability to edit the value based on groups and fields as expected.
Did you try with a new field or with an existing one from before this PR? Because that could make a difference since the asset keys are different now (new is eg 'com_content.field.1`)
I see. However that sounds like a bug in core to me. Technically it's loaded in the wrong JLayout (language instead of item). I guess one was lazy and didn't want to add it to the item, position and menu JLayouts but it's still wrong imho.
At least it should check if it is defined or not.
I have tested this item
I have tested this item
I've tested this twice now on two fresh J3.7-dev clones with the JPatch Tester, all tests fail.
After applying patch I go to the JDatabase checker and apply fixes.
Then viewing Articles -> Fields we have no sidebar: https://www.dropbox.com/s/lttxhn376k75nqi/Screen%20Shot%202016-12-02%20at%2022.13.02.jpg?dl=0
URL was:
http://localhost/jpatch/administrator/index.php?option=com_fields&context=com_content.article
Then clicking Field Groups
We get:
https://www.dropbox.com/s/bwiepkda43793v6/Screen%20Shot%202016-12-02%20at%2022.15.11.jpg?dl=0
with the following url:
http://localhost/jpatch/administrator/index.php?option=com_fields&view=groups&extension=com_content
Also, the Joomla! Admin menu items to field groups is still linking to com_categories.
Hmm, that sounds like an issue with the patchtester then. It certainly works right here with the branch checked out.
I'll try it that way ;-)
Ok @Bakual yep you were right, I monitored the changes the JPatch tester applies and it's only 18 files.. only another 30 missing.
I've checked it out manually and it functions as intended now. A few points:
I0'll have a look at those issues later. It's very likely not related to this PR but should be easy to fix.
@tonypartridge Fixed now.
I have tested this item
Great work @Bakual all working great now and messages are now spot on to me.
I have tested this item
Status | Pending | ⇒ | Ready to Commit |
RTC
Status | Ready to Commit | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-12-05 09:16:43 |
Closed_By | ⇒ | rdeutz |
Isn't there an open issue somewhere for this?
While I'm here, I'd say the way it is now is fine. It's also consistent with how categories are handled for extensions supporting those. Or are you saying you'd rather those all moved in favor of a categories item too?
Fields is integrated on a per-extension basis. For me that means I'd look for it with the extension.
For me it clutters the menu and doesn´t leave space for future improvements. Like that it would be isolated as an own Area to build custom forms for any extension that supports it...
You might be able to pull it off with your own custom admin menu. But the way core handles things, it's too complex to do that by default.
I honestly see the number of top level menus as clutter and adding one more makes it worse.
This should be discussed in an own issue, not in a merged PR. I just linked it because it was discussed here already before the current state got merged.
adding one more makes it worse.
The issue arose because there is another PR now which may raise those field links in the sidebar from currently two to three. It's not one single link like for categories.
Quick comment on fields in com_contact
There is a dropdown (as before) but it is showing more than it should - see screenshot
I dont believe it is supposed to be showing articles and users?
Also by default you open with contact as the context and a url of
when you switch the context to mail in the select you get a url of
I expected