Skip to content

[WIP][Form] Allow to configure selectable locale and countries #28542

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

sstok
Copy link
Contributor

@sstok sstok commented Sep 21, 2018

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #11847
License MIT
Doc PR "to do"

This fixes #11847, and I added support for countries as well. It should be possible to update the other types also, but languages is more tricky because en could mean many different specific locales (this needs some thought if accepted).

@sstok sstok force-pushed the 11847-form-add-locales-option-to-localetype branch from 327449e to a199522 Compare September 21, 2018 14:58
@ro0NL
Copy link
Contributor

ro0NL commented Sep 21, 2018

What about a more generic choice_filter: <callable> option, covering any choice type and allowing anyone to mutate choices as needed.

@nicolas-grekas nicolas-grekas added this to the next milestone Sep 21, 2018
@yceruto
Copy link
Member

yceruto commented Sep 21, 2018

I think it's already supported in user land:

$supportedCountries = ['FR', 'ES', 'CU'];

$builder->add('country', CountryType::class, [
    'choice_loader' => new IntlCallbackChoiceLoader(function () use ($supportedCountries) {
        return array_intersect(array_flip(Intl::getRegionBundle()->getCountryNames()), $supportedCountries);
    }),
])

Although I'm not opposed to adding a shortcut for Intl form types.

@ro0NL
Copy link
Contributor

ro0NL commented Sep 21, 2018

vs.

$builder->add('country', CountryType::class, [
    'choice_filter' => function ($countryName) use ($supportedCountries) {
        return in_array($countryName, $supportedCountries, true);
    }),
])

or 'choice_filter' => $supportedCountries as a shortcut

just saying :)

@yceruto
Copy link
Member

yceruto commented Sep 21, 2018

See #14072 and #18334

@ro0NL
Copy link
Contributor

ro0NL commented Sep 21, 2018

Right, cool :) so i'm not the only one. The feature would be priceless, @sstok WDYT?

@yceruto
Copy link
Member

yceruto commented Sep 21, 2018

That (choice_filter) option would be especially useful for child types that need to filter the choices list built in the parent type, without having to duplicate the logic of the choice_loader, right? Otherwise, it can be done in the same choice_loader option.

@ro0NL
Copy link
Contributor

ro0NL commented Sep 21, 2018

Yes or pre-defined choice loaders like Intl. The main advantage is you dont have to (re)define a new choice loader basically. Thus knowing about the labels, etc. In your last example it could just as well be a ChoiceType directly (or at least you dont really rely on CountryType anymore).

@yceruto
Copy link
Member

yceruto commented Sep 21, 2018

In your last example it could just as well be a ChoiceType directly (or at least you dont really rely on CountryType anymore).

Yeah, true :)

@sstok
Copy link
Contributor Author

sstok commented Sep 23, 2018

I took a deep dive into the Choice loading system, and almost drowned 🦈 !

The way the choices are loaded does not allow for filtering after their list is created, the loader provides a list and only modifying the loader implementation (which is not always possible) would make it possible to filter the choices before the ChoiseList is created.

There is no other option (no pun) but use custom options for each types. But allowing a Closure for a custom filtering (like $k[0, 2] === 'EN') would a good feature, in practice this allows an array of accepted choices and closure to filter the choices (for more advanced use cases).

Edit: I added support for closures.

@ro0NL
Copy link
Contributor

ro0NL commented Sep 23, 2018

There is no other option (no pun) but use custom options for each types.

Isnt it possible to add a FilterChoiceList and a decorating choice loader. If the choice_filter option is passed we decorate the choice loader (or use the callback choice loader) and return a FilterChoiceList instead (which also decorates).

@yceruto
Copy link
Member

yceruto commented Sep 24, 2018

@sstok today I can investigate about this choice_filter, do you mind if I make an alternative proposal?

@sstok
Copy link
Contributor Author

sstok commented Sep 25, 2018

@yceruto Feel free to do 😄 this thing is driving me crazy.

@yceruto
Copy link
Member

yceruto commented Sep 26, 2018

See alternative with choice_filter #28607

@xabbuh
Copy link
Member

xabbuh commented Oct 1, 2018

Thank you for your engagement in this @sstok! 👍 Do you agree to close here in favour of #28624?

@sstok
Copy link
Contributor Author

sstok commented Feb 7, 2019

Whoops, a bit late :) yes I agree this can be closed in favor of #28624

@sstok sstok closed this Feb 7, 2019
@sstok sstok deleted the 11847-form-add-locales-option-to-localetype branch February 7, 2019 14:37
@nicolas-grekas nicolas-grekas modified the milestones: next, 4.3 Apr 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Form] add "locales" option to LocaleType
6 participants