Skip to content
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

[Form] add a convenience method to get the parent form in Twig templates #28812

Merged
merged 1 commit into from Feb 13, 2019

Conversation

Projects
None yet
7 participants
@xabbuh
Copy link
Member

xabbuh commented Oct 11, 2018

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #28686
License MIT
Doc PR symfony/symfony-docs#10999
@ro0NL

This comment has been minimized.

Copy link
Contributor

ro0NL commented Oct 11, 2018

it's a BC break no? also ref #19492 (comment)

@xabbuh

This comment has been minimized.

Copy link
Member Author

xabbuh commented Oct 11, 2018

Why would that be a BC break?

@xabbuh

This comment has been minimized.

Copy link
Member Author

xabbuh commented Oct 11, 2018

Taking into account #19492 (comment) we could of course think about adding a Twig function instead if we do not want to add this method to the FormView class.

@ro0NL

This comment has been minimized.

Copy link
Contributor

ro0NL commented Oct 11, 2018

the order changes right? before a child named parent might be returned first, whereas now it's always the parent form

@ro0NL

This comment has been minimized.

Copy link
Contributor

ro0NL commented Oct 11, 2018

alternatively, getting rid of arrayaccess in formview would also do it towards consistent accessors in php as well as twig, but that's a bigger step :)

@xabbuh

This comment has been minimized.

Copy link
Member Author

xabbuh commented Oct 11, 2018

If you refer to form.parent, this still returns the child form as Twig considers ArrayAccess methods before looking at actually implemented methods. And form.getParent() just is not usable without this change.

alternatively, getting rid of arrayaccess in formview would also do it towards consistent accessors in php as well as twig, but that's a bigger step :)

I don't think that's a good idea DX wise. Being able to use form.child_name is quite convenient.

@yceruto

This comment has been minimized.

Copy link
Contributor

yceruto commented Oct 11, 2018

@xabbuh could we deprecating twig_is_root_form() test func then? I guess it wouldn't useful anymore, replacing it by {% form.getParent() is null %}.


In the other hand, I think the solution should be done in Twig context, because having a getter for a public property seems odd.

What about adding a new Twig function for access to the FormView public properties? Something like this:

public function getPropertyValue($object, string $propertyName)
{
        return $object->$propertyName;
}

for cases where the public property matches the name of the child form (ArrayAccess ambiguity in Twig context):

{# $formView->parent #}
{% if property_value(form, 'parent') is null %}

{# $formView->vars #}
{% if property_value(form, 'vars').attr.class is defined %}

even, other ArrayAccess objects (in userland) with a composition similar to FormView can benefit from it, too.

@xabbuh xabbuh force-pushed the xabbuh:issue-28686 branch from f79f941 to ff1680d Oct 31, 2018

@xabbuh

This comment has been minimized.

Copy link
Member Author

xabbuh commented Oct 31, 2018

I have rewritten this solution to add a new parent_form() Twig function instead. I would not deprecate the filter for now. Both the filter and the new function serve their own use cases.

@xabbuh xabbuh force-pushed the xabbuh:issue-28686 branch from ff1680d to a88f55d Oct 31, 2018

@ro0NL

ro0NL approved these changes Nov 1, 2018

@yceruto

yceruto approved these changes Nov 1, 2018

@javiereguiluz

This comment has been minimized.

Copy link
Member

javiereguiluz commented Nov 7, 2018

@xabbuh please, create an issue in Symfony Docs to not forget about this. Also, please add some usage examples and if this improves an existing feature add a before/after example. Finally, please check if this fixes something that the current docs say it's not possible to do or buggy. Thanks!

@xabbuh xabbuh force-pushed the xabbuh:issue-28686 branch from a88f55d to cb60642 Jan 16, 2019

@fabpot

fabpot approved these changes Feb 13, 2019

@fabpot

This comment has been minimized.

Copy link
Member

fabpot commented Feb 13, 2019

Thank you @xabbuh.

@fabpot fabpot merged commit cb60642 into symfony:master Feb 13, 2019

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details

fabpot added a commit that referenced this pull request Feb 13, 2019

feature #28812 [Form] add a convenience method to get the parent form…
… in Twig templates (xabbuh)

This PR was merged into the 4.3-dev branch.

Discussion
----------

[Form] add a convenience method to get the parent form in Twig templates

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #28686
| License       | MIT
| Doc PR        |

Commits
-------

cb60642 add a convenience method to get the parent form in Twig templates

@xabbuh xabbuh deleted the xabbuh:issue-28686 branch Feb 13, 2019

@xabbuh

This comment has been minimized.

Copy link
Member Author

xabbuh commented Feb 16, 2019

@javiereguiluz it took me some time, but here we go now: symfony/symfony-docs#10999

javiereguiluz added a commit to symfony/symfony-docs that referenced this pull request Feb 16, 2019

minor #10999 document the parent_form() Twig function (xabbuh)
This PR was merged into the master branch.

Discussion
----------

document the parent_form() Twig function

documents symfony/symfony#28812

Commits
-------

2ea9797 document the parent_form() Twig function
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.