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

[Form] Support dependent fields #5807

Open
webmozart opened this Issue Oct 22, 2012 · 43 comments

Comments

Projects
None yet
@webmozart
Contributor

webmozart commented Oct 22, 2012

We long have the problem of creating fields that depend on the value of other fields now. See also:

I want to propose a solution that seems feasible from my current point of view.

Currently, I can think of two different APIs:

API 1
<?php

$builder->addIf(function (FormInterface $form) {
    return $form->get('field1')->getData() >= 1 
        && !$form->get('field2')->getData();
}, 'myfield', 'text');

$builder->addUnless(function (FormInterface $form) {
    return $form->get('field1')->getData() < 1 
        || $form->get('field2')->getData();
}, 'myfield', 'text');
API 2
<?php

$builder
    ->_if(function (FormInterface $form) {
        return $form->get('field1')->getData() >= 1 
            && !$form->get('field2')->getData();
    })
        ->add('myfield', 'text')
        ->add('myotherfield', 'text')
    ->_endif()
;

$builder
    ->_switch(function (FormInterface $form) {
        return $form->get('field1')->getData();
    })
    ->_case('foo')
    ->_case('bar')
        ->add('myfield', 'text', array('foo' => 'bar'))
        ->add('myotherfield', 'text')
    ->_case('baz')
        ->add('myfield', 'text', array('foo' => 'baz'))
    ->_default()
        ->add('myfield', 'text')
    ->_endswitch()
;

The second API obviously is a lot more expressive, but also a bit more complicated than the first one.

Please give me your opinions on what API you prefer or whether you can think of further limitations in these APIs.

Implementation

The issue of creating dependencies between fields can be solved by a lazy dependency resolution graph like in the OptionsResolver.

During form prepopulation, the conditions are invoked with a FormPrepopulator object implementing FormInterface. When FormPrepopulator::get('field') is called, "field" is prepopulated. If "field" is also dependent on some condition, that condition will be evaluated now in order to construct "field". After evaluating the condition, fields are added or removed accordingly.

During form binding, the conditions are invoked with a FormBinder object, that also implements FormInterface. This object works like FormPrepopulator, only that it binds the fields instead of filling them with default data.

In both cases, circular dependencies can be detected and reported.

@webmozart

This comment has been minimized.

Contributor

webmozart commented Oct 22, 2012

One limitation of both APIs is that no dynamic values can be passed to add():

->add('myfield', 'entity', array(
    'query_builder' => function (EntityRepository $repo) {
        // depend on some other field here
    }
));
@daFish

This comment has been minimized.

Contributor

daFish commented Oct 22, 2012

Handling dynamic values would require using AJAX anyway, no?

@stof

This comment has been minimized.

Member

stof commented Oct 22, 2012

Ajax ? We are talking about server side form handling here. The way you handle the frontend is not related to this.

But you probably need some JS as soon as your form is dynamic

@webmozart

This comment has been minimized.

Contributor

webmozart commented Oct 22, 2012

Performance should also be kept in mind when implementing this. When a form is submitted, it is both prepopulated and bound in the same request. If possible, fields should only be constructed once if the output of the conditions does not change.

Furthermore, it must be possible to access the form's object in the conditions. Especially for prepopulation, FormInterface must already provide access to that object.

In respect of JS, while you need to write that by hand, it would be a bonus if the API allowed (partial) JS code generation (at least in theory).

@mablae

This comment has been minimized.

Contributor

mablae commented Oct 22, 2012

@daFish No, dynamic means "dynamic to the Entity" bound to form in this case...

@bschussek Like your idea!

@daFish

This comment has been minimized.

Contributor

daFish commented Oct 22, 2012

@mablae Thanks. I was mixing these two when reading the issue.

@webmozart

This comment has been minimized.

Contributor

webmozart commented Oct 22, 2012

@daFish @mablae Actually both issues are the same. You want to add a field with a configuration that depends on the data of the form or one of its children.

