When using the multi select field the raw_value is an array with multiple values else if its a singluar value it's a string.
IMO it should be an array at all times, whereas at the moment to output the raw value I now need to write a check for array do this if not do this.
It would be better practice to be an array at all times given it can handle multiple values.
Labels |
Added:
?
|
Category | ⇒ | com_fields |
Isn't that the behavior of all formfields? If a single value is stored, it's stored as a string and if you have multiple values it's stored as an array of strings. That's not unique to com_fields, you have that anywhere you use JForm. Or not?
@AlexRed I don't beleive so, since @PhilETaylor 's PR is for fixing the output of a single value to the value opposed to a comma separated value.
This is still right in non-numeric values. But I am on about the rawvalue output for multiple selects being either string or array.
I would suggest we adjust the list field @Bakual and set it to array if multi-select is enabled for the field in question. Thoughts?
That would be a BC break. I'm wondering me where do you need the check you are describing in your issue description.
@laoneo Would it though... since if you are using multiple you will need to be checking already if it's an array or not. It would only be a BC break in my eyes if we applied it to lists globally. And not just if it's a multi-select?
Simple, in this scenario we are using the List for departments and if the Person is in multiple departments we select multiple, but it many cases it's a single department.
So it outputs a string for the singles and an array for multiples even though the field is set as a multi-select, we use the raw value to build a filter. Whilst I can explode, clean, lower, replace space with _ to get the same result it's not ideal since we have the rawvalue field there we are using that.
Status | New | ⇒ | Discussion |
If it comes now as string even tough multiselect is enabled and we are going to change that then it is a BC break in my eyes.
the problem is that you will need everywhere you are using the value an if condition to determine if it is a string or an array.
I suppose, maybe move to J4? But it doesn't make sense to return a string or an array on a multi select. It should be one or the other. Since every component using it then needs to code in a check.
On 22 May 2017, 10:40 +0100, Allon Moritz notifications@github.com, wrote:
If it comes now as string even tough multiselect is enabled and we are going to change that then it is a BC break in my eyes.
the problem is that you will need everywhere you are using the value an if condition to determine if it is a string or an array.—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub (#16177 (comment)), or mute the thread (https://github.com/notifications/unsubscribe-auth/ABVglv0s0ymYKGVv6U8QmtH5El2jvtFcks5r8VgWgaJpZM4Nh6Mo).
This value is ment to be used on an output, for example the image list is returning a string with<img/>
HTML code. According to your statement you would expect a list of image paths. That's why we have the raw value available.
If you need a kind of compiled values but not prepared for HTML output, then we need to think about a new attribute like compiledValue
. Changing value
can't be done imo.
If it comes now as string even tough multiselect is enabled and we are going to change that then it is a BC break in my eyes.
If the list is set up to allow multiple, the code already is able to deal with multiple values and thus an array. It's just the count of entries which would change.
As long as you only enforce the array when "multiple" is enabled, I think this should be fine to do.
I first understood that it should be always an array, also when multiple isn't selected. But that would be wrong.
Exactly my point
On 22 May 2017, 13:15 +0100, Thomas Hunziker notifications@github.com, wrote:
If it comes now as string even tough multiselect is enabled and we are going to change that then it is a BC break in my eyes.
If the list is set up to allow multiple, the code already is able to deal with multiple values and thus an array. It's just the count of entries which would change.
As long as you only enforce the array when "multiple" is enabled, I think this should be fine to do.I first understood that it should be always an array, also when multiple isn't selected. But that would be wrong.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub (#16177 (comment)), or mute the thread (https://github.com/notifications/unsubscribe-auth/ABVglidnu2GwsVKXB5Mc2wDbRactwN2gks5r8Xx8gaJpZM4Nh6Mo).
If the value attribute of a field changes within the J3 series from string to array, then it is a BC break IMO. Did I miss her something, but you want to change the value attribute of a field from string to array for multiselect lists, or? Means when I do then in my override echo $item->jcfields[2]->value
, it will print out a comma separated list and after your suggestions then Array
?
He talks about the rawvalue.
Exactly Thomas! Allon, I've kept mentioning rawvalue :). Value is very different, this outputs a readable output and doesn't need to be changed.
On 22 May 2017, 19:37 +0100, Thomas Hunziker notifications@github.com, wrote:
He talks about the rawvalue.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub (#16177 (comment)), or mute the thread (https://github.com/notifications/unsubscribe-auth/ABVglk8zdWtbw1bqXTvHME21CGqs5pyUks5r8dX2gaJpZM4Nh6Mo).
Ah, I tough that I miss something here. Sorry for the noise. Changing the raw value to always be an array is fine IMO.
@tonypartridge can you do a PR for this or shall I close it ?
Status | Discussion | ⇒ | Information Required |
If this Issue get no Response, it will be closed at 22th October 2017.
It is a valid issue that needs resolving by someone
Status | Information Required | ⇒ | Discussion |
I'm struggling for time at the moment, will see what I can do
Title |
|
Status | Discussion | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2018-03-24 22:45:57 |
Closed_By | ⇒ | tonypartridge |
We have a PR with discussion
is it related to https://issues.joomla.org/tracker/joomla-cms/16153 ?
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16177.