User tests: Successful: Unsuccessful:
Currently, com_fields uses an alias field as a name for the field in forms. But that alias really isn't used for anything else and can as well be substituted with the id, making the code actually simpler.
Removes the alias from fields and changes its use to use the ID instead.
To avoid conflicts in general, fields are now stored in an own group called "com_fields" instead of the "params" that is used already by most extensions.
Test creating, editing, deleting fields both in backend and frontend.
Test for fields in articles, users and contacts.
None.
Status | New | ⇒ | Pending |
Category | ⇒ | SQL Administration com_admin Postgresql com_fields Front End com_contact com_content com_users Installation Layout Libraries Plugins |
Yes indeed it is simpler, but there are several advantages when using aliases
Most notable are:
The alias is only used in the form, not when the field is displayed in a regular view. So it would only be useful for customising the edit views.
There is also already a class that can be set per field. Both independent for the form and for the regular view. I'd think that class is much more useful for customising.
The alias is only used in the form, not when the field is displayed in a regular view. So it would only be useful for customising the edit views.
right, i see
so either it should be used more or be removed as this PR does
I don't see the benefit really. The name of the field in the outputed HTML is now jform_2, which is hard for the site admin to imagine.
I like the change to move the fields away from the params fields. I would put that into a separate PR to not mix stuff and keep this one smaller.
I don't see the benefit really.
Main benefit is to remove a field that isn't needed at all. And to remove confusion as to what an "alias" does for fields. Because an alias is used for URL routing in all other forms, which makes no sense at all when it comes to fields (we don't have URLs for fields). So its named alias but does something completely different.
So it either should be removed or renamed to "field-id/name in forms". But the latter doesn't make sense to me as well. What is the benefit here when we already can add custom classes for styling?
Also, you would have to make sure each field name is unique in a form, or you get invalid HTML markup. That's simple with this PR since each ID is unqiue by its definition.
The name of the field in the outputed HTML is now jform_2, which is hard for the site admin to imagine.
It is jform_com_fields_2
for the ID and jform[com_fields][2]
for the name.
And really, why does a site admin even care about the id/name of a field in an edit form?
I like the change to move the fields away from the params fields. I would put that into a separate PR to not mix stuff and keep this one smaller.
The move away from the params was a requirement for removing the alias. I saw that during writing the PR. That is the sole reason it is in this PR and not separate. We can split it of course to make reviewing a bit simpler, but it's the same thing to test in the end.
Some users of DPFields were working with the alias in template overrides, eg. @marcodings https://www.youtube.com/watch?v=fgDoZsD71H8&t=1652s
When we rewind the content parser feature joomla-projects/custom-fields#135 . Then it would mean that {{fields alias=avatar}}
will not work anymore as well.
There was a reason that we introduced it in DPFields, so taking away all the stuff which is technically not obvious on the first sight makes for me no sense. You are right, the naming is wrong, but I would not remove it.
The thing is, if we now introduce stuff which isn't needed currently, we add technical debt that can't be changed later due to backward compatibility. It is simple to add a field (alias, name, whatever) again later if we ever introduce such a content parser feature. However that content parser would obivously work fine also with the ID of the field (as well as template overrides).
Same for the other fields that are of no use currently. We should not add stuff now just because it may or may not be supported later.
Imho we should keep it to the minimum we currently need and support and can be tested, and expand it later if we add the corresponding feature.
I think for a custom field which represents a JFormField it makes sense to have a form name (alias) attribute. It is in use right now as form name, just has the wrong label.
So we would have to rename it so it's clear what it does. Because currently it sure isn't clear and confusing.
However I still don't see the usecase where you would want to use that name/id for something. And especially not why that couldn't be done with the ID as well. Imho we're creating a lot of UI and code overhead for no gain.
Much more important than the formfield name would imho be the ID of the field when shown in frontend. Currently it shows there with an ID eg "field-entry-2". But if you think about this, this is already highly problematic. The field should have no ID at all. Because as soon as you want to show the fields on a page where multiple items are shown (eg one in a module and in component, or in a list), it will produce duplicate IDs on the page and thus rendering the HTML invalid. That doesn't matter if it's an alias or id that is used here. But that certainly is a different PR to remove that.
Thomas the definition of what is not used or is used is very subjective. We use (dp-)fields extensively and we use for example overrides and tags to assign fields to content.
Whilst i do agree we should be carefull about running up technical debt we should be equally reluctant in stripping out stuff just because as an individual we don't see the use case.
The stripping of tags will make it impossible for my to migrate from dp-fields to fields. Removing the ability to override fields, which as i understand it to be a consequence of this pr would be equally detremental.
Sure anything can be added later as bespoke code, however the whole point, to me was to have decent support from core
Thomas the definition of what is not used or is used is very subjective.
Of course I understand that. If someone can explain me why the need for an alias is there, then I'm fine with that.
I'm not sure I understand what you mean with "tags" and "overrides". This PR doesn't touch anything like that.
What it does is changing the name of the field in the item (eg article) form from using the alias to use directly the ID. So making the formfield name jform[com_fields][1]
instead of jform[com_fields][alias]
.
As you may noticed it also changes the field group from jform[params][1]
to jform[com_fields][1]
.
I don't see how that would change the ability to override the field or assign it to content.
Keep also in mind that this only changes something for the edit forms (eg article edit). For regular views like a single article page, the fields will behave exactly as before as the alias isn't used at all there.
Labels |
Added:
?
|
I have tested this item
Test OK
Using ids as form field names, and when creating custom layouts is a little weird and problematic for portatibilty, imagine i want to copy i custom layout to another web-site i will have to remap the ids
about form field name, to give a comparison imagine:
article (/record) 'title' was field '1' and
article (/record) 'description' was was field '2'
I agree 'alias' term here is not proper, just use another term like 'name'
I obviously disagree
Using ids as form field names, and when creating custom layouts is a little weird and problematic for portatibilty, imagine i want to copy i custom layout to another web-site i will have to remap the ids
First: We're only speaking about forms, not where the field is displayed in eg an article view. In that view it already uses the ID and not the alias. The alias is only ever used in a form generated by JForm. Are you really going to customise that one somehow? And why are you needing the id/name then and not take the class?
Second: You're exchanging one possible issue with another one. If the user is going to adjust the alias, it would break the custom layout as well. Personally I'd rather do the layout on something I'm sure the user doesn't accidentally change.
about form field name, to give a comparison imagine:
article (/record) 'title' was field '1' and
article (/record) 'description' was was field '2'
Imho it's not the same case as in a record the developer knows for sure how each field is named and the name has to relate to table fields, In our case, they are user generated and it doesn't really matter at all how they are named. Actually using the ID does again map to the database entry and makes the code much simpler.
A use case to use names in template overrides can be found here https://www.youtube.com/watch?v=fgDoZsD71H8&t=1652s. As I said already quite a bit of DPField users, did use the fields based on their alias, like
foreach ($item->fields as $field) {
if ($field->alias == 'avatar') // do the work
}
They used then that layout among different sites and didn't had to mess with ID's. I understand you, that it technically makes sense to remove the alias. But I think we need to hear here the end user as well.
Please don't link to the video if you don't give the exact poisition where he explains the use case. I find it very hard to follow Q/A type of records to find the answer you're referring
As I said already quite a bit of DPField users, did use the fields based on their alias
That code wouldn't work because the alias isn't available in the formfield. They need to use $form->id (eg jform_params_alias) or $form->name (eg jform[params][alias]).
DPField users will likely need to adjust their overrides anyway (since we're likely changing the params
part to com_fields
).
Still I don't see why those overrides couldn't be achieved as well using the edit class
property. Or if it for some reason has to be something unique per field and you don't want to add an arbitary unique class there, we could talk about adding the title property to the formfield so that one could be read for such code.
Sorry tought the t parameter was correct, but it always started from where I left. Here is the correct link https://www.youtube.com/watch?v=fgDoZsD71H8&t=26m5s.
Ah I see it now, but that is about frontend views, not form. The alias isn't used here for anything at all, it would just serve as a way to distinguish fields in custom overrides, but there are several other properties that can be used to achieve the same.
In eg an article view you could as well just use the title instead of the alias to achieve the exact same thing. Or use the render class.
Title |
|
Title |
|
In eg an article view you could as well just use the title instead of the alias to achieve the exact same thing. Or use the render class.
Use a CSS class to reference a specific field ? that sounds a little strange
Drupal instead of "alias" or "name" calls it "Machine name", and looks like:
field_tags
field_color
and i understand why the "Machine", it is because this is not directly visible by site visitors / users
<select name="field_color" ...
Then besides using it layouts / custom layouts it is also used to create a field specific class (the "render" class we already have can be used by many fields)
So in to total the "alias" or "name" or "field name" or "machine name" is useful as:
About being uniqueness it should be on:
(context, alias)
Use a CSS class to reference a specific field ? that sounds a little strange
What is strange here? We do that all the time. Actually we use IDs very rarely in core.
The only time you should use an ID is when you're absolutely sure that ID is only shown once on a page. Or you have invalid HTML.
In a CMS you can only be sure of that when it comes to the template file (index.php) itself. Each content may be present multiple times on a page.
human readable and site-portable reference to a field (copy my layout file to other website)
You're aware we're still only talking about forms here. And I honestly doubt a lot of adjustments happen for those forms.
How the field is rendered in an article is a completely different thing and depends on the various layouts. Imho, it shouldn't use an ID at all here as this will break if you have the same field twice on the screen. Eg if someone wants to add fields to a list view.
create a friendly HTML TAG ID and / or friendly field specific CSS class (the "render" class would usually be not specific to a field)
As explained already, it's a bad idea to use the ID in most cases. For forms it may work because we usually don't have multiple instances of the same form open on the same page, but for rendered fields eg in an article it's a very bad idea to use IDs.
About being uniqueness it should be on:
(context, alias)
Alias has to be unique sidewide. Because even if you don't have the same form open twice, you may have different forms open on the same page (eg a login form in the module and a contact form in component). If both forms allow for custom fields and one field shares the same alias, then you already got a possible problem.
Use a CSS class to reference a specific field ? that sounds a little strange
What is strange here? We do that all the time.
We speak of the 'render' class parameter, right ?
or i understood you wrongly ?
-- because it is a parameter, thus DB queries cannot reference it,
-- because it is not unique,
-- because it is not required parameter
it's a bad idea to use the ID in most cases. For forms it may work
No using IDs does not work well, it makes working with forms unfriendly
You're aware we're still only talking about forms here
This is something that should change / can change with little effort,
-- It should also be usuable in viewing ('value rendering') layouts
(should be easy to index the fields of every record via their alias)
-- About CSS class that uses the alias (=field name),
this is really trivial to do, it can be added at the same place where 'render' class is added
About uniqueness, i see, maybe unique site wide, ok,
still i think it could do unique like this:
(context, alias)
And in places that it is needed,
e.g. to make HTML tag ID unique it can then be:
id="jform_custom_color_45" name="jform[custom][color]"
where 45 is the id of the field,
name=".....", only needs to be unique inside the form, and not in side the page or inside the iframe,
right ?
Whatever is decided, i explained some points in my above answers, about:
-- the usefulnes of keeping the alias and calling it "Field name"
A think we all agree, that alias is the wrong label. But I would not remove it.
We speak of the 'render' class parameter, right ?
No, the "edit class" (the one used in the form).
Yes using the table ID as ID attribute makes the formfields ID and name a bit less easy to identify when you want to write custom JS code (or CSS that relies on IDs). On the other hand the admin is not able to break that code by accidentally changing the alias.
Also, if that code is part of a custom formfield, then it's no issue at all. It's only an issue if you write that code somewhere in a template override. Chances are you're using development tools anyway and then it's just a copy-pasing of the ID. And honestly, are you really going to write JS in a layout override to adjust the behavior of a formfield? Shouldn't you better create your own custom formfield (which will be easy) and add the JS there? That sounds much more reusable to me than relying on the correct value of an alias property.
This is something that should change / can change with little effort,
-- It should also be usuable in viewing ('value rendering') layouts
(should be easy to index the fields of every record via their alias)
NO. When rendering (outside forms), the fields should not use any ID attribute at all because it will produce issues on a page as I explained multiple times already. So we actually need to remove the existing ids from the current output. But I'd like to keep this PR on the forms.
-- About CSS class that uses the alias (=field name),
this is really trivial to do, it can be added at the same place where 'render' class is added
What's the point in having both a "render class" plus an "alias" that is added to the class as well? Makes no sense to me.
name=".....", only needs to be unique inside the form, and not in side the page or inside the iframe,
right ?
Yes, but the way JForm works the ID and name is tied together. I don't think you can have different names there.
A think we all agree, that alias is the wrong label. But I would not remove it.
Yes, I think we all agree "alias" is a wrong label. I don't agree on the second part
Yo need to use the latest staging code as many things have changed since alpha1.
Category | SQL Administration com_admin Postgresql com_fields Front End com_contact com_content com_users Installation Layout Libraries Plugins | ⇒ | SQL Administration com_admin Postgresql com_fields Front End com_contact com_content com_users Installation Layout Plugins |
I've updated the branch and solved the conflicts. Hopefully I didn't miss something.
I tested this patch today.
I created a lot of field and applied this to categories and field groups. In the backend everything works fine. It was possible for me to create articles. All fields were showing in the correct groups and categories. I could edit this fields in the backend. And - if I forgot to fill a required field - I get a message.
But it was not possible for me to save changes in the frondend. I could edit the fields and when I save I get the message that everything is OK. But the changes have not been saved. I saw the unchanged field values. I made this changes with the super admin.
But it was not possible for me to save changes in the frondend. I could edit the fields and when I save I get the message that everything is OK. But the changes have not been saved. I saw the unchanged field values. I made this changes with the super admin.
@astridx It should be fixed with #13539. Can you make sure you have the latest staging in your installation? It was fixed only yesterday.
I have tested this patch successfully.
Sorry, you're right. I'm sure that I used the command
"Git pull upstream staging"
this morning.
Probably I have made a typo.
Now the saving in the frondend is OK too
I also updated the branch of this PR today. Maybe that fixed it as well. Patchtester only copies the files, it doesn't really merge them. So there may be some issues like that due to its workings.
Can you log a successfull test then in JIssues?
I have tested this item
I have tested this item
Test with Joomla! 3.7.0-alpha2-nightly, macOS Sierra, 10.12.2, Firefox 50.1.0, PHP 7.0.4, MySQLi 5.5.53-0
@franz-wohlkoenig That happens with and without this PR, right? At least I can reproduce it in clean staging as well. So it would be unrelated to this PR and a different issue needs to be opened.
I'd say the reason is because the label is shown even if there is no value saved for the field. In other places, we just don't show the whole field in that case.
@franz-wohlkoenig
Do you have the latest version of staging?
As you can read in this conversaiton I had the same problem earlier.
Testing on Joomla! 3.7.0-alpha2-nightly.
The fix for the frontend issue has been merged 3 days ago. If your nightly is older, then it explains that behavior.
@franz-wohlkoenig See #13589 for the misaligned values.
I have tested this item
Test OK
@franz-wohlkoenig You were right, something was wrong there again. I have no rebased the branch with current staging and now it should work again.
I have tested this item
Works with "User" and "Article".
In "Contact"
Please always test if the issues found are because of this PR or also in current staging.
I have tested this item
I still don't understand the need for it and I think it will produce more issues than advantages.
I'll leave that to the team to decide.
I agree that if this PR doesn't get accepted, the other one is the right approach and needs to go in.
There are three people just here who say, they need that field. So why then remove something which is needed by some?
So why then remove something which is needed by some?
Because they can do the same with IDs, or they didn't understand what this PR is about (this is about forms, not item display). And we otherwise make the code more complex for no gain.
I still don't believe anyone will do any template overrides for eg an article form specific to some custom field and then reuse that across other sites. And even if they do, it's still dead simple to adjust the number in the layout to the ned ID of the field.
On the other hand those same layouts suddenly all break as soon as an admin changes the name of the field, which I try to avoid here.
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2017-02-02 20:38:48 |
Closed_By | ⇒ | Bakual |
@laoneo This should also fix #13179 because it just removes the "generateNewTitle" method completely. Imho the fields don't need to automatically change the title with a batch copy and thus I removed it.