Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[2.3] [Form] Support buttons in forms#6528
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
why not<button type="submit"> to be consistent with other buttons ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
There are problems with button as a submit in old versions of Internet Explorer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@norzechowicz Problems regarding submit buttons are happening only when you don't specify the type of them. There were browsers using "button" as default type whereas others were using "submit".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I changed it to<button type="submit"> now.
webmozart commentedJan 2, 2013
Apart from a few missing tests, this PR is pretty much complete now. Feedback welcome. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
this is wrong. It is not always a clickable one here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks, fixed.
vicb commentedJan 2, 2013
@bschussek could be a good nice to have... after the 100+ form issues are tackled ! |
webmozart commentedJan 2, 2013
Support for validation groups in submit buttons added. |
webmozart commentedJan 2, 2013
This PR is now complete. I adapted the description above. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
You forgot to remove the validation group normalizer here when moving it to the base class
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks, fixed
marijn commentedJan 2, 2013
@vicb I understand your frustration but I think your comment about the outstanding issues is uncalled for... Nice addition @bschussek! I've long wondered why buttons are not a part of the framework 👍 |
willdurand commentedJan 3, 2013
That's nice, really, it's really useful and well-designed, so thanks @bschussek. But as@vicb there are (a few) issues related to the Form component, and it would be nice to focus on fixing them (IMO). |
sstok commentedJan 5, 2013
Changing validation_group depending on the button is great, but I wonder if its a good idea to allow a event (onclick) when a button is clicked. I have some forms that have a 'cancel' button and depending which button is clicked 'submit' or cancel the form is either processed or the user is redirected to another page. But as binding/bounding a complete form to determine if cancel was clicked could a performance hit, I wonder if its better to keep using the 'old' method for that. And the 's label, its not uncommon to use HTML (Twitter Bootstrap for instance) knowing that Twig automatically escapes the label. |
webmozart commentedJan 5, 2013
@sstok What do you mean by that? Have you actually looked at the code of this PR?
Performance is not affected (significantly) by this PR. |
sstok commentedJan 5, 2013
A was talking in general, in JavaScript you can apply an event handler on a button for when its clicked. As an example I have form with a normal submit button for saving and a cancel button which when clicked redirects the user back to the viewing page. Currently I'm checking this with. if ($this->request->request->has('cancel')) {$isCancelled =true;return$this->onCancel($items) ?:true;}$this->form->bind($this->request);if ($this->form->isValid()) {$this->onConfirm($items);returntrue;} The form is only binded when its actually gonna be used, but as binding a form even tho I'm only checking if a button was checked could be performance hit (compared to what I'm using now). I was wondering if supporting events when clicked could be useful. It was just a thought. |
webmozart commentedJan 5, 2013
webmozart commentedJan 8, 2013
Postponed to 2.3 |
webmozart commentedApr 11, 2013
I rebased this branch on master now, fixed a few issues and wrote the documentation that is linked above. Unless reviews discover problems, this PR is ready for merging. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
There is a simpler way:type="{{ type|default('button') }}"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
not pushed yet?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
now it is
webmozart commentedApr 13, 2013
The latest build fail seems to be unrelated to the content of this PR. The build succeeded on 5.4. |
stof commentedApr 13, 2013
@bschussek see#7662 |
…ble validation. This is identical to setting it to an empty array.
webmozart commentedApr 13, 2013
Rebased again. |
raziel057 commentedApr 14, 2013
+1 Usefull to ease the creation of multi step forms. It would be interesting to write a simple usage case sample for multipage forms. |
GromNaN commentedApr 14, 2013
👍 Very useful to detect the clicked button (eg: save / preview). What is the BC break you mention in the description ? |
stof commentedApr 14, 2013
@GromNaN copied from the changelog: |
webmozart commentedApr 15, 2013
@stof Good point, added this. |
This PR was merged into the master branch.Discussion----------[2.3] [Form] Support buttons in formsBug fix: noFeature addition: yesBackwards compatibility break: yesSymfony2 tests pass: yesFixes the following tickets:#5383Todo: -License of the code: MITDocumentation PR:symfony/symfony-docs#2489The general idea of this PR is to be able to add buttons to forms like so:```php$builder->add('clickme', 'submit');```You can then check in the controller whether the button was clicked:```phpif ($form->get('clickme')->isClicked()) { // do stuff}```Button-specific validation groups are also supported:```php$builder->add('clickme', 'submit', array( 'validation_groups' => 'OnlyClickMe',));```The validation group will then override the one defined in the form if that button is clicked.This PR also introduces the disabling of form validation by passing the value `false` in the option `validation_groups`:```php$builder->add('clickme', 'submit', array( 'validation_groups' => false,));```The same can be achieved (already before this PR) by passing an empty array:```php$builder->add('clickme', 'submit', array( 'validation_groups' => array(),));```See the linked documentation for more information.Commits-------faf8d7a [Form] Added upgrade information about setting "validation_groups" => falsed504732 [Form] Added leading backslashes to @exceptionMessage doc blocksc8afa88 [Form] Removed deprecated code scheduled for removal in 2.336ca056 [Form] Simplified Twig codece29c70 [Form] Fixed incorrect doc comment0bc7129 [Form] Fixed invalid use of FormException600007b [Form] The option "validation_groups" can now be set to false to disable validation. This is identical to setting it to an empty array.277d6df [Form] Fixed concatenation operator CS (see7c47e34)7b07925 [Form] Changed isset() to array_key_exists() to be consistent with ParameterBag7b438a8 [Form] Made submit buttons able to convey validation groupscc2118d [Form] Implemented support for buttons
… option (xabbuh)This PR was merged into the 2.3 branch.Discussion----------[Reference] add description for the `validation_groups` option| Q | A| ------------- | ---| Doc fix? | no| New docs? | yes (symfony/symfony#6528)| Applies to | all| Fixed tickets |#3358Commits-------c41b17c add description for the `validation_groups` option
Bug fix: no
Feature addition: yes
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets:#5383
Todo: -
License of the code: MIT
Documentation PR:symfony/symfony-docs#2489
The general idea of this PR is to be able to add buttons to forms like so:
You can then check in the controller whether the button was clicked:
Button-specific validation groups are also supported:
The validation group will then override the one defined in the form if that button is clicked.
This PR also introduces the disabling of form validation by passing the value
falsein the optionvalidation_groups:The same can be achieved (already before this PR) by passing an empty array:
See the linked documentation for more information.