Refactoring JFormField & Co.

[Donald Gilbert]

I spent this weekend refactoring the JFormField* set of classes, and came up with this - https://github.com/joomla/joomla-platform/pull/1684

I'm posting it here so I can get a broad sense of what the community thinks of it, rather than expecting everyone to know about / be on github in order to comment. I would prefer most of the conversation to happen on github, but if you don't have an account, we can discuss it here instead.

[Donald Gilbert]

Anyone care to chime in on this?

[Louis Landry]

I care about it, but honestly between now and the world conference I don't think I'm going to have much (if any) time to put towards code review. Hopefully others have thoughts.

[Don]

Forgot about JWC next week, since I'm not going. :(

I'll sit tight 'tip then. Thanks  :)

Sent from my iPhone

[Mario]

I'll be looking on it today and thanks for the weekend effort Donald.

Mario

[Mario]

I've tested and it was a great work! Added a couple of notes, one to correct a small bug in textarea typecast to (string). 

The password field, imho, could benefit from the use of the placeholder. I've added the code (same as in text type) and looks good.

The groupdelist type, that have search capabilities, might use the placeholder to, although, imho, I don't think there's a great adavntage in this case, so keeping the placeholder attr out, looks fine.

Elin has mention the editor possibility, to use the placeholder, although not sure at what level this could be made.

Mario 

On Tuesday, November 13, 2012 5:08:26 AM UTC, Donald Gilbert wrote:

[Donald Gilbert]

Grouped list with search - do you mean the new CMS 3 Chosen select / live filter select fields?

If so, I see your point how it could be useful. However, since placeholder is not a valid attribute on select lists, I'm not sure if we should reuse that property for this scenario, or create a new property that can be set. Maybe "chosen_placeholder" or something.

I for one don't think that every select list in the CMS should have that feature - sometimes, it just plain isn't needed. I would almost prefer there to be another field type, called JFormFieldChosenList or something similar, that, when called, would style it accordingly. The other select lists should appear the same, but not have the search functionality built in. Like, If a select list has less than 5-6 items, it should NOT have the live search, IMO.

[Mario]

Yes, correct, the new CMS 3 Chosen. But as I mentioned, imho, I don't see a great advantage on these fields having the placeholder. I just threw the point to check if someone could see an advantage. Even the little magnifier icon on the right is quite self explanatory, so adding "more lights to the christmas tree" doesn't seem to make it prettier. ;) 

The second point you mention, for the limit on when to use or not the search capability - if, as you mention, have less tha 5-6 items, do you really feel that adding a couple of more lines of code to enable/disabale that "harmless" functionality will be beneficial? Imho, I would let it stay as it is, but if you see that as condition to be implemented, I presume you'll have no opposition on that change.

Mario

[Gary Mort]

I don't really like the JFormField classes to begin with, so I really can't comment.  My main problem is:

JHtml and JFormField often have redudant, or worse, conflicting HTML markup.  I'd like to see all getInput methods end with calls to JHtml like JFormFieldCalendar does.  In any case, anything that moves towards more standards is good.

Moreover, I don't like how JHtml works in that the html is embedded in the class itself.  If you want to change the layout fields in a module for a specific template you have to do a lot of manual coding - wheras instead if the JHtml* classes extended the JView class they could load html template files directly from the template or defaulting to their own directory just as modules and components do today/

However, that being said, code talks and BS walks - so since you have some actual code here I'd go with the actual code over "what if"

[Mario]

Don, since you've been working on the JFormField and specially in "& Co.", you probably have a clearer and fresher idea about were to find a possible bug in the system that seems to exist with the fields that have the attribute multiple="true". Take for instance a field <field name="field_name" type="UserGroup" multiple="true" (...) />. Here's what I've found so far:

1. In the first access to the form, loading data and saving data, even if there's an empty multiple field, have no issues. 

2. Access the form again and populate the multiple with some data.

3. Saving is correct, with no errors and the database is populated with the correct data.

4. Edit this same id and and change the data in the multiple, so that some data stays there to be saved.

5. Saving / update is correct, with no errors and the database is populated with the correct data.
6. Edit again the same multiple field and remove all data from the multiple and save.

7. The system doesn't update the empty multiple field in thee database. More, in $_POST, the field_name is absent.

Imho, this seems to be a bug somewhere in the system that is not contemplating the possibility of empty multiple fields to be updated to null.

Thanks

Mario

[Elin Waring]

Checkboxes that are completely empty are not posted. That is part of the W3C definition. This is why a "none of the above" option is so useful in a lot of situations where you have checkboxes. Since userGroup is a checkbox it's not going to work. Specifically on userGroup it would be improper to have a user with no groups anyway. 

In terms of saving of multiple data this is something that we have had several long threads about. Some argue that it's up to the developer to implement saving in the way that is most useful for a given use case. Others (like me) say that we should have a standard default or even better build in an optional attribute to save as: a string of comma separate values, an array or a JSON string.

Elin

[Mario]

Elin,

