fix up ListboxField

[sunnysideup]

fix doc information on the __construct method
allow SS_Map as source (just like parent class DropdownField)
I am wondering if the set / get source method could be simpler - more like the dropdown field...

[kinglozzer]

@sunnysideup Do you have an example setup that failed prior to this change? I just tested and ListboxField already seems to work with an SS_Map source

[sunnysideup]

Hi @kinglozzer , look at getSource, it checks for commas in arrays. That gave me an error.

[sunnysideup]

my pull request needs work, but I do think it needs fixing. I will make another one.

[kinglozzer]

Sorry, @sunnysideup, you’re quite right: for some reason I had warnings suppressed in my dev environment! 😳

What do you think of the following approach (casting $source to an array)?

$hasCommas = array_filter(
    array_keys((array) $source),
    create_function('$key', 'return strpos($key, ",") !== FALSE;')
);

By doing it this way, the original $source variable will stay as an SS_Map. My thinking here is that if you call setSource() with an SS_Map, then call getSource() you might not expect it to have been converted to an array.

Given that the parent class accepts either an SS_Map or an array, I’d argue that this is a bug so you could make your pull request against the 3.1 branch instead of master. You could also fix the docs for DropdownField::__construct(), DropdownField::setSource() and DropdownField::getSource() as none of them mentions SS_Map even though it’s supported.

[dhensby]

Please can we get a test to prove this bug?

[stevie-mayhew]

Its doesn't strictly have to be an SS_Map, just something with ArrayAccess - https://github.com/silverstripe/silverstripe-framework/blob/master/forms/DropdownField.php#L122

[dhensby]

closing due to age and because the root cause of this problem will be resolved by having a more consistent API with SS_List

[tractorcow]

Also this field would have been changed when I did my listfield redux. ;)