Skip to content

[Form][LanguageType] Add whitelist option #35456

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
wants to merge 1 commit into from

Conversation

fancyweb
Copy link
Contributor

@fancyweb fancyweb commented Jan 23, 2020

Q A
Branch? master
Bug fix? no
New feature? yes
Deprecations? no
Tickets -
License MIT
Doc PR -

From my experience, it's common to want a language choice input, but only on "available" (prefiltered) languages from the application. For example, I know my application content must only be available in 5 languages, so I want to display only those 5 options when contributing it.

Currently, I think there is no way to use the core LanguageType with only some defined choices. That means I have to create a custom LanguageType (whatever the implementation) and that I don't benefit from the core one easily. I can extend it but that requires additional logic /code.

I think that adding a whitelist option on all Intl related core form types would make sense. I'm only pushing the language one right now as a demo and to open the discussion. Maybe that actually adding this option to all core form types that have the ChoiceType as their parent and that somehow force the choices would make sense as well.

@fancyweb fancyweb force-pushed the form-intl-types-whitelist branch from ebd5c70 to 418f5bf Compare January 24, 2020 11:43
@xabbuh xabbuh added this to the next milestone Jan 24, 2020
@xabbuh
Copy link
Member

xabbuh commented Jan 24, 2020

Another option would be to filter out options that should not be available (see #32657).

@fancyweb
Copy link
Contributor Author

I didn't know these related issues but the idea is the same I guess: we need more control on "forced" choices.

@javiereguiluz
Copy link
Member

I'm sure my comment is naïve, but here it is: in my head, all these prepopulated choice types (language, country, locale, etc.) should define an internal allChoices option and a public/external choices option.

If choices is empty, then we do choices = allChoices ... but if I define choices, then we filter allChoices to pick only those values. Don't know if this is possible or makes sense ... but to me, as a user, I'd love to just define choices to filter the values of these types.

@fabpot
Copy link
Member

fabpot commented Feb 4, 2020

I like @javiereguiluz idea.

@fancyweb
Copy link
Contributor Author

fancyweb commented Feb 4, 2020

Closing this one then, and let's try to implement Javier idea.

@fancyweb fancyweb closed this Feb 4, 2020
@fancyweb fancyweb deleted the form-intl-types-whitelist branch February 4, 2020 16:30
@HeahDude
Copy link
Contributor

HeahDude commented Feb 4, 2020

I don't think we need another option in choice subtypes for this, if one wants to define the choices option one should use the ChoiceType instead. I agree that we should leverage a choice_filter option for more specific use cases (like type extension for core types) as suggest by @xabbuh. I've got a branch for it based on my other open PRs, just waiting to be pushed once they are dealt with.

@javiereguiluz
Copy link
Member

javiereguiluz commented Feb 4, 2020

@HeahDude I don't understand how these things work internally, but this is what we want to provide to end users. Do we agree on it?

// display all languages (translated into the user language)
$builder->add('language', LanguageType::class);

// display only these languages (translated into the user language)
$builder->add('language', LanguageType::class, [
    'choices' => ['es', 'fr', 'en'],
]);


// display all countries (translated into the user language)
$builder->add('country', CountryType::class);

// display only these countries (translated into the user language)
$builder->add('country', CountryType::class, [
    'choices' => ['JP', 'US'],
]);

// etc.

@HeahDude
Copy link
Contributor

HeahDude commented Feb 4, 2020

We agree on the need not on the implementation yet.
The choice_filter would allow (for every choice type) something like:

// display only these languages (translated into the user language)
$builder->add('language', LanguageType::class, [
    'choice_filter' => fn($choice) => in_array($choice, ['es', 'fr', 'en'], true),
]);

But once we have this feature we could indeed use the choices options of intl choice types as a short hand for DX.

@ro0NL
Copy link
Contributor

ro0NL commented Feb 5, 2020

i generally like @javiereguiluz's proposal, but semantically i expect choices to hold values and labels.

In this case, adding a new choice_filter option is the same approach. Either ChoiceType generic or specific for Intl types.

back to #32657 :)

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.

8 participants