Skip to content
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

How to Submit a Form with Multiple Buttons #11777

Closed
LPodolski opened this issue Jun 18, 2019 · 12 comments
Closed

How to Submit a Form with Multiple Buttons #11777

LPodolski opened this issue Jun 18, 2019 · 12 comments

Comments

@LPodolski
Copy link

Documentation is incorrect in:
https://github.com/symfony/symfony-docs/blob/master/form/multiple_buttons.rst
https://symfony.com/doc/current/form/multiple_buttons.html

in code:

$form->getClickedButton()

this method do not exists

if you use example above, that creates $form variable:

$form = $this->createFormBuilder($task)
    ->add('task', TextType::class)
    ->add('dueDate', DateType::class)
    ->add('save', SubmitType::class, ['label' => 'Create Task'])
    ->add('saveAndAdd', SubmitType::class, ['label' => 'Save and Add'])
    ->getForm();

return type of $form variable would be FormInterface

and form interface does not contain method getClickedButton

  1. If method above would return Form, which has getClieckedButton method, then
/**
     * Returns the button that was used to submit the form.
     *
     * @return ClickableInterface|null
     */
    public function getClickedButton()

returns ClickableInterface

which does not have method getName

used in example:

$form->getClickedButton()->getName()
@xabbuh
Copy link
Member

xabbuh commented Jun 18, 2019

Did you run into a real issue during execution or is this about warnings some static code analyzer tools give you?

@LPodolski
Copy link
Author

LPodolski commented Jun 18, 2019

my IDE also does not see those methods, if you mean running PHP script then no, but still this code is not correct, because it uses methods outside interface it declares to return. And also I do not think that you should show examples in official documentation that fail static analysis. Because people will start suppressing this error. Because this static check would help if button would happen to be removed or something like that.

After some scratching of my head I changed code from example to something like this:

$button = $form->get('saveAndAdd');
if ($button->getClickedButton() instanceof SubmitButton && $button->getClickedButton()->isClicked()) {

and this passes static analysis and IDE sees all methods

@javiereguiluz
Copy link
Member

javiereguiluz commented Jun 19, 2019

Although it's true that FormInterface doesn't define the getClickedButton(), the Form class (which all form types inherit) does define it: https://github.com/symfony/symfony/blob/6f21ccf32324f5cd3f54725b800cdc045aa04ab0/src/Symfony/Component/Form/Form.php#L757 Would that be enough?

@LPodolski
Copy link
Author

in example code form variable comes from:

$form = $this->createFormBuilder($task)
    ->add('task', TextType::class)
    ->add('dueDate', DateType::class)
    ->add('save', SubmitType::class, ['label' => 'Create Task'])
    ->add('saveAndAdd', SubmitType::class, ['label' => 'Save and Add'])
    ->getForm();

->getForm();

in symfony code this method returns:

    /**
     * Creates the form.
     *
     * @return FormInterface The form
     */
    public function getForm();

https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Form/FormBuilderInterface.php#L84

I myself use and see more often this way of creating form:

        $form = $this->createForm(AdminCategorySaveType::class, $category);

but it still returns form interface

code in example would be correct, if method getForm above code would return implementation, like Form, but it does not do that and return interface, which is probably better, but it does not return used here method

I would think some about this and maybe report bug in symfony core, to add this method to interface, rather than change documentation, I just thought there is other correct way to do that. But as of now do not see one. So it is more likely that it is bug in symfony.

@xabbuh
Copy link
Member

xabbuh commented Jun 19, 2019

From the code side that's not a bug. We can just simply write the code this way as we know that a form type that is a SubmitType will be a SubmitButton after the form has been built.

In the docs we need to find a balance between what would make all static analyzers happy and wha adds more confusion for readers. That's why we chose to write code examples the way they are.

@jarbey
Copy link

jarbey commented Jan 19, 2020

+1 with LPodolski

It's wierd to me that $this->createForm returns FormInterface and not another interface that expose the real Form methods.

It means that we must "cast" the result of createForm into Form to access to methods like getClickedButton.
Not a real problem in PHP while there is no real "cast" at precompilation, but that not "clean".

Is there any reason why createForm does not return Form instead of FormInterface ? (or another Interface that From could implements).

By the way thanks, for you amazing job on symfony !

@OskarStark
Copy link
Contributor

@xabbuh we had the same discussion a few weeks ago. Can you find the issue and reference your explanation?

I am currently on a phone

Thanks

@xabbuh
Copy link
Member

xabbuh commented Jan 20, 2020

see symfony/symfony#35277 (especially symfony/symfony#35277 (comment))

@jarbey
Copy link

jarbey commented Jan 21, 2020

Thank you.

I personnaly use the annotation solution to tell the IDE that the type is Form and not FormInterface and avoid warning. The instanceof solution use more CPU at runtime ! ;-)

@carsonbot
Copy link
Collaborator

Thank you for this issue.
There has not been a lot of activity here for a while. Has this been resolved?

@carsonbot
Copy link
Collaborator

Hello? This issue is about to be closed if nobody replies.

@carsonbot
Copy link
Collaborator

Hey,

I didn't hear anything so I'm going to close it. Feel free to comment if this is still relevant, I can always reopen!

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

No branches or pull requests

6 participants