{% block %} does not work inside a slot about django-components HOT 17 CLOSED

@philipstarkey, just to make sure I understand your first approach, do you have the whole component tree in the root parent template, with a {% block sections %} tag where the <!-- content here --> placeholder is? I don't think there's a good reason that shouldn't work. That said, Django's template inheritance system is very complicated, so I won't be very surprised if we're screwing up the context in some way that interferes with it.

You're right that the second approach doesn't work right now. It used to, but it creates a risk of infinite recursions if there are naming collisions, so components currently only get slots from their immediate parent. An easy fix would be to add an option to allow unsafe global slot searching, but that's not optimal. I'll think about whether there's a way to do that safely.

It's a bit unwieldy, but you ought to be able to insert an {% include content_template %} tag where you want the content, then pass the parent component the path to another template with the content as a prop.

from django-components.

Ok, I've put together (a probably over the top) example project derived from cookiecutter-django.

Relevant files (since it has a giant amount of boilerplate) are:

Component:

• https://github.com/philipstarkey/django_components_issue97/blob/master/django_components_issue97/users/components.py
• https://github.com/philipstarkey/django_components_issue97/blob/master/django_components_issue97/templates/container.html

Base templates:

• https://github.com/philipstarkey/django_components_issue97/blob/50c99ddb08085e5f43d53638c21da5bc3be05056/django_components_issue97/templates/base.html#L94-L104
• https://github.com/philipstarkey/django_components_issue97/blob/master/django_components_issue97/templates/middle.html

Templates:

• https://github.com/philipstarkey/django_components_issue97/blob/master/django_components_issue97/templates/pages/block_in_component.html
• https://github.com/philipstarkey/django_components_issue97/blob/master/django_components_issue97/templates/pages/block_in_component_in_block.html

The two templates match the two scenarios mentioned in my previous post (simple template inheritance and a more complex template inheritance). Both show the default content of the block inside the component and not the content of the block as defined in the templates.

Let me know if I can do anything more to help!

from django-components.

@philipstarkey Thanks so much for the extremely detailed example. I'm heading out on a short vacation today, so I should have some time to delve in over the next few days.

You're right about the extends tag being the workhorse. As I recall, it basically hijacks the parser, grabs all the block tags in the current template, then rewrites the parent template with them. So I would not expect things to work if the extending block tag is nested inside anything else, because extends won't be able to find it. I haven't looked at the reverse situation, where the source block is nested. I would think that ought to work generally, but I can imagine that it might not with this library, because we also do some on-the-fly template rewriting, which might step on Django's toes.

Let me play around with your examples so I have a better understanding of your use case, and I'll report back.

from django-components.

I would actually put it a bit more strongly--that code unambiguously renders the card component with an empty contents slot.

from django-components.

@rbeard0330 I think you might know the best way forward here. Is it possible to support mixing blocks with slots like Philip assumed above?

from django-components.

Without thinking to deeply about this I think fixing eventual bugs around combining blocks and slots should be a first good step. My guess is that most people will use this library by adding components gradually, not to replace the extends/block system.

@philipstarkey Can you help us put together a simple testcase where slot and block doesn't work well together?

from django-components.

@EmilStenstrom @rbeard0330 Thanks for both of your replies. I'll see if I can put together a minimal Django project that demonstrates the issue shortly.

My actual template structure is a little more complicated than your examples, in that I have a template (top) extending a template (middle), which in turn extends another template (base). The base template contains a {% block content %} which is overridden by the middle template. It is in this middle template that I'd like to put the components, inside the content block, and in turn have a {% block component_content %} block defined within a slot of a component which is overridden in the top template. This nesting of blocks works without components.

That said, I also just tried the simpler version of having components in the base template, with a block inside, and overriding the block from within a template that extends the base, and it also didn't work. I'll try and put both examples in the test repo.

I'm not hugely familiar with the internals of the Django templating library, so still trying to wrap my head around how it all works, but is it actually the {% extends "base.html" %} template tag that handles the processing of {% block %} template tags? It didn't appear like the code for the {% block %} tag in Django did much.

from django-components.

You're right that the second approach doesn't work right now. It used to, but it creates a risk of infinite recursions if there are naming collisions, so components currently only get slots from their immediate parent. An easy fix would be to add an option to allow unsafe global slot searching, but that's not optimal. I'll think about whether there's a way to do that safely.

And while I'm thinking about this issue, could this be resolved by either an additional argument to {% slot ... %} that states the nested slot belongs to the component the template is for, or assuming that a {% slot %} inside a {% slot %} means the inner slot belongs to the component the template is for, or using a different template tag name to differentiate between "here is the content for the slot" and "here is where the slot content should go" (e.g. {% component_block ... %}{% slot ... %}{% export_slot ... %})? There are probably obvious reasons why these wouldn't work though that I'm unaware of!

from django-components.

And while I'm thinking about this issue, could this be resolved by either an additional argument to {% slot ... %} that states the nested slot belongs to the component the template is for, or assuming that a {% slot %} inside a {% slot %} means the inner slot belongs to the component the template is for, or using a different template tag name to differentiate between "here is the content for the slot" and "here is where the slot content should go" (e.g. {% component_block ... %}{% slot ... %}{% export_slot ... %})? There are probably obvious reasons why these wouldn't work though that I'm unaware of!

@philipstarkey I've been working on something along these lines, which I think will be the best solution. I've added an optional exported_as argument to the slot tag, which causes the parent component to look for a slot under the exported-as name and pass it to the relevant child component under its regular name. I think this should solve your use case--in your original example, you could export the sections slot as innercontent. I'm also working on letting you decide whether to specify new default content for the exported slot or defer to the inner component's default content.

I'm working out the kinks now, but hopefully should have it available pretty soon.

from django-components.

@rbeard0330 Could you show an example of how the syntax would change here? I think this would be a good way to talk through the design before you put too much effort in.

from django-components.

@EmilStenstrom Sure. To take a simple example, imagine you had a card component with a contents slot, and you wanted to make a version of the component with a header. The template for the new component would look like this:

<h1>{{ header_text }}</h1>
{% component_block 'card' %}
    {% slot contents export_as='body' %}{% endslot %}
{% endcomponent_block %}

This would make it possible for a client of the new component to provide content to be rendered in the contents slot of the original card component. To use, you'd write this in your page template:

{% component_block 'header_card' header_text="This is the header" %}
    {% slot body %}This is the body{% endslot %}
{% endcomponent_block %}

This would produce something like this (assuming the card component just wraps the slot in a styled div):

<h1>This is the header</h1>
<div class="card">This is the body</div>

The renaming is to allow the user to avoid namespace issues.

The other potential enhancement is to make it possible to override the default content provided by the original component if the user of the new component doesn't provide anything. That would look something like this:

<h1>{{ header_text }}</h1>
{% component_block 'card' %}
    {% slot contents export_as='body' with_new_default_content=True %}
        Default content for header_card
    {% endslot %}
{% endcomponent_block %}

So in this example, card's contents slot would be populated in this order of priority:

1. If the user provides a body slot when they instantiate the header_card component, that content is rendered.
2. If they don't, and with_new_default_content is True, then whatever content is in the contents slot tag in the header_card template is rendered.
3. Otherwise, the default content in the card template is rendered, as usual.

There wouldn't be any changes for components that aren't using the export mechanism.

from django-components.

Thanks for a great run-through of how you're thinking through this. Adding support for making hierarchies of components like this is a really nice addition to django-components I think.

I'm not trilled about the export_as and with_new_default_content parameters, and are thinking about ways to avoid adding them.

Let's see if I understand this correctly. The problem with removing the export_as-parameter in...

In header-card.html:

<h1>{{ header_text }}</h1>
{% component_block 'card' %}
    {% slot contents %}{% endslot %}
{% endcomponent_block %}

... is that we don't know if the intent of the contents slot is to fill the card slot, or define a new header-card slot with the same name?

from django-components.

In my mind, this gets easier to think about if we imagine creating slots and filling slots as two different actions.

So your example becomes {% fill contents %}{% slot body %}{% endslot %}{% endfill %}, and the version with default contents {% fill contents %}{% slot body %}Default content for header_card{% endslot %}{% endfill %}. Am I understanding this correctly?

Which leaves us with these options:

1. {% slot content export_as="body" %}
2. {% fill contents %}{% slot body %}
3. {% slot contents %}{% slot body %} -- The direct decendant of component_block is always a fill, all other calls create new slots.

Thoughts?

from django-components.

Two thoughts:

1. This is minor, but how important is it to give the parent component author flexibility about what is rendered if the component user doesn't specify anything? More specifically, I think we have to let the parent component author specify their own default content, so do we just assume that the parent default content overrides the child default? Perhaps slot.super would make it ergonomic for the parent component author to defer to the child component default content if that's what's desired.
2. One reason I was drawn to (further) overloading the existing slot tag is the implementation. As a refresher, how this works now is:

• component_block tag is parsed in a template. The parser creates the requested Component, collects all the top-level slot tags, parses them as SlotNodes, then passes everything to the new ComponentNode. The ComponentNode initializer then passes the nodelists from those SlotNodes to the Component. These are the fill nodes, and they're never actually rendered.
• ComponentNode is rendered, which basically defers to Component.render. Component.render sets up a context including only the slots that were included in its component_block tag, then renders its component template.
• While the component template is being rendered, slot tags can be compiled into SlotNodes and rendered. When that happens, they look in the context for a nodelist corresponding to their name. If they find one, they render it, otherwise they render their default content.
  The tricky bit is what was bolded. I haven't tested this, but I think in older versions, something like:

<h1>{{ header_text }}</h1>
{% component_block 'card' %}
    {% slot body %}
        {% slot contents %}Default content for header_card{% endslot %}
    {% endslot %}
{% endcomponent_block %}

Would work just fine. card would interpret the body slot as a fill, and put the corresponding nodelist in its context. Then it renders its template, and when it comes to the body slot, it replaces it with the provided nodelist and renders that. When it gets to the content slot, it just renders it, so that slot looks in the context for a contents nodelist it can use. In older versions, all the slots that were available further up on the tree would be visible, so it could find a slot provided to an outer component, render it, and be fine. We changed this because it creates a risk of infinite recursions (which I think you'd get if you didn't rename the body slot to contents.

All that explanation is to point out that either: (a) we need to come up with a solution to the infinite recursion problem that's more nuanced than just limiting slot scope to a single component, or (b) our solution needs to inform all the components in the chain that they need to pass the exported slot along in the context. What's probably easier for (b) is to have each component put two things in the context: a "local" dict, with just its own fill slots, which are available for slots to use, and a "global" dict with all the slots it knows about. Then the component that's actually using the exported slot (card in the simple example), can fish the relevant nodelist out of the global dict and insert it into the local dict, so it can be used.

So, if we're going down path (b), we need to figure out how card knows to add an extra nodelist to the local context for its own template. What was easy about the export_as slot approach is that the top-level slots under a component_block tag (the fill tags) are being parsed at the same time as the Component is being created, so it's straightforward to look at that arguments to those tags. We don't currently do anything with the nodelists that are contained in those fill tags--they just get passed along until a slot requests them, then the slot renders them.

Now, we do traverse all the nodes in a component template. It's conceivable that we could use that pass to identify all the slots that are used by the template, and have the component include them in the context somehow. This approach worries me though:

1. Traversing nodelists is a little brittle--Django's built-in nodes adhere to a consistent protocol for how you can find all their subsidiary nodelists, but I'm not sure how universally followed this protocol is.
2. There's more bookkeeping/intercomponent communication required. The outermost component is the one that will parse the template, see the exported slots, and know that they have to be made available. So it will have to pass along that information to each child component in the tree.
3. I have to think about this more, but is there a risk we reintroduce infinite recursion problems if we do something along these lines? I think the answer is no, but I'm not 100% comfortable with that yet.

To round it out, if we went with your suggestion (2) and added an explicit fill tag, we could probably handle it much the same as the export_as approach. When we compile the fill tag, we would know we need to look within to find the exported slot and handle the context appropriately. I think this wouldn't be too hard, especially if the fill tag isn't allowed to contain anything other than a slot.

Sorry for the wall of text--couldn't stop writing once I got going... 😟

from django-components.

Sorry for the late reply! Thanks for trying to explain to me what the code does. I am a bit lost, and don't have the time to sit down and really understand things properly. So feel free to run with it, I trust that you find the best solution.

I wonder, would it make things easier if we require Components with slots to define the names of those slots in the class?

from django-components.

@rbeard0330 Did you happen to have some time to think more of this? I think nesting components with slots would be a very useful addition to this library.

from django-components.

I have split this issue into two different issues, #133 and #134, let's continue the discussion there.

from django-components.