[Form] Add option "widget" to ChoiceType #6602

Open
webmozart opened this Issue Jan 7, 2013 · 37 comments

Comments

Projects
None yet
Member

webmozart commented Jan 7, 2013

The PR #1951 "[Form] Added form type "entity_identifier" has been open for ages now. The number of supporters alone justifies its existence, but I think the requested functionality should be solved in a more generic way.

I think that we should add two new options widget and delimiter to ChoiceType.

  • widget can be one of the values select, checkbox, radio and text
  • delimiter can be any single character

The option expanded would be deprecated. I think having a widget and an expanded option at the same time is inconsistent.

Behavior:

  • widget = 'select': This is equivalent to expanded = false right now.
  • widget = 'checkbox': multiple must not be set to false. Otherwise equivalent to expanded = true.
  • widget = 'radio': multiple must not be set to true. Otherwise equivalent to expanded = true.
  • widget = 'text': A text input is shown.
    • if multiple is false, the input must equal one of the predefined choices.
    • if multiple is true, the input is split by the character defined in delimiter (a comma by default), then each value is trimmed (unless trim is false). Each resulting input must equal one of the predefined choices.
Contributor

michaelperrin commented Jan 7, 2013

Sounds as a good replacement of the formerly proposed entity_identifier. I may have a look at how to implement this feature if I have time.

Contributor

henrikbjorn commented Jan 7, 2013

The most important thing i think is that having it lazy

Contributor

Gregwar commented Jan 9, 2013

Be careful with the requests, one of the issues with the curent entity type is that it will fetch all the possible entries from the database in any situation, this is exactly what #1951 intended to avoid for obvious performances reasons.

Think of a message form in which you'd have a "addressee" field, that could be a text widget using the "login" property to automatically fetch back the good user when you type it. Another example, think of a full custom javascript UI, that would for instance use a google map or an autocompletion field and automatically fill an hidden id.

More accurately, the semantic of the query is not the same depending on which widget you will use.

  • If you are using a choice, select or checkbox widget, the query will fetch all the lines
  • If you use a text widget, I think you'll want your query to be used to fetch the records with the text given by users

Then, the query_builder option would be confusing, don't you think ?

Narretz commented Jan 24, 2013

Indeed, in #1951 the most important change was that entity does not fetch all the entries from the db. Maybe that is too generic, but I understood entity_id as a form field that references a collection of entities of a specific type, but without prefetching all the choices. Is this included in this proposal? In any case, we should clarify what exactly this change should accomplish.

Member

webmozart commented Feb 21, 2013

@Narretz Yes it is. Obviously, when entering the choice through a text input, the choices are not (and need not be) loaded in advance.

Member

webmozart commented Feb 21, 2013

@Gregwar With the current choice lists, you could only enter the primary key into the text field (in EntityType). We could extend EntityType though so that the property path to the choice's value can be configured (see also #4067). In that case, a WHERE clause needs to be appended to the created query (best to be done by adding EntityLoaderInterface::getEntitiesByField($field, array $values)).

Contributor

Gregwar commented Feb 24, 2013

@bschussek, I'm OK with the idea, but what about user's custom query builder ? I think this should always be a possibility, and in this case the query_builder option have to be a Callable returning a query that fetches one record, don't you think so ?

Member

webmozart commented Feb 25, 2013

@Gregwar If you check the source of ORMQueryBuilderLoader (which implements EntityLoaderInterface), you will see that it uses the provided query builder and adds some functionality on top of that.

winzou commented Mar 18, 2013

This is a great feature. I totally agree with the way you've planned to do it. 👍

bamarni referenced this issue in genemu/GenemuFormBundle May 15, 2013

Open

Lazy choice list (Autocompletion cities) #122

Contributor

bamarni commented May 19, 2013

@bschussek : for the text widget, when you say "the input must equal one of the predefined choices", "choice" means the "label" or the "value"?

and when?

Yes Please, ran into the hidden entity_id issue with an elegant solution needed

Contributor

tyx commented Mar 18, 2014

Any status about this feature ?

Member

Tobion commented Mar 31, 2014

I think the implementation should also support HTML5 "datalist": http://www.w3schools.com/tags/tag_datalist.asp
This would probably require another option like "load_options" : true|false.

@webmozart text choices can only be of other type than "text". Like <input type="url" list="urls" name="urls"><datalist id="urls">.... So the widget option should also support all the other input types like url, email and not only text.

Member

stof commented Mar 31, 2014

@Tobion From a backend PoV, the datalist is not a choice. The datalist provides completion for choices, but it does not restricts the input at all. The user can still submit anything, so the Symfony side cannot be implemented using a ChoiceType

Member

Tobion commented Mar 31, 2014

That' just a matter of validation. In a text input for choices the user can submit anything as well.

Member

stof commented Mar 31, 2014

@Tobion but you are asking to add this to a choice type, not to a text type. the choice type is selecting a value in the ChoiceList when binding the value

Contributor

michaelperrin commented Jun 4, 2014

I started working on this and I hope to propose a PR soon.

@michaelperrin michaelperrin added a commit to michaelperrin/symfony that referenced this issue Jun 6, 2014

@michaelperrin michaelperrin Add `widget` option for `choice` type (#6602) - WIP
Log of debug and ugly things, this commit will be squashed with others
680718a

@michaelperrin michaelperrin added a commit to michaelperrin/symfony that referenced this issue Sep 18, 2014

@michaelperrin @michaelperrin michaelperrin + michaelperrin Add `widget` option for `choice` type (#6602) - WIP
Log of debug and ugly things, this commit will be squashed with others
f610b1d

@michaelperrin michaelperrin added a commit to michaelperrin/symfony that referenced this issue Oct 24, 2014

@michaelperrin @michaelperrin michaelperrin + michaelperrin Add `widget` option for `choice` type (#6602) - WIP
Log of debug and ugly things, this commit will be squashed with others
8086c88

@michaelperrin michaelperrin added a commit to michaelperrin/symfony that referenced this issue Oct 26, 2014

@michaelperrin michaelperrin Fix BC issue (#6602) 45f0bc2

@michaelperrin michaelperrin added a commit to michaelperrin/symfony that referenced this issue Oct 26, 2014

@michaelperrin michaelperrin #6602 : WIP 70ea093

@michaelperrin michaelperrin added a commit to michaelperrin/symfony that referenced this issue Oct 28, 2014

@michaelperrin michaelperrin Start new way to fix #6602 4ddf45b
Contributor

colinodell commented Feb 11, 2015

+1 to adding hidden as a supported widget type

austinh commented Jun 4, 2015

Any update on this? :(

👍 It would be great if this can be added. I see there are already a lot of open PRs

bamarni referenced this issue Feb 1, 2016

Closed

[Form] Add option widget to ChoiceType #17646

2 of 3 tasks complete
Member

HeahDude commented Feb 4, 2016

Hi there,

since this issue is very old and there were a lot of PR's about it with their own discussions,
I wanted to try to resume the situation here, regarding :

  • the actual good work made by @bamarni (in #8112, #15053 and #17646)
  • the changes made in 2.7 and 2.8 (choice_as_values, choice_translation_domain, choice_label, choice_value and choice_attr) and possibly made in 3.x (choice_label_attr
  • and the not so evident new text widget for ChoiceType.

@webmozart said :

Behaviour:
widget = 'select': This is equivalent to expanded = false right now.
widget = 'checkbox': multiple must not be set to false. Otherwise equivalent to expanded = true.
widget = 'radio': multiple must not be set to true. Otherwise equivalent to expanded = true.
widget = 'text': A text input is shown.
if multiple is false, the input must equal one of the predefined choices.
if multiple is true, the input is split by the character defined in delimiter (a comma by default), then each value is trimmed (unless trim is false). Each resulting input must equal one of the predefined choices.

@bamarni and @stof made a point (in #15053) that allowing multiple for widgets select and text but ignoring it checkbox and radio may be inconsistent and that we may have three widget option instead of four using select, text and expanded.

Doesn't really matter this can be easily solved by a normalizer as in #17646.
But from my POV it's more understandable and less confusing while it justifies the deprecated expanded option and keeps actual multiple option implementation.

But the real issue - IMHO - now faced in #17646 is the way labels and values are handled for the text widget. as @stof said in #8112 :

labels are not valid. You cannot identify the choice by its label. You can have several choices with the same label (or even all of them).

Actually (as of 2.7) it can be identified and labels represent choices displayed to users
and values the information to deal with in code (objects...) needing a string representation for the value html attribute.

But in the last PR text widget as a value showing up in the input which allow to think it's editable but if multiple is false you don't have any clue of what choices are available to get through validation as it needs to be one of the predefined choices.
choice_value needs to be set to avoid ids to show up in text input as they don't mean anything in the view this is partially why #17694 should be merged IMO.

I assume we don't want to play again with choices_as_values here.

Plus there's another issue showing up : #17579

So what can be done ?

ping @webmozart

Member

HeahDude commented Feb 4, 2016

From my POV, the idea of having choice as an input text calls a need to put whatever you want in it.

When I first use the form component it was about 2.4, and I was confused between CollectionType and ChoiceType because I wanted to append an "other" option to my choices showing an input text by using an option like allow_add in CollectionType.

I'm convinced it would be very useful.

Now with regards to the hook implemented in #17609 and this debated widget option, I think it would be nice to find a proper way to have this working.

I could try to implement it if some symfony users or deciders agree with this solution.

Contributor

bamarni commented Feb 5, 2016

But in the last PR text widget as a value showing up in the input

It is expected, the original idea was to have an entity_id type (for example a field where you can type an invoice_id). There is also another use case which is lazy-loading, sometimes you might not want to query for all the data, but autocompleting choices with an ajax query for example, then with some js filling the value in the hiddenfield.

So maybe @webmozart can confirm, but in my understanding text and hidden widgets should generally not be used directly, it's more of a generic solution to allow other types to be plugged on top of it.

Member

HeahDude commented Feb 5, 2016

@bamarni I understand but this quite a complication to access a little data in such an edge case IMO, that's why @_webmozart said this feature

should be solved in a more generic way

Actually, with #17646 implementation, nothing prevents users to use text widget as they want to and so it needs to the more consistent with global usage of ChoiceType and all its inheritance.

Besides your use case targets only a usage of EntityType or ChoiceType dealing with entities, anyway it could imply the choice loader or/and a data mapper to dynamically add a choice with the value you need (like some ids imploded by a delimiter for auto-completion).

Thinking about this made me suggest above that following this path we could handle an allow_add option for ChoiceType where we would add another choice option input to the view like CollectionType does for prototype.

Contributor

bamarni commented Feb 9, 2016

@fabpot @webmozart : any news about this one? Is the implementation explained here still the plan for this feature?

Contributor

Gregwar commented Feb 9, 2016

Actually, I remember some use cases from the original feature proposal:

  • Be able to create a form where you enter a login, for instance as a message receiver, and that login would identify directly an entity
  • Be able to have hidden fields representing entity id(s) and allowing you to do all your fancy logics to help the user to find the entity using autocompletion, maps, JS etc.
  • Be able to "transport" an entity in some way. Imagine a comment form that would contain the ID of the film that is being commented. This point is arguable, but I think there is use cases.
Member

HeahDude commented Feb 11, 2016

Hi @Gregwar, (a lot of arguments below, feel free to scroll to the conclusion :)

  • I'm not sure to understand what you mean by "message receiver" but that first case you describe seems to be about sending a scalar value as property from client to server and getting back an entity with that property, which perfectly fits the controller's reponsability (even easier using params converters).
    You could use any FormType to get this scalar and generate (on client side using javascript) an url to query the right controller.
    One should not need to tweak the EntityType in its core for this what do you think ?
  • the second use case appears to be not so far from the first and definitly close to the last and we could here debate about class responsability.
    When you actually create a form and pass it some data (and options), it usually happens in the controller (or a service e.g a custom form factory).

Arguments

Tell me if I'm wrong but the EntityType is nothing but a ChoiceType with default choices set by loading all entities of a class which can be tuned by defining the query_builder option. Beside that, they are the same form type, all about "choosing" something.

That does not mean dealing with entities in a form should imply using EntityType.

You can already add many HiddenType holding entities as data in a form, no need to use ChoiceType or EntityType for that.

webmozart's comment in this issue makes it very clear, saying that when you already have loaded your entities in a controller, you should directly use them as choices in a ChoiceType instead of using EntityType unless you need a special extra query and cannot do this by filtering the ones you got.

But code is always more explicit, example below does not work as it is but imagine you need to prepare a lazy choice list from ids, in a controller you may want to use something like :

// symfony 2.8+
use Symfony\Component\Form\Extension\Core\Type as CoreType;

// I use findAll() in this example but it could be any specific query
$entities = $this->getDoctrine->getRepository(MyEntity::class)->findAll();

if (10 < count($entities)) {
    $choices = array_slice($entities, 0, 10);
    $moreChoices = array_slice($entities, 10);
} else {
    $choices = $entities;
}

$builder = $this->createFormBuilder(array('entities' => new MyEntity()))
// if multiple was true it should be array('entities' => array(new MyEntity()))
    ->add('entities', CoreType\ChoiceType::class, array( // same as EntityType 
        'choices' => $choices, // same as using a custom query builder with a 10 limit
        'choice_label' => 'name',
        'choice_value' => 'property',
        'choice_name' => function ($entity) {
            return 'entities['.$entity->getId().']';
            // should be 'entities['.$entity->getId().'][]' if multiple is true
        },
    );

if (isset($moreChoices) {
    $extraChoicesIds = array_map(function($entity) {
        return $entity->getId();
    }, $moreChoices);
    $builder->add('more_choices', CoreType\HiddenType::class, array(
        // 'data' option will fill the "value" html attribute of the hidden input
        'data' => implode(';', $extraChoicesIds),
        'mapped' => false, // ignore while $form->handleRequest($request)
    ));
}

// and in another for XmlHttpRequest assuming you serialized the hidden input to post it here :
public function lazyEntityChoicesAction(Request $request) {
   // get $em = entity manager
    $field = $this->createFormBuilder()
        ->add('entities', CoreType\ChoiceType::class, array(
            'choices' => $em->findIds(explode(';', $request->request->get('more_choices')); // a custom repository method
            'choice_label' => 'name',
            'choice_value' => 'property',
            'choice_name' => function ($entity) {
                return 'entities['.$entity->getId().']'; // same caution as above if multiple is true
            },
        ))
        ->getForm()->get('entities');

    // return a view of extra choices to append in your form,
    // passing array('choices' => $field->createView()) and using a simple twig loop
}

or you could even return a json and build html options with javascript, using param converter and serializer if needed.

There so many ways to deal with entities data, this would work :

foreach ($moreChoices as $idx => $extraChoice) {
    $builder->add('extra_choice_'.$idx, CoreType\HiddenType::class, array(
        'data' => json_encode($extraChoice), // for the example but you may rely on a serializer
        'mapped' => false,
        'attr' => array(
            'data-entity-id' => $extraChoice->getId(),
            // ...
            'data-entity-value' => $extraChoice->getName(),
        ),
    ));
}

Could also be done easily in twig with simple div tags using $moreChoices in the view.

You could of courses acheive this globally using form events and keep this edge case logic in an appropriate listener. The Form Component allows you to add any custom mappers and transformers.

In any case you should rely on a second controller for xhr so you may not need form's help there to deal with submitted data.

However, you could not get my first example working because actually you cannot validate choice fields if options do not match predefined ones. $moreOptions are added in the form on client side and won't get mapped in the fist controller. To make it work you need a third controller which create a form holding a choice field with all the entities (here come the EntityType ;).

Aside
That's why I suggested an allow_add option for ChoiceType, but anyway this use case should not be solved by the widget option, this is another issue.

A form type IMO looks more like a structural map of some data to handle (server side) matching some form fields in a view to interact with. I don't think it should hold the logic for user's interactions on client side, but just help you to do it and it already does it very well.

Other examples
You can see another example of use case showing what you already can do with EntityType thanks to options introduced in #14050 with this question on stack overflow.

Here another example of form holding some entities as choices with a set of checkboxes, some with data in hidden inputs and some in sub forms as custom form types from a controller (it all could be defined in a custom form type as well :) :

// in a controller assuming you already got all entities and a builder :
use AppBundle\Form\Type as AppType;
foreach ($entities as $entity) {
    $choices = array();
    $reviews = array();
    $supplies = array();
    $specials = array();

    switch ($entity->getStatus()) {
        case Entity::STATUS_READY:
        $choices[] = $entity;
        break;
    case Entity::STATUS_NEEDS_REVIEW:
        $reviews[] = $entity;
            break;
    case Entity::STATUS_SUPPLY:
        $supplies[] = $entity;
            break;
    case Entity::STATUS_SPECIAL:
        $specials[] = $entity;
            break;
    } // ignore others
}
// Then we add a field to choose to affect or not some ready entities
$builder->add('entity', CoreType\ChoiceType::class, array(
    'choices' => $choices,
    'expanded' => true,
    'multiple' => true,
));
// Add a sub form to validate one
foreach ($reviews as $idx => $review) {
    $builder->add('review_'.$index, AppType\ReviewEntityType::class, array('data' => $review));
}
// and another sub form to edit one
foreach ($supplies as $idx => $supply) {
    $builder->add('supply_'.$index, AppType\SupplyEntityType::class, array('data' => $supply));
}
// Finally add some hidden input for a special treatment on client side
foreach ($specials as $idx => $special) {
    $builder->add('special_'.$index, CoreType\HiddenType::class, array(
            'data' => $special->getId(),
            'attr' => array(
                'data-property' => $special->getProperty(),
                // ...
    ));
}

You could also add some custom submit button to help javascript interactions.
It's even possible use the attr option of submit type to custom "formaction" html attribute, so each forward to a different route.

The third use case

  • I'll make it short for this one case by saying that adding an id of a parent entity in another entity form should be done directly is the view as it's the place where you should have everything you need there (movie variable, comments form ...).
    Anyway, it's seems as easy in a controller :
public function movieAction($id) {
    // get your movie by its id (or use a param converter)
    $buiilder = $this->createForm('AppType\CommentsCollectionType', array('movide_id' => $id))
        ->add('movie_id', HiddenType::class) // could be in the custom form type but it's for the example
        // ...

Conclusion

Changing the core of ChoiceType for these use cases looks wrong to me unless it's consistent with other choices types (e.g. EntityType, CountryType, LanguageType ...) since hidden input cannot be selected as choice in a form anyway I don't see the point of a widget option as hidden in ChoiceType.

This issue is about changing the way we define expanded by using widget as select, checkbox or radio other options may not be consistent with the idea of choice.

Data to be validated needs to match one or many choices of the predefined choices, in an attempt to use a "new" text option as widget an user should know what to choose and the label should indicate that. This is what needs to get fix in #17646 IMHO.

Contributor

Gregwar commented Feb 11, 2016

@HeahDude I never said changing ChoiceType was the better way to achieve these use cases
I was just developping and noticied that, something was kind of missing because it seems odd to have a field type that basically populate a select or so with all the entities that result a query and not being able to identify entity with other mean
I actually don't care how this should be implemented in Symfony, (my first proposal was actually about adding a separate form field type (entity identifier)), I just this this would be a good feature (see #1951 (comment))

Contributor

bamarni commented Feb 29, 2016

What about having a separate type instead? The original feature is basically just a text type with a custom data transformer.

While implementing this in the choice type I've found it confusing sometimes (eg. expanded option not always meaningful, no concept of label / value for some widgets), a new type would avoid deprecation and make it easier to review for the core team, it's hanging since 2.2 now...

Contributor

MacDada commented Mar 30, 2016

The original feature is basically just a text type with a custom data transformer.

@bamarni Yep, I just did that: https://gist.github.com/MacDada/33ab6b74ae681ff83854bd542fb209d3

But now I'm working on a nicer validation handling than error 500 because of the transformer failure ;-)

Contributor

bamarni commented Mar 31, 2016

Nice, I think your snippet goes in a better direction than my PR. I'm not sure about the multiple option though, in case of a hidden field filled programatically, it might be easier to deal with a json array for example.

I know it feels like going back to the starting point, but to me the form type from @Gregwar is the best option here. This issue is pretty old and maybe @webmozart also changed his mind about this feature. Hopefully we'll get some clear feedback from the core team at some point.

Member

HeahDude commented Mar 31, 2016

@MacDada Yes the data transformers of choice type and entity type are broken because of this regression

Comment this line and you'll be just fine until next release :)

Member

HeahDude commented Mar 31, 2016

@bamarni I agree, it should rely in another form type

Glideh commented Jun 10, 2016

This discussion is very interesting, however finally how do we do HiddenEntityType ?

My use case is managing a form with embedded forms in collection types with Javascript.
I need to select an entity for each child form and hide the selected id into the DOM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment