Commits
-------
e8326aa Renamed attributes to attr to be consistent with templating.
c707467 Added support for additional attributes in Form types that list field as their parent.
Discussion
----------
[Form] Added support for additional attributes
Added support for additional attributes in Form types that list field as their parent.
This is needed particularity for html5 data and data- attributes support, unfortunately $options['data'] is already taken, so adding a general $options['attributes'] is the easiest solution without breaking BC.
Now I know that this will be tempting for some to stuck style and class attributes here also, but I'd rather not restrict the keys that are passed in.
---------------------------------------------------------------------------
by Seldaek at 2011/05/22 14:51:02 -0700
Maybe it should be called attr for consistency with the template stuff, or the other should be renamed attributes. Other than that, I'm +1, data-* attributes are awesome, and abusers will find ways to abuse things either way.
---------------------------------------------------------------------------
by mvrhov at 2011/05/22 21:48:01 -0700
Naming it attr also crossed my mind when I was signing off yesterday.
Along with the possibility to go the way xml attributes are handled when node is converted to/from array.
So every option with @ prefix would automatically become html attribute. However going the latter path, it'd be harder to implement.
---------------------------------------------------------------------------
by fabpot at 2011/06/13 07:43:52 -0700
Can you give an example of a real-world use case? Thanks.
---------------------------------------------------------------------------
by Seldaek at 2011/06/13 07:54:51 -0700
You can use `array('attr' => array('data-foo' => 'bar'))`, which will output `data-foo="bar"`, which can be read easily by jQuery for example as `$('el').data('foo')`. It's a standard compliant and elegant way to pass extra data needed by the JS code along with DOM nodes, without polluting everything with script tags and all.
---------------------------------------------------------------------------
by fabpot at 2011/06/13 08:01:08 -0700
@Seldaek: I understand that. But why not doing this in the template?
---------------------------------------------------------------------------
by Seldaek at 2011/06/13 08:04:10 -0700
Well, I agree it most likely belongs in the template, but it's kind of data stuff that is not directly impacting the display rules of the element, so in some cases having the possibility to set that from the php code might be useful. Anyway I'll let @mvrhov answer maybe he had a more concrete use case. I just think it's nice to leave the door open, but I don't really need it.
---------------------------------------------------------------------------
by mvrhov at 2011/06/13 10:27:03 -0700
A bit late to the party. Ok, here is my use-case.
I have a pretty large form where part of the data is of tabular form. The number of rows is almost every time a lot larger than the number of columns
So I can either output a lot of text inputs filled with data and make already a large form intimidating. Or I can use a grid that supports editing. So I serialize that tabular data as json and put it as a value into one hidden field. Somehow I also have to get the column definitions to that grid. I decided to I serialize it and put it inside data-* attribute. Putting it into another hidden field doesn't make sense as when data is submitted back I don't need the column definitions as only the number of rows changes.
---------------------------------------------------------------------------
by fabpot at 2011/06/13 10:44:58 -0700
@mvrhov: ok, but what prevents you from doing this in the template directly?
---------------------------------------------------------------------------
by mvrhov at 2011/06/13 11:22:53 -0700
I have to get it into the view somehow. What I'd really not like to do is, iterate through that data in a controller and then pass it as another template variable.
---------------------------------------------------------------------------
by fabpot at 2011/06/14 00:10:22 -0700
But the controller is where you prepare the data that you want to send to the view. Without any concrete example, I'm going to close this PR.
---------------------------------------------------------------------------
by mvrhov at 2011/06/14 01:21:10 -0700
IMHO this has to go out through form as this is the part of the form also I do have a very slim controllers in this case 10 LOC, where half of them is just setting up an view data..
Nonetheless I went looking again very closely at the AbstractType and I do have buildView function available which I can override and set the column data I need there, and then provide custom view for that, so at least from this part this is an non issue.
With this PR all default form attributes can be set from outside and when searching for a good use-case I have found out that @henrikbjorn has implemented this via extensions [1], maybe he has a good use-case for this.
[1] - https://github.com/Comways/ComwaysFormExtraBundle/blob/master/Form/Extension/FieldTypeExtension.php
---------------------------------------------------------------------------
by henrikbjorn at 2011/06/14 01:48:53 -0700
Convenience is the only use case
---------------------------------------------------------------------------
by stof at 2011/06/14 02:08:09 -0700
@fabpot The issue is that passing it from the controller as another template variable makes it really hard when you use the type twice with different values. Passing them from the form would be the easiest way
---------------------------------------------------------------------------
by shuogawa at 2011/06/14 19:37:50 -0700
hello. thanks for great form library.
I want to support two ways to display additional attributes for form elements.
1) control in template like
{{ form_row(form.name, { 'width': '30' }) }}
2) control from php
$builder->add('name', 'text', array('attr'=>array('width'=>'30')));
If form elements configure by end user like cms,
and form elements dynamically change.
The second method is useful.
template designer can write {{form_row(form)}}
but
template designer can not write {{form_row(form.name), { 'width': '30' }}}
Thank you
---------------------------------------------------------------------------
by fabpot at 2011/06/14 23:01:18 -0700
@shuogawa: That's what I fear. Setting the `width` or any other attribute in PHP is wrong. This belongs to the templates.
---------------------------------------------------------------------------
by stloyd at 2011/06/15 00:09:05 -0700
@fabpot Then maybe just restrict allowed tags to `data-*` and don't use `attr` but some other not confusing name ? Just like we do with i.e. `maxlength`.
---------------------------------------------------------------------------
by fabpot at 2011/06/15 04:44:25 -0700
I'm going to merge it as a "convenience" tool, but the documentation should clearly state that the only usage should be for `data-*` attributes.
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 !
* kriswallsmith/form/collection-proto:
added script[type="text/html"] collection prototype to form themes
[Form] removed collection prototype from form tree
The current implementation is not ready for inclusion in 2.0. It has several
known problems (security, not possible to disable it, not "cloud-compatible",
...) and it's not a must have feature anyway.
Some references:
* Security issue in FileType: https://github.com/symfony/symfony/issues/1001
* Validation fails on file, still stored in TemporaryStorage: https://github.com/symfony/symfony/issues/908
* Add a size argument & ability to configure TemporaryStorage: https://github.com/symfony/symfony/pull/748
This feature should be reworked and discussed for inclusion in 2.1.
* CodeMeme/1058-fix-radio-input-listener:
Fix for RadioInputListener's empty value erroneously becoming extra data Refs #1058
Added test for RadioInputListener bug treating no data as extra data
The form component should now guarantee to always pass an UploadedFile object to your model. There you can call getOriginalName() to retrieve the original name of the uploaded file. For security reasons, the real file name is a generated hash value.
