?
avatar tonypartridge
tonypartridge
22 May 2017

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.

avatar tonypartridge tonypartridge - open - 22 May 2017
avatar joomla-cms-bot joomla-cms-bot - change - 22 May 2017
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - labeled - 22 May 2017
avatar franz-wohlkoenig franz-wohlkoenig - change - 22 May 2017
Category com_fields
avatar AlexRed
AlexRed - comment - 22 May 2017
avatar Bakual
Bakual - comment - 22 May 2017

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?

avatar tonypartridge
tonypartridge - comment - 22 May 2017

@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?

avatar laoneo
laoneo - comment - 22 May 2017

That would be a BC break. I'm wondering me where do you need the check you are describing in your issue description.


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

avatar tonypartridge
tonypartridge - comment - 22 May 2017

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

avatar franz-wohlkoenig franz-wohlkoenig - change - 22 May 2017
Status New Discussion
avatar laoneo
laoneo - comment - 22 May 2017

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.

avatar tonypartridge
tonypartridge - comment - 22 May 2017

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

avatar laoneo
laoneo - comment - 22 May 2017

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.

avatar Bakual
Bakual - comment - 22 May 2017

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.

avatar tonypartridge
tonypartridge - comment - 22 May 2017

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

avatar laoneo
laoneo - comment - 22 May 2017

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?

avatar Bakual
Bakual - comment - 22 May 2017

He talks about the rawvalue.

avatar tonypartridge
tonypartridge - comment - 22 May 2017

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

avatar laoneo
laoneo - comment - 23 May 2017

Ah, I tough that I miss something here. Sorry for the noise. Changing the raw value to always be an array is fine IMO.

avatar brianteeman
brianteeman - comment - 20 Aug 2017

@tonypartridge can you do a PR for this or shall I close it ?

avatar franz-wohlkoenig franz-wohlkoenig - change - 20 Aug 2017
Status Discussion Information Required
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 17 Sep 2017

If this Issue get no Response, it will be closed at 22th October 2017.

avatar brianteeman
brianteeman - comment - 17 Sep 2017

It is a valid issue that needs resolving by someone

avatar franz-wohlkoenig franz-wohlkoenig - change - 17 Sep 2017
Status Information Required Discussion
avatar tonypartridge
tonypartridge - comment - 17 Sep 2017

I'm struggling for time at the moment, will see what I can do

avatar franz-wohlkoenig franz-wohlkoenig - change - 7 Nov 2017
Title
J3.7 com_fields list field with multi-select
[com_fields] list field with multi-select
avatar joomla-cms-bot joomla-cms-bot - edited - 7 Nov 2017
avatar tonypartridge tonypartridge - change - 24 Mar 2018
Status Discussion Closed
Closed_Date 0000-00-00 00:00:00 2018-03-24 22:45:57
Closed_By tonypartridge
avatar tonypartridge
tonypartridge - comment - 24 Mar 2018

We have a PR with discussion

avatar tonypartridge tonypartridge - close - 24 Mar 2018

Add a Comment

Login with GitHub to post a comment