Skip to content

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!

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