Using `if_filled` with the built in `if`

[timothyis]

Is it possible to use if_filled along with the built-in if that Django templates provide?

In some of our components we offer – for example – filling a title with either a prop passed in on the component definition, or a slot fill if we need some greater flexibility. This allows us to keep complexity down when using components. It would be great to render the title either if the prop is passed or the slot is used but not have to use duplicate elements in the component definition because we need to use two conditional types.

[EmilStenstrom]

Could you give an example of how you would like the code to work?

[timothyis]

Yes, here is a minimal example that is based on our note component:

<div class="bg-slate-100 rounded-lg p-4 flex flex-col sm:flex-row">
	{% if iconName %}
		<div class="mr-3 mt-0.5">
			{% component "icon" name=iconName category=iconCategory set=iconSet only %}
		</div>
	{% endif %}

	{% if title %}
		<div class="flex flex-col">
			 <h4 class="font-medium">{% slot "title" %}{{ title }}{% endslot %}</h4>
 	{% endif %}
			<div>
				{% slot "message" %}{{ message }}{% endslot %}
		 	</div>
	{% if title %}
		</div>
	{% endif %}
</div>

In this example, I'm conditionally rendering a div if there is a title because otherwise it interacts with the icon (if there is one) incorrectly, but I also render the h4 that the title goes inside of so that the styling is consistent (I've removed a lot of classes here to make it easier to read).

I'd like to conditionally render all of these where {% if title %} but also taking into account if the slot is filled or not.

Currently we have to say "title=True". Here's an example of the current usage:

{% component "note" title="This is a title" message="This is a note" %}

^ With a prop (clean version)

{% component_block "note" title=True message="This is a note" %}
	{% fill "title" %}
		This is a title with something dynamic {{ pageContext.item }}
	{% endfill %}
{% endcomponent_block %}

^ With something dynamic inside the title to avoid doing messy stuff like title="This is a title with something dynamic"|add:pageContext.item, which can get messy with multiple variables and placing variables in the centre or using integers etc.

We also have a dialog component where you can either pass a title or define a custom header and currently we have to duplicate the header HTML depending on if there's a "header" fill or if there's a title.

{% if_filled "header" %}
	<header class="w-full min-h-10 pb-2">
		{% slot "header" %}{% endslot %}
	</header>
{% endif_filled %}
{% if title %}
	<header class="w-full min-h-10 pb-2">
		<h3 class="text-xl font-medium">{{ title }}</h3>
	</header>
{% endif %}

[a3lem]

Sorry for the wait. Been re-reading your example this week, hoping I'd eventually comprehend what you're trying to accomplish, but I'm afraid my brain isn't having any of it. Nonetheless, thanks for going to the effort of providing examples and extra detail. Let me see if I can't help out all the same.

What your use case (whatever it may be precisely) highlights for me is something that's bothered me about if_filled ever since I implemented it: it doesn't compose with the builtin if tag.

Out of curiosity, would your needs be better met if, instead of the if_filled tag, we went with an is_filled variable filter that simply composes with the default if? An example should make this clearer:

{% if "header"|is_filled %} ... {% endif %}

Would that be an improvement for this use case?

[timothyis]

Yes, 100% would be better. This would allow us to combine the if/if_filled statements as we require in quite a few places.

[a3lem]

Alright, noted!

@EmilStenstrom, how would you feel if we backtracked on the if_filled tag and replaced it with a more composable is_filled filter? Both could in principle co-exist, although I'd recommend eventually dropping the tag.

[EmilStenstrom]

@lemontheme It's relatively new, so I think it's ok to drop. Agreed that a is_filled filter is probably a nicer API.

[a3lem]

So, I took a stab at implementing an is_filled filter the other day. I'd already designed a solution on paper based on some earlier exploring of the Django template internals.

Hit a snag, unfortunately. Turns out that filters, annoyingly, and differently than custom tags, do not have access to the render context. There are workarounds for this but so far they all suffer from the same problem: slot filling is a render time property, which means we can't set that information on the template nodes without creating thread safety issues. The only real workaround is to inspect the call stack from within the filter function, go up a few frames, and find the context. This is definitely doable. With sufficient documentation it may even be maintainable. But it's a big WTF for anyone new to the code. Let's call this option A.

@EmilStenstrom, would you like me to go ahead with a prototype of that idea?

Nonetheless, I feel a more composable alternative to if_filled is still a good idea. An easy alternative might be to just create a new context variable, e.g. filled_slots. So as not to pollute the context, we could namespace it (and potentially other variables later on) to a general variable, e.g. component_vars. Let's call this option B.

Usage:

(B.1)

{% load component_tags %}
{% if component_vars.filled_slots.slot_name %} ... {% endif %}

Seeing as it's an advanced feature, component_vars can be made optional. To include it in the context, users could either use a new tag to set it, e.g. 

(B.2)