I'm trying to understand when you say "userGroup is a checkbox", because in fact, in code, it's a multi-select styled with Chozen (just to confirm we are talking about the same field, I've attached an image as example). 

"multiple" is a property that can be used in several fields, as an example, in sql type field, allowing to choose more than one option for the field loaded from the db. Imho, any field that is available to be saved to database, should be enabled to be updated, independently of being empty and to let the database to check if the field is allowed to be null, otherwise it becomes sticky - you can change it but you can't reset it, which, imo, doesn't make sense, even checkboxes. Check is 1 uncheck is 0. If you say that checkboxess aren't posted if empty, how can you update the 0?

The multiple is already working in the system using JSON. And it's working 99% well and doing a great job. The question, or the 1% missing, is the update to null, which isn't being done by the system.

An example of multiple in sql type field that works (those 99%) - The field loads options from the database and allows the user to select multiple fields, and change his selected options when needed. But if the one decides to reset the field to null and use none of the options, he can't. 

So we already have that field you mention, the only question is that it doesn't allow a reset, or in other words, doesn't allow to save empty to update the database.

Mario 

[Elin Waring]

Hmm a change from the UI for 2.5.

Have you read the W3C standard?

http://www.w3.org/MarkUp/html-spec/html-spec_8.html

Multiple isn't even valid attribute for checkboxes html. They must be multiple (hence we had forceMultiple to manage them)

A totally blank checkboxes set is NOT successful which means it is not posted. You may not agree with that, but it is what the standard is. If you want to do it differently then you need to extend the field to behave in a non standards compliant way.

http://www.w3.org/TR/html401/interact/forms.html

checkboxes

Checkboxes (and radio buttons) are on/off switches that may be toggled by the user. A switch is "on" when the control element's checked attribute is set. When a form is submitted, only "on" checkbox controls can become successful.

Several checkboxes in a form may share the same control name. Thus, for example, checkboxes allow users to select several values for the same property. The INPUTelement is used to create a checkbox control.

In fact multiple  works sometimes and not others and that is well described in a series of threads on this list. If you have a multiple inside of fields it works great as json but when it's in just a plain field you need to do some work. Usergroup is an example where the saving format has been specified.

Elin

[Mario]

Elin,

Thank you for your reply and I'm not going to insist in this subject, but just to say that nobody who codes to interact with databases can agree with the W3C for this. If they were totally correct, and if we "must" blindly follow W3C, not taking into account that they may be wrong or not seeing the whole picture, then we will just keep following a path that doesn't add value to our software, be it Joomla!, iOS or any other, without even breaking the so called rules.

In any field that contemplates the use of the multiple parameter, even in the userGroups, I personally will continue to make use of it, extracting all its great capabilities that they may offer, and not adding more than a simple line of code to allow them to completely interact with the database. I could give a handful examples of their possible usage outside the scope for what they were initially built. Well, that was what I thought by posting it here, as a possible contribution for a more effective usage of what is already greatly done in the platform.

Thanks again

Mario

[Elin Waring]

Look I don't disagree in principle, but the fact is that all we can do in a project like this is really code baseline to standard and then possibly allow a non standard option.  I actually started writing a post about that possibility when I was working on the multiple save/multiple checked problems in checkboxes.  In survey research (which I do) it is very important to be able to distinguish "did not answer" from "did not select any of the choices offered."  That's a slightly different issue because you only save once, but it's basically impossible to do that in a standards compliant way without offering the "none" option.   Even the baseline percentages are inherently wrong unless you do some dubious assumptions about what an empty checkbox array means i.e. that it means that none of the options applied to the person (that's what Google does with checkboxes data in Google forms for example).  For example we could do like some statistical packages and reserve a value of  '.' (or something else) to mean saved the form but did not provide values for this item and leave null to be didn't complete the form either.

Elin

[Mario]

Why updating empty text or textarea fields if they have no content? Because, as everyone well knows (even the W3C), we need to tell the form processing system to update the correspondent database field with null, meaning, we no longer want data in this field. The same should be happening with the multiple select fields, if the system doesn't ignore them if they are empty and only looks at it when they have positive value. Imagine this in a text field... you could never update an empty text field after you have added some content the first time. The absence of evidence, is not evidence of absence, so the form processing system, should, imo, look at all the fields actually present in a form, despite their values, and act accordingly. 

[Elin Waring]

Text boxes etc are not booleans.

I think the thinking is that in boolean world nothing is the same as false.  And actively unchecking things is no false but is really doing something so you should not be attempting to post a false in that case you should be posting something like "none" or "." or some other value that is boolean true.

 

So I'm thinking that you do agree with me that we could perhaps add an attribute to the field types like boolean, checkboxes and checkbox that don't succeed when empty  setting a value to be posted if nothing is selected but the form is saved. 

Whether that value should actually be saved is  a somewhat separate question i.e. it could be an attribute as well.  

Of course any one can just use a field in this way by adding an attribute in code or extending the field in question. 

Elin

[Mario]

Yes, I agree with you and perhaps adding to the booleans fields an optional attribute like updateNulls="true", would perhaps do the work, and save some extra coding, without the need to extend the field each time a boolean is used. In this possible scenario, the developer would have a simple way to control the output.

Mario

[Gary Mort]

On Wednesday, November 21, 2012 10:37:39 PM UTC-5, Elin Waring wrote:

Look I don't disagree in principle, but the fact is that all we can do in a project like this is really code baseline to standard and then possibly allow a non standard option.  I actually started writing a post about that possibility when I was working on the multiple save/multiple checked problems in checkboxes.  In survey research (which I do) it is very important to be able to distinguish "did not answer" from "did not select any of the choices offered."  That's a slightly different issue because you only save once, but it's basically impossible to do that in a standards compliant way without offering the "none" option.  

I'm a bit confused here...the standards you pointed out refer to how the BROWSER should treat checkbox submissions.

The sample given on the website is quite accurate in that you have the checkbox:

name=flavor value=vanilla

name=flavor value=strawberry
name=flavor value=chocolate

So, assuming one checkbox is checked, then you will have

flavor=vanilla

Wheras if 2 checkboxes are selected you will have

flavor=vanilla&flavor=strawberry

An array of values which can then be handled server side.

Wheras if NO checkboxes are selected, then you have...well, nothing.

The standard does not dictate how the SERVER is required to handle the input, so the server can certainly use the lack of any value posted to update the data can't it?