Skip to content

Update submit.rst #7772

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

Closed
wants to merge 1 commit into from
Closed

Update submit.rst #7772

wants to merge 1 commit into from

Conversation

RobertErobert
Copy link

@RobertErobert RobertErobert commented Apr 8, 2017

Add note that some examples are inherited from ButtonType::class #6864

Add note that some examples are inherited from ButtonType::class
Copy link
Contributor

@HeahDude HeahDude left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for opening this PR!
I've left some comments as I'm not sure about the wording, let's wait for other reviewers opinion, thanks!

@@ -34,6 +34,8 @@ useful when :doc:`a form has multiple submit buttons </form/multiple_buttons>`::
Inherited Options
-----------------

Note some examples come from the ButtonType documentation. If you want to use the SubmitType field, you have to use SubmitType::class instead of ButtonType::class to generate a submit button.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say:
"Note that some examples are inherited from the ButtonType documentation".

Also the use of ::class looks weird here, maybe just replace :: by a space?

@javiereguiluz
Copy link
Member

@DJWindless thanks for making this contribution. I think you found an interesting issue here. Instead of adding this help note as you propose, what do Symfony Doc maintainers think about this other idea: let's not include this help note and let's not show ButtonType examples for SubmitType or ResetType. The wrong include is just a small example. I think it's worth it to duplicate it in the other button type to avoid confusions.

@HeahDude
Copy link
Contributor

I agree with Javier, let's duplicate this small content.

@xabbuh
Copy link
Member

xabbuh commented Apr 19, 2017

I agree as well. The contents doesn't change that frequent anyway and it will be less confusing for the reader.

@javiereguiluz
Copy link
Member

@DJWindless I'm closing your PR in favor of #7824 ... but I thank you for creating this pull request and for forcing us to think about a solution for this issue. Thanks!

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.

5 participants