-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
[Cookbook] Fix doc on Generic Form Type Extensions #5227
Conversation
It's been more than a month, any news? |
Generic Form Type Extensions | ||
---------------------------- | ||
|
||
Although it is not possible to have a Form Type Extension applying to all Form |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
form type extension
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There, it's fixed.
87a0d4a
to
f1f5292
Compare
I fixed the typo mentioned by @xabbuh. If you have any other feedback to provide, I'd be glad to hear it. |
@lemoinem Can you please change that everywhere in the parts you modified (the same by the way also applies to "Form Type"). |
f1f5292
to
465ffdd
Compare
@xabbuh Done! (: |
@lemoinem This looks great now! Could you do another rebase please (sorry for that, but we had some activity on the repo the last days)? |
Avoid duplicating documentation by redirecting to the cookbook entry (as many other entries do).
465ffdd
to
a309be8
Compare
@xabbuh Thanks for the feedback! Here is the rebase. Enjoy! |
👍 |
👍 thanks for this @lemoinem Reducing doc duplication is a big plus for us! |
adding a "help" text to every field type); | ||
#. You want to add a **specific feature to a single type** (such | ||
as adding a "download" feature to the "file" field type). | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like these 2 bullet points - they quickly answer the question "what can I do with this form type extension thing?". I think you removed them because it says "every field type", but that's effectively true (the button being the exception). I'd like to have these back, and let the last paragraph explain the edge case.
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree this was a great way to explain Foem Type Extensions.
I restored them but changed the example. I now use ""adding a "help" text to every "input text"-like type"". This seem to be closer to an actual use case but also more truthful regarding what can be done.
87efc95
to
855301d
Compare
@weaverryan : Thanks for the feedback. Here is a new tentative version with a more positive wording. Let me know what you think about it. Once we all agree on a better version, I will squash the commits and rebase. |
@weaverryan @xabbuh |
Thanks for the ping! I just left one small comment - but a 👍 from me! A merger can even add that small change of we get to it first :) (unfortunately I'm on my phone now!) |
@weaverryan If everything is OK, I'll squash it all and rebase. |
…nem) This PR was squashed before being merged into the 2.3 branch (closes #5227). Discussion ---------- [Cookbook] Fix doc on Generic Form Type Extensions | Q | A | ------------- | --- | Doc fix? | yes | New docs? | no | Applies to | >=2.3 | Fixed tickets | N/A Follow up on #5154 (Sorry the first one didn't use the PR Template BTW) and symfony/symfony#14456. I cleaned up the reference on DIC Tags, I think referring to the cookbook article prevents duplicating documentation (and taking the risk it will get out of date or inconsistent later on, as it did). Then I removed the mention of Generic FTE at the top of the cookbook article. Since the feature doesn't exists anymore, I think it's better to leave it aside. I simply mentioned why it's impossible in a short last paragraph at the end of the CB entry. I kept both commit unsquashed because they seemed to address different enough concerns. But I will squash them upon request if you think it's necessary. Commits ------- a6bc497 [Cookbook] Fix doc on Generic Form Type Extensions
Thank you very much for the follow up! |
Follow up on #5154 (Sorry the first one didn't use the PR Template BTW) and symfony/symfony#14456.
I cleaned up the reference on DIC Tags, I think referring to the cookbook article prevents duplicating documentation (and taking the risk it will get out of date or inconsistent later on, as it did).
Then I removed the mention of Generic FTE at the top of the cookbook article. Since the feature doesn't exists anymore, I think it's better to leave it aside. I simply mentioned why it's impossible in a short last paragraph at the end of the CB entry.
I kept both commit unsquashed because they seemed to address different enough concerns. But I will squash them upon request if you think it's necessary.