Example 1: Add a field only if $entity->isXxx() ($entity is the form's data) returns true

Example 2: Add a province field with provinces for the selected country (= the data) of a country field

@webda2l

This comment has been minimized.

webda2l commented Oct 22, 2012

I prefer personally the actual way with event..
Would not it be interesting to decide about issue_5480#issuecomment-9343882 which would generate of large changes?

@webmozart

This comment has been minimized.

Contributor

webmozart commented Oct 22, 2012

@webda2l The current way you refer to has strong limitations. You can only rely on information stored in the underlying entity, but not on the (transformed) values of other fields. E.g., if a field is submitted, you cannot observe the value of that field for initializing a different field.

I don't think that #5480 is related to this topic.

@webda2l

This comment has been minimized.

webda2l commented Oct 22, 2012

Ok I understand
About the issue_5480 (I edited my comment to see if the autolink can be remove..), it's because it seems change the way to getData but you know better the impacts.

@webmozart

This comment has been minimized.

Contributor

webmozart commented Oct 22, 2012

@webda2l Don't worry about the links on GH ;) You're right that it's vaguely related, but it doesn't influence the underlying design decisions (as far as I can tell right now).

@jonathaningram

This comment has been minimized.

Contributor

jonathaningram commented Oct 22, 2012

@bschussek from a cosmetic view, I prefer API 2, but a) are the underscores necessary? And b) instead of endif and endswitch can it just be end like in the DependencyInjection Configuration classes? Or at least for consistency endIf and endSwitch.

@webmozart

This comment has been minimized.

Contributor

webmozart commented Oct 23, 2012

@jonathaningram "if", "switch" and the like are PHP keywords and cannot be used as method names, that's why the methods include an underscore. I'm ok with naming the last method just end().

@jonathaningram

This comment has been minimized.

Contributor

jonathaningram commented Oct 23, 2012

@bschussek of course they are, duh.

@kadryjanek

This comment has been minimized.

kadryjanek commented Oct 27, 2012

I would prefer API 2 - it's more fexible and powerfull.
The "builder" way of adding dependent fields is very nice but i also like the "EventListener" way. Do we have access to binded data in listeners? And do we have to create 2 listeners for just one filed FormEvents::PRE_SET_DATA or FormEvents::BIND?
Creating dependencies in the "builder" way is better in this case - dependent fields are created according to the default and binded data, i am right?
@bschussek what do you mean by "no dynamic values can be passed to add()"?

@Burgov

This comment has been minimized.

Contributor

Burgov commented Oct 30, 2012

The limitation you describe is a big drawback, in my opinion. Most of the dynamic fields I add are entity types, which cannot be generated until the value of a specific other field is known.

How about

<?php

$builder
    ->_if(function (FormInterface $form) {
        return $form->get('field1')->getData() !== null;
    }, function(FormInterface $form) use ($builder) {
        $data = $form->get('field1')->getData();
        $builder->add('myfield', 'text', array('label' => 'Choose a relation for ' . $data));
        $builder->add('myotherfield', 'entity', array('query_builder' => function (EntityRepository $repo) use ($data) {
            return $repo->createQueryBuilder('s')->where('s.relation = ?1')->setParameter(1, $data);
        });
    })
;

This, of course, would not work for the switch method, but elseif and else could probably be implemented the same way

@webmozart

This comment has been minimized.

Contributor

webmozart commented Oct 30, 2012

@Burgov The problem with this code is that we don't know in advance which fields this if-statement potentially adds ("myfield" and "myotherfield"), which is necessary in order to build the dependency graph. For example:

$builder
    // (a)
    ->_if(
        function (...) { ... $form->get('myfield')->... },
        function (...) { ... }
    )
    // (b)
    ->_if(
        function (...) { ... },
        function (...) { ... $builder->add('myfield', ...) ... }
    )

When the condition (a) is evaluated, the field "myfield" is accessed and needs to be initialized. Thus we need to know that (b) must be executed, which is impossible if the call to add() is hidden in the closure.

@Burgov

This comment has been minimized.

Contributor

Burgov commented Oct 30, 2012

That makes sense, too bad... However I feel that this feature will only come to full potential if the matter at hand can somehow be tackled... Just throwing some ideas out there:

$builder
    ->_if(function (FormInterface $form) {
        return $form->get('field1')->getData() >= 1 
            && !$form->get('field2')->getData();
    })
        ->add('myfield', 'entity', function(FormInterface $form) {
            return array('query_builder' => function (EntityRepository $repo) use ($data) {
                return $repo->createQueryBuilder('s')->where('s.relation = ?1')->setParameter(1, $data);
            });
        })
        ->add('myfield', 'text', array('label' => 'something not dependant'));
    ->_endif()
;

If the add method is called on an _if (or, in this example also possible: a _case), the third argument can be a callback accepting a FormInterface, which can be examined for data, and returning the actual options. Is this something that could be realised?

@webmozart

This comment has been minimized.

Contributor

webmozart commented Oct 30, 2012

@Burgov Yes, that makes much more sense.

@webmozart

This comment has been minimized.

Contributor

webmozart commented Oct 30, 2012

We could even allow this syntax for normal add() calls:

$builder->add('province', 'entity', function (FormInterface $form) {
    $country = $form->get('country')->getData();

    return array(
        'query_builder' => function (EntityRepository $repo) use ($country) {
            return $repo->createQueryBuilder('s')->where('s.country = ?1')->setParameter(1, $country);
        },
    );
});
@webmozart

This comment has been minimized.

Contributor

webmozart commented Oct 31, 2012

Assuming that we always want to access the data of the child (and not some other property of the FormInterface object), the following simplification would be possible:

$builder->add('province', 'entity', function (Country $country) {
    return array(
        'query_builder' => function (EntityRepository $repo) use ($country) {
            return $repo->createQueryBuilder('s')->where('s.country = ?1')->setParameter(1, $country);
        },
    );
});

Using reflection, we can map the parameter names of the closure to the data of the fields with the same names.

@Burgov

This comment has been minimized.

Contributor

Burgov commented Nov 2, 2012

That would be pretty neat. I suppose this would still work for embedded forms, virtual forms and non-mapped forms? What will happen when the entity is completely new? Will an exception be thrown because $country is null, or will the field simply not be generated? I'd vote for the first one, allowing the creation of a listbox without choices but with an empty value "Select a country first" (the function would then become function (Country $country = null)

If you really want to skip the generation of this field, you'd have to fall back on the _if()'s

@ardianys

This comment has been minimized.

ardianys commented Dec 15, 2012

Is this proposal have been accepted ?

@LeioDos

This comment has been minimized.

LeioDos commented Feb 14, 2013

Good morning friend. I get this error when I deploy to a particular case:

An exception has been thrown during the rendering of a template ("Route "select_provinces" does not exist.") in YLBcombosBundle:Ubicacion:ubicacion.html.twig at line 13.

What should I check? I have:

$(function (){
$('#Ubicacion_estado').change(function(){
var data = {
estado: $(this).val()
};
$.ajax({
type: 'post',
url: '{{ path('select_provinces') }}',
data: data,
success: function(data) {
$('#Ubicacion_municipio').html(data);
}
});
});

Other part:

  • @route("/municipios", name="select_provinces")
  • @template()
    */
    public function municipiosAction()
    {
    $estado= $this->getRequest()->request->get('estado');
    $em = $this->getDoctrine()->getManager();
    $municipios = $em->getRepository('YLBcombosBundle:Municipio')->encontrarPorEstado($estado);
    return array(
    'municipios' => $municipios
    );
    }

thank you....

@stof

This comment has been minimized.

Member

stof commented Feb 14, 2013

@LeioDos This has strictly nothing to do with this issue. Why are you posting this in the conversation ?

@LeioDos

This comment has been minimized.

LeioDos commented Feb 14, 2013

As simple as that is NOT RESPOND if your interest or want to help, I'm following the recommendation of the scheduled discussion in: http://showmethecode.es/php/symfony/symfony2-selects-dependientes-mediante-eventos/ @stof

@gnutix

This comment has been minimized.

Contributor

gnutix commented Mar 6, 2013

Is anyone currently working on this ? Is there any chance to see it become true in 2.2.x / 2.3 ?
I'm surely not the only one badly needing this since Symfony has first been released. ;)

@marcospassos

This comment has been minimized.

marcospassos commented Mar 10, 2013

Why not use event approach? From a cosmetic view, _if and addIf sounds very strange in SF's world, where only beautiful things exist :)

@sstok

This comment has been minimized.

Contributor

sstok commented Mar 10, 2013

@LeioDos The error says that the route does not exists, the routing component is completely unrelated to this and the error is properly because you forgot to import the route in your configuration. http://symfony.com/doc/2.0/bundles/SensioFrameworkExtraBundle/annotations/routing.html

And please use the mailing list for asking support, the issue tracker is explicitly only for bugs and features.
https://groups.google.com/forum/?fromgroups#!forum/symfony2

@gnutix

This comment has been minimized.

Contributor

gnutix commented Apr 10, 2013

Any chance to see this done for 2.3 ?

@lwc

This comment has been minimized.

lwc commented Jul 18, 2013

The main problem I see in using closures to define these dependencies instead of something more declarative is that it becomes hard to, for example, bake the dependencies into data attributes so one could use js to show / hide the dependent fields.

I have another solution proposed that would allow for this: #8513

Feedback welcome / desired.

@xzyfer

This comment has been minimized.

xzyfer commented Jul 24, 2013

@stof @bschussek thoughts in @lwc alternative approach in #8513?

@webmozart

This comment has been minimized.

Contributor

webmozart commented Oct 16, 2013

An important use case for dynamic fields is to support fields that depend on the built object of the form. For example, take the following two classes:

class Contact
{
    public function addEmail(Email $email)
    {
        $this->emails->add($email);
    }
}

class Email
{
    public function __construct(Contact $contact, $text)
    {
        $this->contact = $contact;
        $this->text = $text;

        $contact->addEmail($this);
    }
}

Currently, this setting can only be made to work using a POST_SET_DATA listener, because the Email constructor depends on the Contact object, which is only created after submitting the fields of the contact form.

@ioleo

This comment has been minimized.

ioleo commented Oct 21, 2014

@webmozart If possible, I'd go with API 2. When dealing with multiple fields added in each scenario, the API 2 approach remains readable.

@webdevilopers

This comment has been minimized.

webdevilopers commented Dec 2, 2014

👍

@webdevilopers

This comment has been minimized.

webdevilopers commented Dec 11, 2014

For instance you have a radio button addFoo that was checked with the value 1 resp. true.
The form now expects the user to select a value from a choice widget selectFoo that is disabled by default.
After submitting the form should be invalid - because a constraint is used - and the error states that a choice has to be selected from selectFoo. Of course now selectFoo should no longer be disabled.

I created such an dependency in this example:
https://gist.github.com/webdevilopers/5306ee3417791227abf3

Will the API2 approach also take care of dynamically disabling elements?

@webdevilopers

This comment has been minimized.

webdevilopers commented Dec 11, 2014

I have moved my issue here:
#12946

It focusses on my general understanding of the disabled option.

@Tobion

This comment has been minimized.

Member

Tobion commented Mar 26, 2015

Just read the ticket and you can see yourself what the status is.

@Glideh

This comment has been minimized.

Glideh commented Apr 29, 2016

+1 for the API 2
Dependent fields are still a pain in the ass today.

@CyrilleGuimezanes

This comment has been minimized.

CyrilleGuimezanes commented May 12, 2016

+1 for the API 2
Still blocked by dependent field....

@DavidBadura

This comment has been minimized.

Contributor

DavidBadura commented May 12, 2016

I am still wondering. What do we need this API for? Dynamic forms can be done with events. I solved complex Forms (Multi-Step-Forms etc.) with events simply.

@Glideh

This comment has been minimized.

Glideh commented Mar 14, 2017

Well, events allow powerful things to be done indeed.
But this dependent fields (lists most of the time) feature is a common need and would deserve some shortcut.
However the event subscriber still requires at least 50 lines of code for each dependent list:
Each time we have to implement at least the 3 postSetData, preSetData, preSubmit events with the preSubmit parameters "convertion"
And since we can't modify a field from the subscriber, it have to be configured entirely in the subscriber, which is not very readable from the related form definition (I mean if the field config is not repeated there).
This is without speaking about the usual ajax needed.

@psyray

This comment has been minimized.

psyray commented Jun 4, 2018

You coud try the choice_loader option
https://symfony.com/doc/current/reference/forms/types/choice.html#choice-loader

use Symfony\Component\Form\ChoiceList\Loader\CallbackChoiceLoader;
use Symfony\Component\Form\Extension\Core\Type\ChoiceType;
// ...

$builder->add('constants', ChoiceType::class, array(
    'choice_loader' => new CallbackChoiceLoader(function() {
        return StaticClass::getConstants();
    }),
));
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment