merged branch vicb/form-proto (PR #1315)

Commits
-------

07fa82d [Form] Revert changes impacting perfomance and readability
b709551 [Order] Make Form::types and FormView::types use the same order (Parent > Child)
e56dad6 [Form] simplify the code
bdd755e [Form] Fix the exception message when no block is found
c68c511 [Form] Make theming form prototypes consistent (start by looking for a '_<id>_<section>' block)
9ec9960 [Form] Simplify the code
4e3e276 [Form] Make the prototype view child of the collection view

Discussion
----------

[Form] Make the prototype view child of the collection view

This PR should be a base for discussion.

The [current implementation](https://github.com/symfony/symfony/pull/1188) has some drawbacks because the prototype view is not a child of the collection view:

  * The 'multipart' attribute is not propagated from the prototype to the collection,
  * The prototype view do not use the theme from the collection.

Those 2 points are fixed by the proposed implementation and one more benefit is that the template markup might be easier to work with:

before:

```html
<div id="form_emails">
  <div>
    <label for="form_emails_0">0</label>
    <input type="email" id="form_emails_0" name="form[emails][0]" value="a@b.com">
  </div>
  <script type="text/html" id="form_emails_prototype">
    <div>
      <label for="$$name$$">$$name$$</label>
      <input type="email" id="$$name$$" name="$$name$$" value="" />
    </div>
  </script>
</div>
```
after:

```html
<div id="form_emails">
  <div>
    <label for="form_emails_0">0</label>
    <input type="email" id="form_emails_0" name="form[emails][0]" value="a@b.com">
  </div>
  <script type="text/html" id="form_emails_prototype">
    <div>
      <label for="form_emails_$$name$$">$$name$$</label>
      <input type="email" id="form_emails_$$name$$" name="form[emails][$$name$$]" value="" />
    </div>
  </script>
</div>
```

@kriswallsmith I'd like to get your feedback on this PR. thanks.

---------------------------------------------------------------------------

by stof at 2011/06/14 07:01:01 -0700

@fabpot any ETA about merging it ? Using the prototype currently is a pain to build the name. The change makes it far easier

---------------------------------------------------------------------------

by fabpot at 2011/06/14 07:09:46 -0700

The templates are much better but I'm a bit concerned that we need to add the logic into the Form class directly. That looks quite ugly. If there is no other way, I will merge it.

---------------------------------------------------------------------------

by vicb at 2011/06/14 07:14:32 -0700

I have found no better way... I am testing some minor tweaks I want to submit.

---------------------------------------------------------------------------

by kriswallsmith at 2011/06/14 07:34:25 -0700

I'm not happy with the code in Form.php either... would creating a PrototypeType accomplish the same thing?

---------------------------------------------------------------------------

by vicb at 2011/06/14 07:42:07 -0700

@kriswallsmith tried and dismissed, the id and name are bad & you have to go for `render_widget(form.get('proto'))` in the template. That should be fixeable but not any better.

---------------------------------------------------------------------------

by kriswallsmith at 2011/06/14 07:45:21 -0700

What do you mean the id and name are bad? If we have a distinct type for the prototype, can't we do whatever we want using buildView() and the template?

---------------------------------------------------------------------------

by vicb at 2011/06/14 07:53:31 -0700

@kriswallsmith the id would be smthg like `form_emails_$$name$$_prototype` but yes we should be able to do whatever we want but the code might end up being more complex.

I am done with the tweaks but still open to feedback on this PR.

---------------------------------------------------------------------------

by kriswallsmith at 2011/06/14 08:08:21 -0700

Yes, that is the type of name I would expect.

---------------------------------------------------------------------------

by kriswallsmith at 2011/06/14 08:08:33 -0700

Oops -- I mean id.

---------------------------------------------------------------------------

by kriswallsmith at 2011/06/14 08:09:42 -0700

Maybe I'm confused what id you're referring to. I'll try to spend some time on this today.

---------------------------------------------------------------------------

by vicb at 2011/06/14 08:23:56 -0700

That should be the id of the `<input>`, the id of the script would be `form_emails_$$name$$_prototype_prototype` (if prototype is the name of the nested node).

I am trying to setup a branch with my code (playing with git & netbeans local history)

---------------------------------------------------------------------------

by vicb at 2011/06/14 08:46:25 -0700

@kriswallsmith https://github.com/vicb/symfony/tree/kris/proto if that can help (there are still changes in Form.php)

---------------------------------------------------------------------------

by kriswallsmith at 2011/06/14 08:47:08 -0700

Thanks, I'll take a look.

---------------------------------------------------------------------------

by vicb at 2011/06/15 00:48:38 -0700

I would have expected it to be faster however `array_map` is about twice slower... reverted !

src/Symfony/Bridge/Twig/Extension/FormExtension.php

@@ -226,12 +226,12 @@ class FormExtension extends \Twig_Extension
 
         $blocks = $this->getBlocks($view);
         $types = $view->get('types');
-        array_unshift($types, '_'.$view->get('id'));
+        $types[] = '_'.$view->get('proto_id', $view->get('id'));
 
-        foreach ($types as $type) {
-            $block = $type.'_'.$section;
+        for ($i = count($types) - 1; $i >= 0; $i--) {
+            $types[$i] .= '_'.$section;
 
-            if (isset($blocks[$block])) {
+            if (isset($blocks[$types[$i]])) {
 
                 $this->varStack[$view] = array_replace(
                     $view->all(),
@@ -239,7 +239,7 @@ class FormExtension extends \Twig_Extension
                     $variables
                 );
 
-                $html = $this->template->renderBlock($block, $this->varStack[$view], $blocks);
+                $html = $this->template->renderBlock($types[$i], $this->varStack[$view], $blocks);
 
                 if ($mainTemplate) {
                     $view->setRendered();
@@ -251,7 +251,7 @@ class FormExtension extends \Twig_Extension
             }
         }
 
-        throw new FormException(sprintf('Unable to render form as none of the following blocks exist: "%s".', implode('", "', $blocks)));
+        throw new FormException(sprintf('Unable to render form as none of the following blocks exist: "%s".', implode('", "', $types)));
     }
 
     /**

src/Symfony/Bridge/Twig/Resources/views/Form/div_layout.html.twig

@@ -260,11 +260,8 @@
 {% endspaceless %}
 {% endblock email_widget %}
 
-{% block collection_widget %}
+{% block prototype_row %}
 {% spaceless %}
-    {{ block('form_widget') }}
-    {% if prototype is defined %}
-    <script type="text/html" id="{{ id }}_prototype">{{ form_row(prototype) }}</script>
-    {% endif %}
+    <script type="text/html" id="{{ proto_id }}">{{ block('field_row') }}</script>
 {% endspaceless %}
-{% endblock collection_widget %}
+{% endblock prototype_row %}

src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/collection_widget.html.php

@@ -1,5 +0,0 @@
-<?php echo $view->render('FrameworkBundle:Form:form_widget.html.php', array('form' => $form)) ?>
-
-<?php if (isset($prototype)): ?>
-<script type="text/html" id="<?php echo $view->escape($id) ?>_prototype"><?php echo $view['form']->row($prototype) ?></script>
-<?php endif; ?>

src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/prototype_row.html.php

@@ -0,0 +1,3 @@
+<script type="text/html" id="<?php echo $view->escape($proto_id) ?>">
+    <?php echo $view->render('FrameworkBundle:Form:field_row.html.php', array('form' => $form)) ?>
+</script>

src/Symfony/Bundle/FrameworkBundle/Templating/Helper/FormHelper.php

@@ -202,29 +202,25 @@ class FormHelper extends Helper
         }
 
         $template = null;
-        $blocks = $view->get('types');
-        array_unshift($blocks, '_'.$view->get('id'));
+        $types = $view->get('types');
+        $types[] = '_'.$view->get('proto_id', $view->get('id'));
 
-        foreach ($blocks as &$block) {
-            $block = $block.'_'.$section;
-            $template = $this->lookupTemplate($block);
+        for ($i = count($types) - 1; $i >= 0; $i--) {
+            $types[$i] .= '_'.$section;
+            $template = $this->lookupTemplate($types[$i]);
 
             if ($template) {
-                break;
-            }
-        }
-
-        if (!$template) {
-            throw new FormException(sprintf('Unable to render form as none of the following blocks exist: "%s".', implode('", "', $blocks)));
-        }
+                $html = $this->render($view, $template, $variables);
 
-        $html = $this->render($view, $template, $variables);
+                if ($mainTemplate) {
+                    $view->setRendered();
+                }
 
-        if ($mainTemplate) {
-            $view->setRendered();
+                return $html;
+            }
         }
 
-        return $html;
+        throw new FormException(sprintf('Unable to render form as none of the following blocks exist: "%s".', implode('", "', $types)));
     }
 
     public function render(FormView $view, $template, array $variables = array())

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

@@ -26,7 +26,7 @@ class CollectionType extends AbstractType
     {
         if ($options['allow_add'] && $options['prototype']) {
             $prototype = $builder->create('$$name$$', $options['type'], $options['options']);
-            $builder->setAttribute('prototype', $prototype->getForm());
+            $builder->setAttribute('prototype', $prototype);
         }
 
         $listener = new ResizeFormListener(
@@ -55,7 +55,7 @@ class CollectionType extends AbstractType
         ;
 
         if ($form->hasAttribute('prototype')) {
-            $view->set('prototype', $form->getAttribute('prototype')->createView());
+            $view->set('prototype', $form->getAttribute('prototype'));
         }
     }
 

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

@@ -76,7 +76,7 @@ class FieldType extends AbstractType
         }
 
         $types = array();
-        foreach (array_reverse((array) $form->getTypes()) as $type) {
+        foreach ($form->getTypes() as $type) {
             $types[] = $type->getName();
         }
 

src/Symfony/Component/Form/Form.php

@@ -907,7 +907,6 @@ class Form implements \IteratorAggregate, FormInterface
         $view->setParent($parent);
 
         $types = (array) $this->types;
-        $childViews = array();
 
         foreach ($types as $type) {
             $type->buildView($view, $this);
@@ -917,10 +916,22 @@ class Form implements \IteratorAggregate, FormInterface
             }
         }
 
+        $childViews = array();
+
         foreach ($this->children as $key => $child) {
             $childViews[$key] = $child->createView($view);
         }
 
+        if (null !== $prototype = $view->get('prototype')) {
+            $protoView = $prototype->getForm()->createView($view);
+            $protoTypes = $protoView->get('types');
+            $protoTypes[] = 'prototype';
+            $childViews[$prototype->getName()] = $protoView
+                ->set('types', $protoTypes)
+                ->set('proto_id', $view->get('id').'_prototype');
+            ;
+        }
+
         $view->setChildren($childViews);
 
         foreach ($types as $type) {

tests/Symfony/Tests/Component/Form/Extension/Core/Type/CollectionTypeTest.php

@@ -121,11 +121,24 @@ class CollectionFormTest extends TypeTestCase
     public function testAllowAddButNoPrototype()
     {
         $form = $this->factory->create('collection', null, array(
-            'type' => 'field',
+            'type'      => 'field',
             'allow_add' => true,
             'prototype' => false,
         ));
 
         $this->assertFalse($form->has('$$name$$'));
     }
+
+    public function testPrototypeMultipartPropagation()
+    {
+        $form = $this->factory
+            ->create('collection', null, array(
+                'type'      => 'file',
+                'allow_add' => true,
+                'prototype' => true,
+            ))
+        ;
+
+        $this->assertTrue($form->createView()->get('multipart'));
+    }
 }