[Form] Add FilterChoiceLoader + choice_filter option #28624

ro0NL · 2018-09-27T16:21:10Z

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#11847, #14072, #28607, #28542
License	MIT
Doc PR	symfony/symfony-docs#10413

I did it :) i understand form choice lists.. took me only 2 days 😂

But here it is, a choice_filter option for the ChoiceType. Allowing to filter the list of choices independent from any choice loader / array.

This is no magic bullet. To truly optimize one should always prefer a filtered choice list (e.g. passing a query builder to the doctrine choice loader). But for built-in / pre-defined choice loaders, e.g. Intl, this is convenient.

Thanks to @sstok and @yceruto also.

agree on approach (per [Form] filter entity choicelist after hydration #14072 (comment))
consider delegating array choices to ArrayChoiceList for sanity :)
update filtering array choices (see todo in code)
fix callable argument consistency (choice only)
add form integration/rendering tests

src/Symfony/Component/Form/ChoiceList/Loader/FilterChoiceLoader.php

stof · 2018-09-27T17:03:53Z

* fix callable argument consistency (`choice, key, value` ideally)

Looking at the places where you use the callable, you cannot do that, as you may only have the choice at that point.

src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php

src/Symfony/Component/Form/Tests/Fixtures/Author.php

ro0NL · 2018-10-01T06:59:16Z

Ready 👍

HeahDude · 2018-10-09T20:50:29Z

src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php

@@ -296,6 +298,16 @@ public function configureOptions(OptionsResolver $resolver)
            return $choiceTranslationDomain;
        };

+        $choiceFilterNormalizer = function (Options $options, $choiceFilter) {
+            if (null !== $choiceFilter && !\is_callable($choiceFilter)) {


So if we pass a valid set of two values to filter out the list but the array happens to be callable this may break. Since it would add complexity to handle this edge case I suggest to either remove the support of arrays or document the limitation.

not sure i understand

$form = $this->createForm(ChoiceType::class, null, [ 'choices' => [1,2,3], 'choice_filter' => [3], ]);

works as expected...

@ro0NL HashDude meant a choice_filter like this ["ClassName", "methodName"] for this, is_callable might return true

i see, that's edgy yes :) 👍 for removing array support from me

src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php

ro0NL · 2018-10-11T06:19:15Z

@HeahDude i've optimized it a bit, now it only creates a new loader in case a filter is given. To me that's reasonable in terms of caching, the result would differ per filter anyway.. so not really cacheable to me.

src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php

ro0NL · 2018-10-11T07:31:11Z

to share cache between filtered/non-filtered choice lists we could add

ChoiceLoaderFactoryInterface::createLoaderFromChoices(iterable $choices): ChoiceLoaderInterface

then we can always use a cacheable loader (either given or created from choices) before decorating it and passing it to createListFromLoader. But im not sure it's worth it...

This PR was merged into the 4.2-dev branch. Discussion ---------- [Form] Deprecate TimezoneType regions option | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no  | Deprecations? | yes | Tests pass? | yes  | Fixed tickets | #28848 | License | MIT | Doc PR | symfony/symfony-docs#...  I know i've added this option myself in 4.1, but given my recent development for #28624 i realized it's an opinionated feaure, which can/should be solved on user-side (`choice_filter/choice_loader` and/or `group_by`). - blocks translations as we dont have them (see #28831) - blocks possibility of switching to Intl zones which doesnt really have this filter feature (see #28836) ~While at it, i solved a few issues with `OptionsResolver` that is able to deprecate options as of 4.2 also.~ Fixed in #28878 - when resolved trigger the deprecation - allow to opt-out from triggering the deprecation - dont trigger deprecation for default values (only given ones) Commits ------- 5cb532d [Form] Deprecate TimezoneType regions option

jo66 · 2019-02-04T19:14:39Z

@ro0NL choice_filter is nice option, will it be merged?

ro0NL · 2019-02-07T15:01:23Z

@jo66 rebased :) let's see if core wants to merge 👍

javiereguiluz · 2019-07-03T11:21:53Z

Could we restart the work on this to finish it? It looks like a cool feature for Symfony 5.0!

ro0NL · 2019-07-22T16:19:40Z

I tend to close in favor of passing an optimized choice_loader (filtered and sorted as expected), that's the fastest way for core to move forward: no changes involved.

In #31593 i've also concluded this.

The only "issue" for core is the built-in Intl choice types, which manage the choice loader for us.

Im thinking of a feature specific to Intl to handle the sorting/ordering here. Much like concluded in #32657 (comment) as well.

stof · 2019-07-22T16:55:31Z

@HeahDude didn't you start working on an alternative implementation at the choice factory level ? I cannot find a PR for it. You were supposed to open it as the result of the FOSSA Hackathon IIRC.

yceruto · 2019-07-22T17:29:55Z

@stof do you mean this one #30994 ?

ro0NL · 2019-09-27T09:28:22Z

closing this in favor of #32657 / #30994

carsonbot added Status: Needs Review Form Feature labels Sep 27, 2018

ro0NL mentioned this pull request Sep 27, 2018

[Form][DX] Add choice_filter option to ChoiceType #28607

Closed

stof reviewed Sep 27, 2018

View reviewed changes

ro0NL commented Sep 27, 2018

View reviewed changes

src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php Show resolved Hide resolved

This was referenced Sep 29, 2018

[Form] Added choice_filter option symfony/symfony-docs#10413

Closed

[symfony/form] ChoiceType add possibility to change choice_list while building view #28531

Closed

nicolas-grekas added this to the next milestone Sep 29, 2018

xabbuh reviewed Sep 30, 2018

View reviewed changes

src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php Outdated Show resolved Hide resolved

ro0NL commented Oct 1, 2018

View reviewed changes

src/Symfony/Component/Form/Tests/Fixtures/Author.php Outdated Show resolved Hide resolved

xabbuh mentioned this pull request Oct 1, 2018

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

Closed

HeahDude reviewed Oct 9, 2018

View reviewed changes

ogizanagi reviewed Oct 11, 2018

View reviewed changes

src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php Outdated Show resolved Hide resolved

ro0NL mentioned this pull request Oct 14, 2018

[Form] Deprecate TimezoneType regions option #28860

Merged

[Form] Add FilterChoiceLoader

b080d79

ro0NL mentioned this pull request May 23, 2019

[Form] Recall for choice group_by ordering #31593

Closed

ro0NL mentioned this pull request Jul 22, 2019

Option to filter choices #32657

Closed

ro0NL closed this Sep 27, 2019

ro0NL deleted the choice-filter branch September 27, 2019 09:28

nicolas-grekas modified the milestones: next, 4.4 Oct 27, 2019

HeahDude mentioned this pull request Mar 14, 2020

[Form] Added a "choice_filter" option to ChoiceType #35733

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Form] Add FilterChoiceLoader + choice_filter option #28624

[Form] Add FilterChoiceLoader + choice_filter option #28624

ro0NL commented Sep 27, 2018 •

edited

Loading

stof commented Sep 27, 2018

ro0NL commented Oct 1, 2018

HeahDude Oct 9, 2018 •

edited

Loading

ro0NL Oct 10, 2018

Hanmac Oct 11, 2018

ro0NL Oct 11, 2018

ro0NL commented Oct 11, 2018 •

edited

Loading

ro0NL commented Oct 11, 2018

jo66 commented Feb 4, 2019

ro0NL commented Feb 7, 2019

javiereguiluz commented Jul 3, 2019

ro0NL commented Jul 22, 2019

stof commented Jul 22, 2019

yceruto commented Jul 22, 2019

ro0NL commented Sep 27, 2019

[Form] Add FilterChoiceLoader + choice_filter option #28624

[Form] Add FilterChoiceLoader + choice_filter option #28624

Conversation

ro0NL commented Sep 27, 2018 • edited Loading

stof commented Sep 27, 2018

ro0NL commented Oct 1, 2018

HeahDude Oct 9, 2018 • edited Loading

Choose a reason for hiding this comment

ro0NL Oct 10, 2018

Choose a reason for hiding this comment

Hanmac Oct 11, 2018

Choose a reason for hiding this comment

ro0NL Oct 11, 2018

Choose a reason for hiding this comment

ro0NL commented Oct 11, 2018 • edited Loading

ro0NL commented Oct 11, 2018

jo66 commented Feb 4, 2019

ro0NL commented Feb 7, 2019

javiereguiluz commented Jul 3, 2019

ro0NL commented Jul 22, 2019

stof commented Jul 22, 2019

yceruto commented Jul 22, 2019

ro0NL commented Sep 27, 2019

ro0NL commented Sep 27, 2018 •

edited

Loading

HeahDude Oct 9, 2018 •

edited

Loading

ro0NL commented Oct 11, 2018 •

edited

Loading