{% load component_tags %} 
{% set_component_vars %}
{% if component_vars.filled_slots.slot_name %} ... {% endif %}

Alternatively, we could introduce a second installable middleware to modify the context.

(B.3)

MIDDLEWARE = [
    ...
    "django_components.middleware.ComponentVarsMiddleware",
    ...
]

Would love to get your thoughts on this!

[EmilStenstrom]

I think the first thing to consider is the API. Looking at the "header"|is_filled example it is a bit strange that the filter has context other than the string.

Maybe something like component_vars|is_filled:"header" could work, and not require frame diving…?

At the same time, skipping the template tag altogether as in your second post cajole is less code to maintain of course. I think it also opens up new possibilities, like conditionally filling on slot when another is empty?

All in all, I think it makes sense to make this terse, so I like the B1 version the most.

[nerdoc]

What about

{% if is_filled.<slot_name> %}

As abbreviation? Would be easy to implementiert, and less verbose.

[EmilStenstrom]

@nerdoc This is a good suggestion. I'm a bit worried that slot names are strings today, so I think they can contain a space. Is is_filled."my slot" valid template code? Would you have the time to write up a PR?

[nerdoc]

oh, slot names IMHO are "machine names" and should be tested against .isidentifier() in all cases. This also breaks thinks slightly, but is extremely easy to fix, and allows things like my is_filled.xyz suggestion. I'm so badly tied up with my day job (doc office) ATM, so I'm sorry I can't go into making a PR - I'm sorry.

[JuroOravec]

Heya, I just came across this as I was trying to fix another issue with slots/fills (https://github.com/EmilStenstrom/django-components/issues/453).

I'm on the same boat as @nerdoc, in that I'm for having a plain context variables that simply hold True/False. IMO this would be the simplest to implement / maintain, and would be also the most flexible (users could pass the variable around).

[nerdoc]

As I happen to be the new maintainer of Tetra, I solved this problem there this way: https://github.com/tetra-framework/tetra/blob/d66db0caf54d756de5f44392235e7dba964973dd/tetra/templatetags/tetra.py#L266 This is straightforward, and works flawlessly by now.

[EmilStenstrom]

So in our case the API would be: {% if variable > 8 and slots.header %}?

Two questions:

1. What happens if there's already a slots variable in the context?
2. With the above API we require slot names to be valid identifiers, they are strings today. What about backwards compatibility?

[nerdoc]

So in our case the API would be: {% if variable > 8 and slots.header %}?

Two questions:

1. What happens if there's already a slots variable in the context?

It's the responsibility of the dev. If I name a context variable "object" in an UpdateView, I will also override the original model object var. Documentation is the key. Nobody stops anyone to write a templatetag named "component". But he will then get problems when he wants to use django-components.

2. With the above API we require slot names to be valid identifiers, they are strings today. What about backwards compatibility?

I would make a deprecation if you want, but before v1.0.0, this is easy to fix for all implementors...

[JuroOravec]

I had a go at the MR, see https://github.com/JuroOravec/django-components/pull/1. It's still draft, and it's blocked by https://github.com/EmilStenstrom/django-components/pull/457.

Anyway, the changes were fairly simple. What remains still is the design perspective, and updating the docs.

---

As per questions:

Is is_filled."my slot" valid template code?

• No, it doesn't work, I've just tried. In theory, users could make a simple filter that takes an object, a key, and returns object[key] to avoid the issue with non-identifier slot names. But IMO it'd be cleaner if we allowed only valid identifiers.

As for @EmilStenstrom's questions, I agree with @nerdoc.

---

Also I just read the rest of the thread (from the year ago). Separate middleware or loader just to access the info on which slots are used or not feels like an overkill.

However, I like the idea of namespacing the variables provided by django-components, AKA having component_vars.filled_slots.slot_name instead of filled_slots.slot_name.

[EmilStenstrom]

So, we're talking about breaking existing code that uses {% slot "my slot" %}? I think that's a bad idea.

One alternative to this would be to make that slot accessible via some transformation, like "my_slot", transforming the string to a valid identifier if it isn't already one. I think that's a fairly elegant solution, as it doesn't affect proper identifiers at all, but also doesn't break any of the existing code, or change the API from strings to identifiers when defining slots. All we need is a function that takes a string and turns it into a valid identifier.

I think a component_vars global is a nice way around accidentally hitting people's existing code. Slots is such a common term, that I think it might be shadowed accidentally.

So the suggested API is: For a slot defined as {% slot "my slot" %}, you can check it's existence with {% if component_vars.is_filled.my_slot %}?

I guess we deprecate if_filled at the same time?

[JuroOravec]

@EmilStenstrom I like those suggestions, I'll work them in :)

[JuroOravec]

I've implemented the changes in https://github.com/EmilStenstrom/django-components/pull/465, please give it a look whe you've got the time :)

[JuroOravec]

Released as part of 0.70 🎉