-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Form] Add generic way for overriding nested options #9177
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
Comments
Will this require making each form type aware of this syntax when adding its children or would it be handled automatically in the Form component for any form type ? |
@stof This should be handled automatically for any form type. |
@bschussek at which level would it be implemented ? |
@stof Not sure yet |
Star symbol for children settings? Why not a classic hierarchical extension "parent to children" with the opportunity to override into the child option? |
Why @Date? Shouldn't it be @updatedAt? |
@dewos @peterrehm The "updatedAt" field in the example is of type "datetime" and has two children, "date" and "time". In this example, I configured the options for the nested "date" field. |
Hi @bschussek. Ok fine, but maybe the @* "parent to children" can be automatically implicit, with the option to explicit override the setting in the child. |
@dewos you mean that all options should be forwarded to children by default ? This is a bad idea IMO as available options for the children are different than available options for the type itself. |
@stof yeah it's true, but mostly they're the same, and for @* is problematic too, i think. |
@dewos No. For |
I see. Thanks @stof. |
Some more thoughts: This would probably be easier to implement if the syntax was this: $builder->add('updatedAt', 'datetime', array(
'@date' => array(
'required' => true,
'@day' => array(
'empty_value' => 'The day...',
),
),
));
// equivalent to
$builder->add('updatedAt', 'datetime', array(
'@date' => array(
'required' => true,
),
'@date/@day' => array(
'empty_value' => 'The day...',
),
));
// equivalent to
$builder->add('updatedAt', 'datetime', array(
'@date/required' => true,
'@date/@day/empty_value' => 'The day...',
)); Then we can simply split by /. Maybe even the first version is sufficient, where we just need to test the first character of each option. One important point is that this syntax also needs to be usable in setDefaultOptions(). This is not really useful if a form type also calls add(), because then the option can just be added in add(). But for type extensions, this could come in handy. public function setDefaultOptions(OptionsResolverInterface $resolver)
{
$resolver->setDefaults(array(
'@day/required' => true,
));
} |
This would be a fantastic feature to land in 2.7 if possible. Its a real annoyance to work around. |
i think the inherit options should get their own option entry: $resolver->setDefaults(array(
'inherit_options' => array(
'@day/required' => true,
),
)); this solves multiple problems:
|
what i also noticed: this feature does not resolve #12314 in any means, since the prototype is its "own" form, that is not connected to the original, except for sharing some configuration |
some more thoughts: $resolver->setDefaults(array(
'inherit_options' => array(
'#required' => false,
'#inherit_options' => array(
'@child/**#required' => false
),
'@day#required' => true,
),
)); |
I am not convinced that we need this. For example, in many core types where we need to provide options that are ultimately passed through to a nested type we first perform some validation as not all options are useful in all contexts, but also depend on some other option set for the parent type. Thus, this feature wouldn't save as much here, but is something that still needs to be maintained. @symfony/deciders What do you think? |
Could it be already cover by the feature "Nested Options Definition" introduced since Symfony 4.2?
This would at least solve the issue of override some nested options. |
I'm not in favor of this. The problem I see here is that if I have a form type that explicitly doesn't provide an option for an inner type, it can be bypassed and change the behavior of the form. I'm of opinion that if you want to have the option to configure an inner field, the wrapper form should provide this configuration option. Yes it's slightly repetitive, yet makes it possible to restrict configuration where needed. Any configuration for nested types should be configured through the wrapper. |
let's close here then |
We currently use various different ways for influencing the options of the children of an added type:
a) By letting one option accept multiple values for the children (e.g. the "empty_value" option in DateType)
b) By having dedicated "options" options (e.g. "options" in CollectionType, "first_options"/"second_options" in RepeatedType)
And in #3825, a third possibility was suggested:
c) By having one dedicated option per child and option (e.g. "day_widget", "month_widget" etc.)
These different alternatives
Therefore I propose adding a generic way for overriding nested options by supporting a special syntax in the options array:
@child(/nestedChild)*[#option]
Examples:
Additionally, setting an option for all children at once should be supported:
References:
The text was updated successfully, but these errors were encountered: