StoutLogic / acf-builder

An Advanced Custom Field Configuration Builder
GNU General Public License v2.0
794 stars 62 forks source link

Add choices tests with numeric values as keys #127

Closed nlemoine closed 4 years ago

nlemoine commented 4 years ago

Hi @stevep

I stumbled upon an issue with choices. Basically, I have a list a where the value is an ID (as string) and the label is string.

$choices = [
    '1' => 'Label 1',
    '8' => 'Label 8',
];

When adding those choices, I ended up with labels as values.

<option value="Label 1">Label 1</option>

After digging a bit, I discovered that PHP casts keys when looping over an array. More on this: https://stackoverflow.com/questions/4100488/a-numeric-string-as-array-key-in-php

That's why Symfony changed the way they handle choices and use keys as labels (something I didn't understand until now): https://github.com/symfony/symfony/issues/19953

I didn't find a way to fix this without introducing a breaking change (choices formatted like Symfony does). That's why this PR only contains the test for this case.

The only workaround I found is to prevent the use of choices or addChoices with addChoice:

foreach ($choices as $id => $label) {
    $builder->getField('select_field')->addChoice($id, $label);
}

Let me know what you think.

curtisbelt commented 4 years ago

@nlemoine hey there, I had this same issue a while back - check out the answer here: https://github.com/StoutLogic/acf-builder/issues/83

TLDR:

$choices = [
-    '1' => 'Label 1',
-    '8' => 'Label 8',
+    ['1' => 'Label 1'],
+    ['8' => 'Label 8'],
];
nlemoine commented 4 years ago

Whoops, sorry, I didn't check older issues before, usually do.

This might deserve a mention in the docs?