-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
{% if label is empty %} | ||
{% set label = name|humanize %} | ||
{% endif %} | ||
<input type="submit" {{ block('button_attributes') }} value="{{ label }}" /> |
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.
Apart from a few missing tests, this PR is pretty much complete now. Feedback welcome. |
*/ | ||
public function buildView(FormView $view, FormInterface $form, array $options) | ||
{ | ||
/* @var \Symfony\Component\Form\ClickableInterface $form */ |
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.
@bschussek could be a good nice to have... after the 100+ form issues are tackled ! |
Support for validation groups in submit buttons added. |
This PR is now complete. I adapted the description above. |
@@ -53,6 +53,8 @@ public function buildForm(FormBuilderInterface $builder, array $options) | |||
*/ | |||
public function setDefaultOptions(OptionsResolverInterface $resolver) | |||
{ | |||
parent::setDefaultOptions($resolver); | |||
|
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
@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 👍 |
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). |
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. |
@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. |
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);
return true;
} 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. |
Postponed to 2.3 |
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. |
@@ -218,6 +218,36 @@ | |||
{% endspaceless %} | |||
{% endblock email_widget %} | |||
|
|||
{% block button_widget %} | |||
{% spaceless %} | |||
{% if type is not defined or type is empty %} |
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
The latest build fail seems to be unrelated to the content of this PR. The build succeeded on 5.4. |
@bschussek see #7662 |
…ble validation. This is identical to setting it to an empty array.
Rebased again. |
+1 Usefull to ease the creation of multi step forms. It would be interesting to write a simple usage case sample for multipage forms. |
👍 Very useful to detect the clicked button (eg: save / preview). What is the BC break you mention in the description ? |
@GromNaN copied from the changelog: |
@stof Good point, added this. |
This PR was merged into the master branch. Discussion ---------- [2.3] [Form] Support buttons in forms 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: ```php $builder->add('clickme', 'submit'); ``` You can then check in the controller whether the button was clicked: ```php if ($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" => false d504732 [Form] Added leading backslashes to @exceptionMessage doc blocks c8afa88 [Form] Removed deprecated code scheduled for removal in 2.3 36ca056 [Form] Simplified Twig code ce29c70 [Form] Fixed incorrect doc comment 0bc7129 [Form] Fixed invalid use of FormException 600007b [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 (see 7c47e34) 7b07925 [Form] Changed isset() to array_key_exists() to be consistent with ParameterBag 7b438a8 [Form] Made submit buttons able to convey validation groups cc2118d [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 | #3358 Commits ------- 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
false
in the optionvalidation_groups
:The same can be achieved (already before this PR) by passing an empty array:
See the linked documentation for more information.