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

Clarify example for SUBMIT form event #5996

Merged
merged 1 commit into from
Dec 12, 2015

Conversation

bkosborne
Copy link
Contributor

Q A
Doc fix? yes
New docs? no
Applies to 2.3+
Fixed tickets #5990

It's indicated in the docs that form field cannot be added/removed by listeners of the SUBMIT form event, but the example provided was removing fields. While it's possible to do so, it's not recommended since it has little effect. Replaced with a different example implementation.

It's indicated in the docs that form field cannot be added/removed by listeners of the SUBMIT form event, but the example provided was removing fields. While it's possible to do so, it's not recommended since it has little effect. Replaced with a different example implementation.
@ogizanagi
Copy link
Contributor

👍

@xabbuh
Copy link
Member

xabbuh commented Dec 10, 2015

👍 Thanks for submitting a patch so quickly @bkosborne! This looks good to me.

@wouterj wouterj merged commit 4794492 into symfony:2.3 Dec 12, 2015
wouterj added a commit that referenced this pull request Dec 12, 2015
This PR was merged into the 2.3 branch.

Discussion
----------

Clarify example for SUBMIT form event

| Q             | A
| ------------- | ---
| Doc fix?      | yes
| New docs?     | no
| Applies to    | 2.3+
| Fixed tickets | #5990

It's indicated in the docs that form field cannot be added/removed by listeners of the SUBMIT form event, but the example provided was removing fields. While it's possible to do so, it's not recommended since it has little effect. Replaced with a different example implementation.

Commits
-------

4794492 Clarify example for SUBMIT form event
wouterj added a commit that referenced this pull request Dec 12, 2015
@wouterj
Copy link
Member

wouterj commented Dec 12, 2015

Cool! Thanks @bkosborne.

I've done a very minor rewording in 979336b, as I think it reads a bit better. I'm not a native though, so feel free to leave a comment if you have a better suggestion.

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.

4 participants