Skip to content

[Form] Added article for custom choice fields #13490

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

Conversation

HeahDude
Copy link
Contributor

@HeahDude HeahDude commented Apr 4, 2020

I propose to add a new page in the form section in master to fix both #6446 and #13120.

Copy link
Member

@weaverryan weaverryan left a comment

Choose a reason for hiding this comment

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

I like this a lot! I made some minor language tweaks directly (please review) and left 2 small comments.

@HeahDude HeahDude force-pushed the feature/form-choice_loader branch 10 times, most recently from b525b7a to e9ecb25 Compare April 4, 2020 22:28
Copy link
Member

@wouterj wouterj left a comment

Choose a reason for hiding this comment

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

Great! Thanks for writing this, I think it's a nice tip!

Please don't take the number of comments personally. Most are very very minor syntax related things.

Copy link
Contributor Author

@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.

Please don't take the number of comments personally. Most are very very minor syntax related things.

@wouterj No problem thanks for the thorough review :).

@HeahDude HeahDude force-pushed the feature/form-choice_loader branch from e9ecb25 to 65a905b Compare April 4, 2020 23:22
@HeahDude HeahDude force-pushed the feature/form-choice_loader branch 4 times, most recently from f881ff0 to e0771a6 Compare April 5, 2020 02:30
Copy link
Member

@javiereguiluz javiereguiluz left a comment

Choose a reason for hiding this comment

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

Very nice (and useful) article! Thanks for contributing it!

Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

some random notes

@@ -131,6 +65,14 @@ implement the ``getParent()`` method (Symfony will make the type extend from the
generic :class:`Symfony\\Component\\Form\\Extension\\Core\\Type\\FormType`,
which is the parent of all the other types).

.. note::

The PHP class extension mechanism and the Symfony form field extension
Copy link
Member

Choose a reason for hiding this comment

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

The PHP class extension mechanism

I don't know what this is. Do you mean inheritance?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree we should change that, but I propose to do it in #13488 which targets 3.4 instead. You're review is welcome there too :)

@nicolas-grekas
Copy link
Member

This PR has been closed because the master has been removed.
Please submit it again against the appropriate branch.

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.

[Form] Added an AbstractChoiceLoader to simplify implementations and ha… [RFC] [Form] custom choice loader article
6 participants