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

Remove isSubmitted call #5970

Closed
wants to merge 2 commits into from
Closed

Remove isSubmitted call #5970

wants to merge 2 commits into from

Conversation

DanielSiepmann
Copy link
Contributor

Streamline with http://symfony.com/doc/current/book/forms.html#handling-form-submissions
There is no need for isSubmitted as this is done inside isValid

Streamline with http://symfony.com/doc/current/book/forms.html#handling-form-submissions
There is no need for `isSubmitted` as this is done inside `isValid`
@dosten
Copy link
Contributor

dosten commented Dec 7, 2015

But this is intended, because is considered a best practice. http://symfony.com/doc/current/best_practices/forms.html

We recommend using $form->isSubmitted() in the if statement for clarity. This isn't technically needed, since isValid() first calls isSubmitted(). But without this, the flow doesn't read well as it looks like the form is always processed (even on the GET request).

@DanielSiepmann
Copy link
Contributor Author

Okay ... but then this should be changed in the referenced documentation http://symfony.com/doc/current/book/forms.html#handling-form-submissions, shouldn't it?

@dosten
Copy link
Contributor

dosten commented Dec 7, 2015

Yes, you're right. If you can do it would be great!

@DanielSiepmann
Copy link
Contributor Author

Done within #5972 I'll close this pull request.

To follow best practice swap order of calls.
@DanielSiepmann
Copy link
Contributor Author

Following #5972 I've swapped the order of call.

@xabbuh
Copy link
Member

xabbuh commented Dec 7, 2015

👍

@wouterj
Copy link
Member

wouterj commented Dec 7, 2015

Great, thanks Daniel!

wouterj added a commit that referenced this pull request Dec 7, 2015
This PR was submitted for the 3.0 branch but it was merged into the 2.3 branch instead (closes #5970).

Discussion
----------

Remove isSubmitted call

Streamline with http://symfony.com/doc/current/book/forms.html#handling-form-submissions
There is no need for `isSubmitted` as this is done inside `isValid`

Commits
-------

cebd5fd Remove isSubmitted call
@wouterj wouterj closed this Dec 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants