-
-
Notifications
You must be signed in to change notification settings - Fork 312
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
[TwigComponent] Add 'host' context when rendering embedded components #863
Conversation
|
I would expect: <twig:foo>
<twig:bar :deepVar="this.something" />
</twig:foo> To work though, since you are in Component's template. I think this is related to #844, @weaverryan, can you confirm? |
I actually agree with @kbond: {# component.html.twig #}
<twig:foo>
<twig:bar :deepVar="this.something" />
</twig:foo> I would imagine that class Component
public function render()
{
$this->renderComponent('foo', [
'content' => function() {
$this->renderComponent('bar', [
'deepVar' => $this->something(),
]
}
]);
} |
@kbond I purposely stayed away from parent, since the (content) block(s) are embedded and not normal extended blocks. I.o.w. there is no parent relationship, if looked at it from an embed perspective. That's actually why it's understandable that
@weaverryan @kbond I understand the expectation that That being said, walking away from using embedded blocks, and using a separate lexer mechanism as suggested in #844 could be a solution, but then it's not possible to refer to the Embedded Component itself. <twig:embedded_content_foo>
host from embedded content in Foo element: {{ host.something }}
this from embedded content in Foo element: {{ this.something }}
<twig:embedded_content_bar :propGivenThis="this.something" :propGivenHost="host.something" id='1' >
<twig:embedded_content_bar :propGivenThis="this.something" :propGivenHost="host.something" id='2' />
</twig:embedded_content_bar>
</twig:embedded_content_foo> So, you could look at the problem as: "it's not a bug, it's a feature!" 🙂 |
Note that the TwigPreLexer is turning <twig:foo>
<twig:bar :deepVar="this.something" />
</twig:foo> into {% component 'foo' %}
{% block content %}
{{ component('bar', { deepVar: this.something }) }}
{% endblock %}
{% endcomponent %} while {{ component('foo') }}
{% block content %}
{{ component('bar', {deepVar: this.something}) }}
{% endblock %}
I'm just saying that perhaps we need to understand what is happening within the "all components exist in their own isolated universe" paradigm. If you look at it like that, it completely makes sense that $this->renderComponent('foo', [
'content' => function() {
$this->renderComponent('bar', [
'deepVar' => 'this.something',
]
}
]); |
Yeah, I thought about this. In the context of html, parent is the correct term I think: <div>
<p>foo</p>
</div>
How do other front end frameworks handle this issue (both logically and naming)? |
Parent is perfectly fine by me, but I guess we first have to make sure that the compilation is still using the embed flow (see #844 (comment)). |
Seeing @weaverryan's latest comment in the related issue (#844 (comment)) and his & @kbond's desire to have |
Tbh, I'm not super motivated to make changes to the existing |
@weaverryan sounds good!
Is it something I can help with? I have been trying some things the last days, based on https://github.com/giorgiopogliani/twig-components. I'll open a new PR for that if you want to (when it's finished).
Can you elaborate a bit? It's best to know what should be avoided in the new version. |
Sorry guys, but working with it in a real project this Monday, I'm afraid I once again have to change my mind on the embed vs include stuff. I'll try to recap both options, and give the real example afterwards to support my preference (spoiler: it's embed after all). Using slotsMechanism
Syntax (concept){# someComponent.twig #}
{% twig_component Foo %}
some content {# This is used for `content` var in Foo.twig #}
{{ this.methodCall }} {# This will use SomeComponent::methodCall(), also part of `content` var in Foo.twig #}
{% slot namedSlot %}foobar{% slot %}
{% endtwig_component %} {# Foo.twig #}
<div class="foo">{{ content ?? 'default content' }} {{ namedSlot ?? 'foo' }}</div> PRO
CON
Using embed (current)Mechanism
Syntax{# someTemplate.twig #}
{% component Foo %}
some content {# This is used for `content` block in Foo.twig #}
{{ this.methodCall }} {# This will use Foo::methodCall(), also part of `content` block in Foo.twig #}
{{ parent.methodCall }} {# (optional) This will use SomeComponent::methodCall(), also part of `content` block in Foo.twig #}
{% block namedBlock %}foobar{% endblock %}
{% endcomponent %} {# Foo.twig #}
<div class="foo">{% block content%}default content{% endblock %} {% block namedBlock %}foo{% endblock %}</div> PRO
CON
The example to show the benefit of keeping embed{# Table.twig #}
<table class="{% block table_class %}table{% endblock %}">
{% block content %}{% endblock %}
</table> {# Body.twig #}
<tbody>
{% block content %}
{% for row in rows %}
<tr scope="row">
{% block row %}
{% for cell in row %}
<twig:cell>{{ cell }}</twig:cell>
{% endfor %}
{% endblock %}
</tr>
{% endfor %}
{% endblock %}
</tbody> {# Cell.twig #}
<td class="{% block td_class %}{% endblock %}">{% block content %}{% endblock %}</td> {# MyTable.twig #}
<twig:table>
<twig:head :headers="headers" :withAction="true" :sortableColumns="sortableColumns" />
<twig:body :rows="host.rows">
<twig:block name="row">
{% for cell in row %}
<twig:cell>{{ cell }}</twig:cell>
{% endfor %}
<twig:cell>
doing something special with the action column
<twig:block name="td_class">action</twig:block>
</twig:cell>
</twig:block>
</twig:body>
</twig:table> With an include (slot) syntax, it wouldn't be possible to access the PS: #844 is imo a case of not properly grasping embed vs include, because it is confusing, but shouldn't require any changes, except for perhaps adding documentation making that difference more clear leading to proper expectations. |
Hey @sneakyvv! I appreciate the objective detailed discovery that you're doing 👍. I admit that I am "partial" towards the "slot" idea, because I find the block hierarchy very confusing. But I'll do my best to stay objective also ;). By the way, I also think there are 2 separate "things" that are being discusses: (A) the embed block hierarchy and (B) the context inside of Let me challenge you on one point:
This refers to:
That looks really confusing to me - way too magical. The However, I admit that I can't think of a decent solution to this use-case. About #844, if things were kept as they are now, how would the correct code look for doing this: {# DangerButton.html.twig #}
{% component 'Button' with { type: 'danger' } %}
{% block content %}{{ block('content') }}{% endblock %}
{% endcomponent %} That is where someone is calling {# DangerButton.html.twig #}
{% set outerContent = block('content') %}
{% component 'Button' with { type: 'danger' } %}
{% block content %}{{ outerContent }}{% endblock %}
{% endcomponent %} It's not super elegant... /cc @WebMamba gotta include you here, as you're already working on the slot-type. I would still prefer the slot type - but the table/row use-case above deserves a nice solution. |
Hey, @sneakyvv thanks to raised all of this! |
@weaverryan I just add this test to illustrate your DangerButton example: https://github.com/symfony/ux/blob/4f95295caabd21471b8b6c913d5edc337ec91a9a/src/TwigComponent/tests/Integration/TwigComponentExtensionTest.php#LL86C1-L89C1 |
Thanks @WebMamba - that IS way nicer. Any idea on the table for situation above? I was thinking about this situation this morning, and though I love the simplicity of slots, the current solution is VERY powerful. so the question is: keep that power and document or try to find a solution to “forwarding” blocks into a component (setting the block to a variable above the component works, but isn’t great… but works) or continue the slot idea and try to find a solution for the table row problem. |
Ok so here is my solution for the Table problem. // index.html.twig
{% extends 'base.html.twig' %}
{% block body %}
<twig:Table :rows="{foo: ['bar', 'baz']}"/>
{% endblock %} // /components/Table.html.twig
<table>
<twig:TableBody :rows="rows"/>
</table> // /components/TableBody.html.twig
<tbody>
{% for row in rows %}
<tr scope="row">
{% block row %}
{% for cell in row %}
<twig:Cell>{{ cell }}</twig:Cell>
{% endfor %}
{% endblock %}
</tr>
{% endfor %}
</tbody> // components/Cell.html.twig
<td class="{{ attributes.merge({class: "td"}) }}">{{ slot }}</td> Ok now we have our simple component render. But now let's add some difficulties and take into account the @sneakyvv problem. 😁 So we want to add more flexibility to the Table component and give the ability to choose how to render the cell. Here is my solution for it: // index.html.twig
{% extends 'base.html.twig' %}
{% block body %}
<twig:Table :rows="{foo: ['bar', 'baz']}">
<twig:slot name="cell">
<twig:Cell>
<twig:slot-value name="cell_value"/>
</twig:Cell>
</twig:slot>
</twig:Table>
{% endblock %} So here we put the cell component into a slot inside the table component. And we use a new syntax <twig:slot-value, lets see later how it work. // components/Table.html.twig
<table>
<twig:TableBody :rows="rows">
<twig:slot name="cell">
{{ cell }}
</twig:slot>
</twig:TableBody>
</table> Here we just push again the cell component through an other slot // component/TableBody.html.twig
<tbody>
{% for row in rows %}
<tr scope="row">
{% block row %}
{% for cell in row %}
{{ cell.withContext({'cell_value': cell}) }}
{% endfor %}
{% endblock %}
</tr>
{% endfor %}
</tbody> And here we are! We render the slot that contains the cell, but with the function withContext. This function takes an array containing the key: cell_value and the value of the cell. And remember, we used: <twig:slot-value name="cell_value"/> so this gonna be replaced by the cell value, and everything will be rendered as expected. @weaverryan tell me what you think and I can push a commit to add this withContext method. |
Hey @weaverryan, @WebMamba, Sorry for the slow response, we're having a 4-day weekend here in Belgium and good weather so... 😎 😅 Anyhow... Here's my (once again, long (sry)) feedback:
In my mind that's always how you'd work with blocks, whether in an extend/include or embed way. That is, you'd look at the template you're using/overriding and replace some content within it, so indeed as if it's copied right in there. Most of the time those blocks don't really use the context where they're being replaced into, so the "magic" usage of those context variables isn't that common, but as you also noted it is a VERY powerful feature. Perhaps, again referring to the component's own "universe" helps to understand the magic (or not being confused by it). Now, about the content-in-content block issue (for now leaving @WebMamba's comments out of the equation):
I was thinking that the fact that a content block is being use inside a component could perhaps be detected and that the variable assignment in that case could be automized? Not sure if would be that easy though... I wouldn't necessarily document "Never use the block() function inside of {% component", or at least not as such. {# Template A #}
<twig:B>
My inner content for B {# content defined at top level #}
</twig:B>
{# Template B (component) #}
<twig:C>{%- block content -%}{%- endblock -%}</twig:C> {# passing content to a C component #}
{# Template C (component) #}
<div class="dressing up the content a bit in an atom fashion">{%- block content -%}{%- endblock -%}</div> The problem is actually that the content of a twig block is automatically used as a content block. It's from there that the logical consequence ensues that you shouldn't use a content block inside a twig component instance. So therefore, I would argue that you don't need to document the "logical thing". However, since this kind of (atom) composition will be a very common use case, I would document it perhaps within that frame of reference. That in those cases, i.e. cases where you have a twig component instance (C) inside a twig component's (B) template, and you want to define the content of C from template A, that in those cases you need to pass the content along via a new variable because (indeed) you cannot ever use a content block inside a component, because that is being translated into content-in-content, which is illegal. Either that or just use another name for the inner-content block, but then again that's not elegant to use in template A, because you can't write it as the body of a component (using the default content block). Ok, then finally, I want to address @WebMamba's approach: Second of all,
I just love having a problem being named after me 😂 Ok, let's dive into the proposed solution. Right off the bat... Let me confess, I'm having troubles understanding it 🙈 A few things also came to mind seeing this example:
PS: I don't have any stakes in either solution, so don't take my comments as me trying to pull down the slot solution. I'm probably just missing the key part about it :-) |
Here is a comment to resume my thought about all of this. |
Hi! Apologies for being a bit slow/absent. @WebMamba while I agree with what you said (I really like the simplicity of slots), I've become convinced of the power of the block case... and I think we should try to make it work first. But, it would require 3 things:
{# anyTemplate.html.twig #}
<twig:SuccessAlert>You did it!</twig> {# SuccessAlert.html.twig #}
{# the workaround: you must set the block to a variable *outside* of the block first #}
{% set content = block('content') %}
<twig:Alert type="success">
{{ content }}
</twig> This last one is a serious problem and I think we need to come up with a decent solution for it in order to move forward with the the block-based system. Any ideas on what the proper syntax should look like? Or... do both?Btw, another idea would be to combine the idea of slots and blocks where "slots" are the default mode, but you could also use blocks for advanced use-cases. Something like: {# anyTemplate.html.twig #}
<twig:SuccessAlert>You did it!</twig> {# SuccessAlert.html.twig #}
<twig:Alert type="success">
{{ slot }}
{% block footer %}
I can print the {{ type }} var from Alert.html.twig
{% endblock %}
</twig> {# Alert.html.twig #}
<div class="alert alert-{{ type }}">
{{ slot }}
{% block footer %}{% endblock %}
</div> I kinda like this :). I think it would just require that the new |
No worries @weaverryan! That was for a good cause, I m eager to try the AssetComponent! |
Hi all, A lot has happened meanwhile. So a quick recap:
Making that last change made me realize that this PR is insufficient, if #920 is to be merged, because it would bring the need to access the nested component hierarchy more than one level up (which is now |
I created a separate PR for this => #936 (which continues on the changes of #920) |
…endering embedded components (sneakyvv) This PR was squashed before being merged into the 2.x branch. Discussion ---------- [TwigComponent] Add variable to refer outer scope when rendering embedded components | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | Tickets | | License | MIT This PR's is an alternative approach to the problem stated in #863. Due to the changes proposed in #920, which deals with passing blocks down to nested embedded components, I realized that just being able to access the host (or parent) component isn't sufficient if blocks can be passed down multiple levels deep (as already mentioned in #863 (comment)). This PR adds a proxy object `Hierarchy` which allows this syntax: `Component.php` ```php final class Component { public function scream(int $screamHowHard = 1): string { return 'sf-ux rules' . str_repeat('!', $screamHowHard); } } ``` ```twig {# component.html.twig #} <twig:Foo :someProp="this.something(1)"> {{ someProp }} {{ outerScope.this.something(3) }} </twig:Foo> ``` ```twig {# Foo.html.twig #} <div>{% block content %}{% endblock %}</div> ``` Resulting in ```twig <div> sf-ux rules! sf-ux rules!!! </div> ``` Adding another layer below Foo, would ask for something like ```twig {# component.html.twig #} <twig:Foo> {{ outerScope.outerScope.this.something(3) }} </twig:Foo> ``` to still point to Foo's function. --- ### Questions There's also a `ComputedPropertiesProxy` object. Should we have a cached version of the `Hierarchy` proxy? --- ### NOTE This PR continues on the changes of #920, because having a hierarchy only makes sense if blocks can be passed down multiple times. So #920 may have to be merged first, but if needed, and the `outerScope.this` syntax is preferred over the syntax of #863, I could push my branch that starts of the main branch instead. Edited: using outerScope variable now instead Hierarchy proxy Commits ------- e618111 [TwigComponent] Add variable to refer outer scope when rendering embedded components
…endering embedded components (sneakyvv) This PR was squashed before being merged into the 2.x branch. Discussion ---------- [TwigComponent] Add variable to refer outer scope when rendering embedded components | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | Tickets | | License | MIT This PR's is an alternative approach to the problem stated in #863. Due to the changes proposed in #920, which deals with passing blocks down to nested embedded components, I realized that just being able to access the host (or parent) component isn't sufficient if blocks can be passed down multiple levels deep (as already mentioned in symfony/ux#863 (comment)). This PR adds a proxy object `Hierarchy` which allows this syntax: `Component.php` ```php final class Component { public function scream(int $screamHowHard = 1): string { return 'sf-ux rules' . str_repeat('!', $screamHowHard); } } ``` ```twig {# component.html.twig #} <twig:Foo :someProp="this.something(1)"> {{ someProp }} {{ outerScope.this.something(3) }} </twig:Foo> ``` ```twig {# Foo.html.twig #} <div>{% block content %}{% endblock %}</div> ``` Resulting in ```twig <div> sf-ux rules! sf-ux rules!!! </div> ``` Adding another layer below Foo, would ask for something like ```twig {# component.html.twig #} <twig:Foo> {{ outerScope.outerScope.this.something(3) }} </twig:Foo> ``` to still point to Foo's function. --- ### Questions There's also a `ComputedPropertiesProxy` object. Should we have a cached version of the `Hierarchy` proxy? --- ### NOTE This PR continues on the changes of #920, because having a hierarchy only makes sense if blocks can be passed down multiple times. So #920 may have to be merged first, but if needed, and the `outerScope.this` syntax is preferred over the syntax of #863, I could push my branch that starts of the main branch instead. Edited: using outerScope variable now instead Hierarchy proxy Commits ------- e618111be0 [TwigComponent] Add variable to refer outer scope when rendering embedded components
I just stumbled over this PR and it's discussing an exact problem I have and I didn't find a solution yet. I have quite a complex setup with blocks and components and for some reason, even tho there is the outerBlocks variable, I can't pass normal twig blocks down as content/named block of a component. Example:
<ul>{% block content %}{% endblock %}</ul>
<li>
{% block content %}{% endblock %}
{% if hasSubList %}
{{ block(outerBlocks.sublist) }}
{% endif %}
</li>
<span>{% block content %}{% endblock %}</span> Now I wanna use it in a block based template (think about a form theme for example, or something selfcooked): {% block list %}
<twig:List>
{{ block('children') }} // doesn't work
{{ block(outerBlocks.children) }} // seems to work... but
</twig:List>
{% endblock %}
{% block children %}
{% for child in children %}
{{ block('item') }} // also still seems to work... but
{% endfor %}
{% endblock %}
{% block item %}
<twig:ListItem>
{{ block(outerBlocks.label) }} // doesn't render anything
<twig:block name="sublist">
{{ block(outerBlocks.list) }} // also doesn't render anything
</twig:block>
</twig:ListItem>
{% endblock %}
{% block label %}
<twig:Label>{{ child.label }}</twig:Label>
{% endblock %} What am I missing, I would have expected this to work, but it seems it's not working, so there is no way to build components in such a nested way? Any help would be really appreciated 🙏 |
@CMH-Benny it would be really easier for us if you could create an issue (and there link to this PR if you need) instead of reviving old PR/issues, because we can miss them, and their original author receives notifications ;) I let you reopen one if this issue is still not resolved ? thx |
Problem
It's not possible to access properties and methods of a component that's using embedded components from within the embedded content (or other embedded blocks).
Reproduce
Create a wrapper component, which is including some components that have embedded content
Component.php
component.html.twig
foo.html.twig
(given a Foo.php, with empty class, not important)bar.html.twig
(given a Bar.php, with empty class, not important)Expected behavior
The template renders like (with some
dump
markup of course)Actual behavior
Cause
While rendering an embedded component the generated template will contain something like
The
$props
is the context used when rendering the embedded content, which is being composed by https://github.com/symfony/ux/blob/2.x/src/TwigComponent/src/ComponentRenderer.php#L92.And there's the problem: the original
$context
containsthis
, but when rendering theFoo
componentthis
becomes a reference toFoo
and the reference to the originalComponent
is lost.Imo this behavior is perfectly normal after all, knowing how the rendering works now. I.e.
this
should referenceFoo
, because the block is being embedded in the template offoo.html.twig
, so while renderingFoo
, but it makes sense that in the above template you want to be able to access properties from the "host" component, the component which is embedding some other component.Solution
Add a
host
entry to the context referencing the component which is embedding the nested components.PS: I'm not sure whether I ought to classify this as a bug or as a feature. It's perhaps not a bug in the sense that it's just not possible right now. On the other hand, it is imo something that you'd expect to be able to do, so then it might be a bug after all. You tell me 🙂
For now I classified it as a feature. If it shouldn't be I would have to remove the CHANGELOG entry.