feat: Add a feature to change country display format in form #95
Conversation
| @@ -128,6 +132,7 @@ public function configureOptions(OptionsResolver $resolver) | |||
| 'by_reference' => false, | |||
| 'error_bubbling' => false, | |||
| 'country_choices' => [], | |||
| 'country_display_type' => self::DISPLAY_COUNTRY_FULL, | |||
There was a problem hiding this comment.
This looks like a break, we currently use short format, isn't it?
There was a problem hiding this comment.
nop, currently we use "France (+33)" the "FULL" format
| @@ -61,12 +64,13 @@ public function buildForm(FormBuilderInterface $builder, array $options) | |||
|
|
|||
| $countryChoices = []; | |||
|
|
|||
| foreach (Countries::getNames() as $region => $name) { | |||
| if (false === isset($countries[$region])) { | |||
| foreach ($countries as $regionCode => $countryCode) { | |||
There was a problem hiding this comment.
Why this change? It's not equivalent to what was here before. Therefore it potentially breaks the compatibility.
There was a problem hiding this comment.
The results is almost the same. I made this change to keep the order given in the countries configuration entry.
There was a problem hiding this comment.
If I'm not mistaken, the performances are not the same this way because before the data was loaded and browsed only once in the buffer of the component while now we have it as many times as countries support x2 (exists & getName)
There was a problem hiding this comment.
You're right ! Not sure its a big deal. Let me check :)
There was a problem hiding this comment.
I would not be so convinced. I would like to have a verification on this before
There was a problem hiding this comment.
The performance overhead is actually small. But I'm surprised there's indeed a topic here. That said, a solution can be quickly found here:
if (!Countries::exists($regionCode))could be changed easily with
$intlCountries = Countries::getNames();
// and later
if (!isset($intlCountries[$regionCode]))(and Countries::getName($regionCode) by $intlCountries[$regionCode])
Co-authored-by: Maxime Veber <[email protected]>
|
Hi ! |
|
Hello @nicolassing sorry for not taking the time to have a look, and thank you very much for your patience. This feature is nice and can be merged. Can you add the (little) optimization I suggested? Side note: please rebase your PR and squash your commit as soon as #134 is merged. |
I'll try to work on this week ! |
|
Thank you very much @nicolassing ! |
No description provided.