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

Pass context variables down to components #44

Open
Thutmose3 opened this issue Jul 12, 2023 · 4 comments
Open

Pass context variables down to components #44

Thutmose3 opened this issue Jul 12, 2023 · 4 comments

Comments

@Thutmose3
Copy link

Currently context variables are not passed down to the componenets, which is really a pain when using components extensively.

I have to repeat the context variables multiple times like this:
{% header_title_component count=object_list.count tab_name=tab_name title=title %}

It's redundant and error prone, and also a bit against the whole concept of using components.
Components should allow us to write less code, by having to repeat these variables i feel like we are going against that concept.

@mixxorz
Copy link
Owner

mixxorz commented Jul 28, 2023

This is a deliberate design decision (the rationale of which I should add to the docs at some point).

I like to think of components as functions. You "call" a component, pass in necessary variables, and it returns some HTML. Just like normal functions, it's not great practice to rely on global variables inside the component code.

Let's take your example above. If slippers passed all current context variables to the component, the code would look something like this:

{% header_title_component %}
<div class="header-title">
    <div class="title">{{ title }}</div>
    <div class="count">{{ object_list.count }}</div>
    <div class="tab-name">{{ tab_name }}</div>
</div>

But this assumes that title, object_list.count and tab_name variables are always going to be present in the parent context. You will now have to ensure that these variables are present in any Django view where {% header_title_component %} is used. This approach means that the component code is now hard coupled to the view logic. A change to one means you have to change the other. And if you change the component code, you will now have to subsequently change all other Django views that use that component.

This is the worst case scenario of course. The level of coupling depends on how you structure your code.

In any case, the option of using {% include %} is still available. Perhaps for some of your components, you may wish to use slippers, but for some others it may be more convenient to use {% include %}. (I can think of a few scenarios where I might do this myself.)

@Thutmose3
Copy link
Author

Thank you for the response, I think your arguments really make sense. But in our working context, where we use slippers extensively (everything is componerised), it feels annoying and redundant.

Also i find that a view that renders an HTML page with multiple component, is like a Class in python, and the components are like the functions in that class. The init.py is sets the context for all the functions in that class. Imagine having to reset/fetch the context for each of the functions. This is what it feels like when using components right now.

Our templates are indeed hard coupled to our view logic, but we use DetailView and ListView a lot, and there by default we have access to the context variables: "object", "object_list" which are always available in all our templates, and we find ourselves having to do {% header_title_component object=object %} a lot.

Like you said there is indeed {% include %} but the whole reason we are using slippers is to be able to pass HTML down to the component which is not possible with {% include %}.

Would there be a way to add a settings/flag somewhere to add this functionality for people who want it?

@JuroOravec
Copy link
Contributor

Problem - How to define context variable that should be available only to the component?

While I love the deliberate decision to not pass context variables, I think it would be great to have an escape hatch to be able to access them. Consider the following:

I have an icon.html component, where I give the component a material design icon name, and it render corresponding icon.

Before using Slippers, icon.html knew HOW to render any given icon, because I also defined a global icons context variable, which was a mapping from icon name to icon's SVG paths, like so:

{
    "bars_3": ["M4.26 10.147a60.436 ....", "M4.26 10.147a60.436 ...."],
    ...
}

Since the icon components has no way to access the "global" icons context variable, I had two options:

1. Pass the icons context var every time I use the icon component:

{% icon icon='outline_chevron_down' icons=icons %}

You can see this would bloat the interface and be horribly repetitive. So I went with another option:

2. Create a new tag load_icon, only to be able to access the icons variable:

@register.simple_tag
def load_icon(icon_name: str):
    """Given an icon name, return its SVG paths."""
    return getattr(icons, icon_name)

Which I then used in the icon.html component like so:

{% var icon=icon %}

{% load_icon icon as icon_paths %}

<svg>
  {% for path in icon_paths %}
  <path d="{{path}}" />
  {% endfor %}
</svg>

Solutions - Define component context

The option 2. in itself is not bad, but it shows that the component logic could be simpler if there was a way to define context variables that are available ONLY to the component, and which did not come from the parent context or through props.

I imagine this would have to be done at registration time. There could be two ways to define this:

1. Via components.yml

Normally, we define components in YAML file like so:

components:
  icon: "components/icon.html"

We could support alternative declaration, that would allow to define Python module import path:

components:
  icon:
    path: "components/icon.html"
    context: "app.constants.icon"

Where app/constants/icon.py could look something like this:

icons = {
    "bars_3": ["M4.26 10.147a60.436 ....", "M4.26 10.147a60.436 ...."],
    ...
}

All variables from app/constants/icon.py would be loaded onto the context of the component, and hence, in the component, we could call icons. Without fearing of polluting parent or child contexts.

2. Via register_components

This would be the same, but using the register_components functions:

Instead of how it's now:

from slippers.templatetags.slippers import register_components

register_components({
  "icon": "components/icon.html"
})

We would allow to pass a tuple/list whose second value would be the context for the component:

from slippers.templatetags.slippers import register_components

import app.constants.icon as icon_context

register_components({
  "icon": ("components/icon.html", icon_context)
})

@JuroOravec
Copy link
Contributor

@mixxorz Change of plans. As I was just implementing the feature I suggested above in my fork, I noticed that the library already allows to pass in additional context via the front matter.

The good news is, that this solves my issue.

The bad news is that it would be unwise to use it when it's not documented.

Hence, I wrote a documentation to it:

See MR #58. Please review. Thanks!

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

No branches or pull requests

3 participants