Skip to content

[Form] Add example for createNamed Form #11272

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 6, 2019
Merged

[Form] Add example for createNamed Form #11272

merged 1 commit into from
Apr 6, 2019

Conversation

wimme002
Copy link
Contributor

@wimme002 wimme002 commented Apr 2, 2019

The docs are not very clear where the FormFactory comes from and how to use this method

@OskarStark
Copy link
Contributor

Hi 👋

Thank you for your contribution! This should be considered a bugfix and target the 3.4 branch of the docs.

The sentence You can even suppress the name completely by setting it to an empty string:: does not match the following code example, as it does not show how to suppress the name, but shows how to set a custom name for the form.

Please keep in mind that we are using AppBundle and the Action-suffix for controller actions.

@OskarStark OskarStark added the Form label Apr 2, 2019
@OskarStark OskarStark changed the title Form: Add example for createNamed Form [Form] Add example for createNamed Form Apr 2, 2019
@wimme002 wimme002 changed the base branch from master to 3.4 April 2, 2019 12:14
@wimme002
Copy link
Contributor Author

wimme002 commented Apr 2, 2019

@OskarStark made the changes you proposed

OskarStark
OskarStark approved these changes Apr 2, 2019
View reviewed changes
Copy link
Contributor

@OskarStark OskarStark 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, this looks good to me 👍

yceruto
yceruto reviewed Apr 2, 2019
View reviewed changes
forms.rst Outdated
public function newAction()
{
$task = ...;
$form = $this->container->get('form.factory')->createNamed('name', TaskType::class, $task);
Copy link
Member

Choose a reason for hiding this comment

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

For short, we can use the shortcut instead $this->get(...).

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes that’s true, do we have a “standard” in the docs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The recommended way is to inject the factory with dependency injection but that will make this example maybe to complex.
$this->container looks in this case more clearly than $this->get

Copy link
Contributor

Choose a reason for hiding this comment

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

I also think using $this->get(...) makes sense here.

@wimme002 as long as you extend AbstractController this service is available in the service locator:

https://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php#L86

So imho no need to inject it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed!

Copy link
Member

Choose a reason for hiding this comment

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

Let's then slightly clearify this code example by putting a class DefaultController extends AbstractController { } block around the method. @wimme002 as you opened this PR, are you willing to push these 2 changes to your branch? We can then merge it. If not, just tell us and we'll take care of it!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made the requested changes

@HeahDude HeahDude added this to the 3.4 milestone Apr 6, 2019
OskarStark
OskarStark approved these changes Apr 6, 2019
View reviewed changes
Copy link
Contributor

@OskarStark OskarStark left a comment

Choose a reason for hiding this comment

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

We could do this change while merging

wimme002 added a commit to wimme002/symfony-docs that referenced this pull request Apr 6, 2019
@wimme002
symfony#11272 extend example for createNamed function
33a2fc5
@OskarStark OskarStark added the ⭐️ EU-FOSSA Hackathon https://symfony.com/blog/the-symfony-and-api-platform-hackathon-is-coming label Apr 6, 2019
@wouterj
Copy link
Member

wouterj commented Apr 6, 2019

Thank you Wim! Very quick responses and a good addition!

@wouterj wouterj merged commit 617621a into symfony:3.4 Apr 6, 2019
wouterj added a commit that referenced this pull request Apr 6, 2019
@wouterj
minor #11272 [Form] Add example for createNamed Form (wimme002)
dbc77f5 
This PR was squashed before being merged into the 3.4 branch (closes #11272).

Discussion
----------

[Form] Add example for createNamed Form

The docs are not very clear where the FormFactory comes from and how to use this method

<!--

If your pull request fixes a BUG, use the oldest maintained branch that contains
the bug (see https://symfony.com/roadmap for the list of maintained branches).

If your pull request documents a NEW FEATURE, use the same Symfony branch where
the feature was introduced (and `master` for features of unreleased versions).

-->

Commits
-------

617621a [Form] Add example for createNamed Form
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wouterj wouterj wouterj left review comments

@dmaicher dmaicher dmaicher approved these changes

@OskarStark OskarStark OskarStark approved these changes

@xabbuh xabbuh xabbuh approved these changes

@yceruto yceruto yceruto approved these changes

@HeahDude HeahDude HeahDude approved these changes

@maxhelias maxhelias maxhelias approved these changes

Assignees
No one assigned
Labels
⭐️ EU-FOSSA Hackathon https://symfony.com/blog/the-symfony-and-api-platform-hackathon-is-coming Form Status: Reviewed
Projects
None yet
Milestone
3.4
Development

Successfully merging this pull request may close these issues.
9 participants
@wimme002 @OskarStark @wouterj @dmaicher @xabbuh @yceruto @HeahDude @maxhelias @carsonbot