[Form][TwigBridge] Fix collision between view properties and form fields #25236
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I tried something similar with But is a new feature :) Should we add a method to get the root from, along with the test. |
|
OK as bug fix to me. |
@ro0NL I don't think so :) this fix a bug that affects all previous versions.
This is a new feature that will be great to have, a lot of third-party bundle check for |
|
(AppVeyor failure unrelated) |
|
LGTM 👍 |
xabbuh
approved these changes
Dec 4, 2017
HeahDude
approved these changes
Dec 4, 2017
ogizanagi
approved these changes
Dec 4, 2017
This was referenced Dec 4, 2017
fabpot
approved these changes
Dec 4, 2017
|
Thank you @yceruto. |
fabpot
added a commit
that referenced
this pull request
Dec 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 | TODO This 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
fabpot
added a commit
that referenced
this pull request
Dec 4, 2017
…nd form fields (yceruto) This PR was merged into the 3.3 branch. Discussion ---------- [Form][TwigBridge] Fix collision between view properties and form fields | Q | A | ------------- | --- | Branch? | 3.3 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Require #25236 merged in 3.3 Commits ------- 888b48a Fix collision between view properties and form fields
symfony-splitter
pushed a commit
to symfony/twig-bridge
that referenced
this pull request
Dec 4, 2017
…nd form fields (yceruto) This PR was merged into the 3.3 branch. Discussion ---------- [Form][TwigBridge] Fix collision between view properties and form fields | Q | A | ------------- | --- | Branch? | 3.3 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Require symfony/symfony#25236 merged in 3.3 Commits ------- 888b48a89c Fix collision between view properties and form fields
|
@yceruto Can you submit a PR on the docs to make the needed changes? Thanks. |
symfony-splitter
pushed a commit
to symfony/twig-bridge
that referenced
this pull request
Dec 4, 2017
…nd form fields (yceruto) This PR was merged into the 3.4 branch. Discussion ---------- [Form][TwigBridge] Fix collision between view properties and form fields | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Require symfony/symfony#25236 merged in 3.4 Commits ------- c330965cfb Fix collision between view properties and form fields
fabpot
added a commit
that referenced
this pull request
Dec 4, 2017
…nd form fields (yceruto) This PR was merged into the 3.4 branch. Discussion ---------- [Form][TwigBridge] Fix collision between view properties and form fields | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Require #25236 merged in 3.4 Commits ------- c330965 Fix collision between view properties and form fields
This was referenced Dec 4, 2017
Merged
Merged
Merged
Merged
Merged
javiereguiluz
added a commit
to symfony/demo
that referenced
this pull request
Dec 7, 2017
This PR was merged into the master branch. Discussion ---------- Use rootform test to check by root form view This is consistent with the latest changes in the form themes and avoids any future collision between form view properties and form fields. See symfony/symfony#25236 Commits ------- bb11ddf Use rootform test to check by root form view
sayjun0505
added a commit
to sayjun0505/sym_proj
that referenced
this pull request
Apr 16, 2023
This PR was merged into the master branch. Discussion ---------- Use rootform test to check by root form view This is consistent with the latest changes in the form themes and avoids any future collision between form view properties and form fields. See symfony/symfony#25236 Commits ------- bb11ddf Use rootform test to check by root form view
spider-yamet
added a commit
to spider-yamet/sym_proj
that referenced
this pull request
Apr 16, 2023
This PR was merged into the master branch. Discussion ---------- Use rootform test to check by root form view This is consistent with the latest changes in the form themes and avoids any future collision between form view properties and form fields. See symfony/symfony#25236 Commits ------- bb11ddf Use rootform test to check by root form view
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This introduce a new Twig test function
rootformthat guarantee the right access to theparentproperty of the form view. The rest of the properties (varsandchildren) 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 and because the problem happen only on Twig.
More details about the problem here:
form.parentview in Twig is wrong #24892if this is approved we should update also:
foundation_5_layout.html.twigin3.3(done in [Form][TwigBridge] Fix collision between view properties and form fields #25305)bootstrap_4_layout.html.twigin3.4(done in [Form][TwigBridge] Fix collision between view properties and form fields #25306)