-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
[Forms] Added data-mapper docs #10756
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
Conversation
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.
great work 👍 just some small minor comments
|
||
The red, green and blue form fields have to be mapped to the constructor | ||
arguments and the ``Color`` instance has to be mapped to red, green and blue | ||
form fields. Recognize a familiar pattern? It's time for a data mapper! |
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.
Maybe end this sentence with a colon.
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 think that !
brings some more joy and interaction here :)
form/data_mappers.rst
Outdated
|
||
.. tip:: | ||
|
||
You can also implement ``DataMapperInterface`` in the ``ColorType`` and add |
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.
[...] implement the DataMapperInterface
[...]
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.
Should we also talk about the possibility to implement the data mapper as an anonymous class?
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 think in this very case when we have two rather complicated methods to implement, an anonymous class will be harder to read.
@xabbuh , thank you for review, addressed your comments. |
Thank you @vudaltsov. |
This PR was squashed before being merged into the 3.4 branch (closes #10756). Discussion ---------- [Forms] Added data-mapper docs #SymfonyConHackday2018 Improved version of #6900 Thank you @wouterj for a great job and the initial text! Thanx @HeahDude for helping me with this PR. Commits ------- b1cb1c0 [Forms] Added data-mapper docs
#SymfonyConHackday2018
Improved version of #6900
Thank you @wouterj for a great job and the initial text! Thanx @HeahDude for helping me with this PR.