mixxorz / slippers

A UI component framework for Django. Built on top of Django Template Language.
https://mitchel.me/slippers/
MIT License
506 stars 37 forks source link

Not working with third party libraries like django_tables2? #43

Closed Thutmose3 closed 1 year ago

Thutmose3 commented 1 year ago

I have a component with a django_tables2 table in it:

components/table.html

{% load django_tables2 %}
{% render_table table %}

But this is not working as i'm having the error:

raise ImproperlyConfigured(context_processor_error_msg % "querystring")
django.core.exceptions.ImproperlyConfigured: Tag {% querystring %} requires django.template.context_processors.request to be in the template configuration in settings.TEMPLATES[]OPTIONS.context_processors) in order for the included template tags to function correctly

It seems that some context processors are not passed, or not yet passed? Any idea how to fix this?

mschoettle commented 1 year ago

How are you calling/using this component?

Thutmose3 commented 1 year ago

products.html

{% extends "base.html" %}

{% load slippers %}

{% #table_components %}
{% /table_components %}

components/table.html

{% load django_tables2 %}
{% render_table table %}

views.py

from .tables import ProductsTable
class InventoryListView(
    LoginRequiredMixin,
    SingleTableMixin,
):
    paginate_by = 25
    template_name = "products.html"
    model = Product
    table_class = ProductsTable

This gives this error -> Expected table or queryset, not str


Also tried this:

products.html

{% extends "base.html" %}

{% load slippers %}

{% #table_components with table=table %}
{% /table_components %}

components/table.html

{% load django_tables2 %}
{% render_table table %}

views.py

from .tables import ProductsTable
class InventoryListView(
    LoginRequiredMixin,
    SingleTableMixin,
):
    paginate_by = 25
    template_name = "products.html"
    model = Product
    table_class = ProductsTable

This gives this error -> File "/Users/.../lib/python3.11/site-packages/django_tables2/templatetags/django_tables2.py", line 59, in render raise ImproperlyConfigured(context_processor_error_msg % "querystring") raise ImproperlyConfigured(context_processor_error_msg % "querystring") django.core.exceptions.ImproperlyConfigured: Tag {% querystring %} requires django.template.context_processors.request to be in the template configuration in settings.TEMPLATES[]OPTIONS.context_processors) in order for the included template tags to function correctly


Like this it works, but we use the children, which is not ideal:

products.html

{% extends "base.html" %}
{% load django_tables2 %}

{% load slippers %}

{% #table_components %}
{% render_table table %}
{% /table_components %}

components/table.html

{{ children }}

views.py

from .tables import ProductsTable
class InventoryListView(
    LoginRequiredMixin,
    SingleTableMixin,
):
    paginate_by = 25
    template_name = "products.html"
    model = Product
    table_class = ProductsTable
mschoettle commented 1 year ago

It might be that the table template requires the request. Try passing it in:

{% #table_components table=table request=request %}
{% /table_components %}

// or use short format

{% table_components table=table request=request %}
Thutmose3 commented 1 year ago

That worked indeed!! Why do we need to pass these variables? Would be nice if it would work like {% include %}, there the variables present in the parent template are passed in the included template

mschoettle commented 1 year ago

I don't know for sure but it was probably a design decision. It’s documented here although without providing a rationale: https://mitchel.me/slippers/docs/using-components/#component-context

Thutmose3 commented 1 year ago

I see, I don't agree with the readability part, I think it makes the code more fragile and redundant and less django-like.

I can understand the rationale for the side-effects, but what side-effects are you thinking about? I probably don't know the codebase enough to see what could go wrong. Maybe it can be foreseen in the future to add current context to child components?

mixxorz commented 1 year ago

I decided not to pass the context by default because I didn't want components to rely on external state. Just like how you wouldn't rely on global variables when you write your methods/functions in Python.

However, I think exemptions could be made for variables that devs expect to be present, like request. I'll open an issue for this.