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

Do not fetch ContentViewhelper nodes recursively #157

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

sascha-egerer
Copy link

Content ViewHelpers should only be fetched as direct children, not recursively. Fetching them recursively in templates will not work if components are nested, as slots of the inner component would also be applied to the outer component. Placing content in an if-else block, thus defining the content twice, will also not work and would no longer be possible with this change, which should prevent some headaches.

Content ViewHelpers should only be fetched as direct children,
not recursively. Fetching them recursively in templates will
not work if components are nested, as slots of the inner component
would also be applied to the outer component. Placing content in an
if-else block, thus defining the content twice, will also not work
and would no longer be possible with this change, which should
prevent some headaches.
@s2b
Copy link
Collaborator

s2b commented Jul 3, 2024

Not sure about the general state of the tests, but this seems to break a lot of them that test slot functionality.

@sascha-egerer
Copy link
Author

Yes sorry it is broken. I’m working on it. I thought I did not yet open the PR. Will push a new version later and also add tests

@sascha-egerer sascha-egerer marked this pull request as draft July 3, 2024 11:04
@sascha-egerer sascha-egerer marked this pull request as ready for review July 3, 2024 21:52
@sascha-egerer
Copy link
Author

@s2b I have added two new tests with nested components that are using slots.
Maybe you can have a look and run the workflow

@ulrichmathes ulrichmathes force-pushed the master branch 2 times, most recently from 347e14d to c676bcd Compare October 24, 2024 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants