Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Form] Added Form view parent method#19492

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
bhavin-nakrani wants to merge7 commits intosymfony:masterfrombhavin-nakrani:form-parent
Closed

[Form] Added Form view parent method#19492

bhavin-nakrani wants to merge7 commits intosymfony:masterfrombhavin-nakrani:form-parent

Conversation

@bhavin-nakrani
Copy link
Contributor

QA
Branch?"master" for new features
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets#18882
LicenseMIT
Doc PRreference to the documentation PR, if any

Adding FormView::hasParent() which should be hardcoded in twig to prevent accessing the property: form.hasParent.

ro0NL reacted with thumbs down emoji
@ogizanagi
Copy link
Contributor

Just asking, but what aboutisRoot() instead, as inFormInterface::isRoot() ?

@bhavin-nakrani
Copy link
ContributorAuthor

@ogizanagiFormInterface::isRoot() is not accessible from twig andFormView does not implement this interface.

@bhavin-nakrani
Copy link
ContributorAuthor

Does anyone knows why i am getting this error in buildAppVeyor build failed

bhavin-nakrani reacted with thumbs up emoji

@HeahDude
Copy link
Contributor

HeahDude commentedJul 31, 2016
edited
Loading

The problem here is that it does not make such sense to add methods likeisRoot orhasParent on a value object with public properties asFormView, and usingform.parent orform.root has the same effect in Twig that checking the public property so adding it to hardcode it looks weird somehow.

The real issue I guess come from Twig and how it tries to access an attributes becauseFormView has both public properties and array access, but the last has a precedence so it will first look for named children.

Maybe we should just better document how to access children via offset get when they have the same name as a view property (e.gform['parent'],form['children'],form['vars']) and suggest to avoid such collision?

/**
* @return array
*/
publicfunctionprovideCommandsAndOutput()
Copy link
Contributor

Choose a reason for hiding this comment

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

Your IDE can already determine the return type, in this state it doesn't really add a value

Copy link
Contributor

Choose a reason for hiding this comment

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

This fix does not belong to the PR's scope anyway.

@ogizanagi
Copy link
Contributor

ogizanagi commentedJul 31, 2016
edited
Loading

@ogizanagi FormInterface::isRoot() is not accessible from twig and FormView does not implement this interface.

@bhavin-nakrani : I was asking about naming the methodisRoot instead ofhasParent and reverse the logic inside 😅

The real issue I guess come from Twig and how it tries to access an attributes because FormView has both public properties and array access, but the last has a precedence so it will first look for named children.

Maybe we should just better document how to access children via offset get when they have the same name as a view property (e.g form['parent'], form['children'], form['vars']) and suggest to avoid such collision?

@HeahDude : IIUC, this won't solve the mentioned issue, whereform.parent will always return something in case the form has a child namedparent. Thus, no simple way to know if it's a child or a parent :/

@HeahDude
Copy link
Contributor

@ogizanagi Yes what I implicitly suggest here is to think about the precedence in Twig, checking object (properties and methods) in an early return before checking for array andArrayAccess, but that sounds like a big BC break (maybe doable with the upcoming 2.0?).

The opposite is not working either because if one wants to test if a form has a child named "parent" it may return the actual parent even usingform['parent'] because[] has the same behavior for objects since it is the only way to make the call dynamic using a twig variable to determine the property/method.

So it may worth to say a word in the doc about it anyway. Although hardcodinghasMethod might help to ensure the checks both ways but IMO it keeps looking like a hack adding this method inFormView to work around this.

ogizanagi reacted with thumbs up emoji

@ro0NL
Copy link
Contributor

Imo. it's worth reservingform.parent as is. A child named liked like that can be accessed viaform.children['parent'], same for havingform.root as proposed in#20369

@fabpot
Copy link
Member

Closing this PR as there does not seem to have any more activity and the PR cannot be merged as is. Feel free to reopen though.

fabpot added a commit that referenced this pull requestDec 4, 2017
…nd form fields (yceruto)This PR was merged into the 2.7 branch.Discussion----------[Form][TwigBridge] Fix collision between view properties and form fields| Q             | A| ------------- | ---| Branch?       | 2.7| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets |#18882| License       | MIT| Doc PR        | TODOThis introduce a new Twig test function `rootform` that guarantee the right access to the `parent` property of the form view. The rest of the properties (`vars` and `children`) are not used at least inside Symfony repo.I've chosen this solution because it doesn't [affect the design of the form view class/interface](https://github.com/symfony/symfony/pull/19492/files#diff-f60b55ea46e40b9c4475a1bd361f6940R168) and because [the problem happen only on Twig](https://github.com/twigphp/Twig/blob/fd98722d15996561f598f9181dbcef8432e9ff85/lib/Twig/Extension/Core.php#L1439-L1447).More details about the problem here:*#24892*#19492*#23649 (comment)_if this is approved_ we should update also:* [`foundation_5_layout.html.twig`](https://github.com/symfony/symfony/blob/336600857b8bb47d5a7ba3d1924a0e7a3e2af7a8/src/Symfony/Bridge/Twig/Resources/views/Form/foundation_5_layout.html.twig#L321-L326) in `3.3` (done in#25305)* [`bootstrap_4_layout.html.twig`](https://github.com/symfony/symfony/blob/76d356f36a692dd8ad796de363484c97d6731d1f/src/Symfony/Bridge/Twig/Resources/views/Form/bootstrap_4_layout.html.twig#L176) in `3.4` (done in#25306)Commits-------8505894 Fix collision between view properties and form fields
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Projects

None yet

Milestone

3.3

Development

Successfully merging this pull request may close these issues.

8 participants

@bhavin-nakrani@ogizanagi@HeahDude@ro0NL@fabpot@linaori@nicolas-grekas@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp