wrabit / django-cotton

Enabling Modern UI Composition in Django
https://django-cotton.com
MIT License
474 stars 19 forks source link

Support passing down `attrs` #192

Open wrabit opened 6 days ago

wrabit commented 6 days ago

As discussed, it's sometimes desirable to pass down the attrs object to other components.

<!-- parent_component.html -->
<c-nested-component attrs=attrs />`
<!-- view -->
<c-parent-component attr1=".." attr2=".." />

In this case, nested_component's attrs will contain 'attr1', 'attr2' etc.

alorence commented 4 days ago

AFAIK, it is currently possible with some drawback.

This is the way I setup my icons, with content from Lucide package:

cotton/icon/base.html

<svg xmlns="http://www.w3.org/2000/svg"
     {% if size == "inherit" %}
     style="width: 1em; height: 1em;"
     {% else %}
     width="{{ size|default:24 }}"
     height="{{ size|default:24 }}"
     {% endif %}
     viewBox="0 0 24 24"
     fill="{{ fill|default:'none' }}"
     stroke="{{ color|default:'currentColor' }}"
     stroke-width="{{ stroke_width|default:2 }}"
     stroke-linecap="{{ stroke_linecap|default:'round' }}"
     stroke-linejoin="{{ stroke_linejoin|default:'round' }}"
     class="lucide{% if class %} {{ class }}{% endif %}">
  {{ slot }}
</svg>

cotton/icon/info.html

<c-icon.base>
  <rect width="20" height="16" x="2" y="4" rx="2" />
  <path d="m22 7-8.97 5.7a1.94 1.94 0 0 1-2.06 0L2 7" />
</c-icon.base>

cotton/icon/bookmark.html

<c-icon.base>
  <path d="m19 21-7-4-7 4V5a2 2 0 0 1 2-2h10a2 2 0 0 1 2 2v16z" />
</c-icon.base>

etc.

In all pages, I can simply use <c-icon.info />, <c-icon.bookmark />, etc. with the ability to easily customize size, colors, classes applied, etc. But the logic is stored only in base.html, so all icons defined on that base have the same customization options.

This is possible because base components inherit context passed to specific one. I don't know if this is a side effect or a desired behavior. And since this behavior is kind of implicit, maybe it would be better to have a more specific way of doing the same thing, with a more explicit syntax

wrabit commented 4 days ago

@alorence agree and I would rather a more explicit approach. TBH I think it would be better to isolate context by default similar to only, but still include the 'builtin' context items, messages, request, perms, csrf_token. (because it would be annoying having to specify these things manually, when the standard django usage you don't have to).

And then there is the context_processors. Which we may also be able to include by default, will need to do some tests and benchmarks around this but what are your thoughts around this?

mattbha commented 4 days ago

AFAIK, it is currently possible with some drawback.

This is the way I setup my icons, with content from Lucide package:

cotton/icon/base.html

<svg xmlns="http://www.w3.org/2000/svg"
     {% if size == "inherit" %}
     style="width: 1em; height: 1em;"
     {% else %}
     width="{{ size|default:24 }}"
     height="{{ size|default:24 }}"
     {% endif %}
     viewBox="0 0 24 24"
     fill="{{ fill|default:'none' }}"
     stroke="{{ color|default:'currentColor' }}"
     stroke-width="{{ stroke_width|default:2 }}"
     stroke-linecap="{{ stroke_linecap|default:'round' }}"
     stroke-linejoin="{{ stroke_linejoin|default:'round' }}"
     class="lucide{% if class %} {{ class }}{% endif %}">
  {{ slot }}
</svg>

cotton/icon/info.html

<c-icon.base>
  <rect width="20" height="16" x="2" y="4" rx="2" />
  <path d="m22 7-8.97 5.7a1.94 1.94 0 0 1-2.06 0L2 7" />
</c-icon.base>

cotton/icon/bookmark.html

<c-icon.base>
  <path d="m19 21-7-4-7 4V5a2 2 0 0 1 2-2h10a2 2 0 0 1 2 2v16z" />
</c-icon.base>

etc.

In all pages, I can simply use <c-icon.info />, <c-icon.bookmark />, etc. with the ability to easily customize size, colors, classes applied, etc. But the logic is stored only in base.html, so all icons defined on that base have the same customization options.

This is possible because base components inherit context passed to specific one. I don't know if this is a side effect or a desired behavior. And since this behavior is kind of implicit, maybe it would be better to have a more specific way of doing the same thing, with a more explicit syntax

this is very similar, almost exactly, to what I do for my icons. love it. I also define "icon buttons" using these icons and attaching a @click handler

ie

<c-icon-button click="location.reload()">
    <c-svg.refresh />
</c-icon-button>
alorence commented 4 days ago

@alorence agree and I would rather a more explicit approach. TBH I think it would be better to isolate context by default similar to only, but still include the 'builtin' context items, messages, request, perms, csrf_token. (because it would be annoying having to specify these things manually, when the standard django usage you don't have to).

I definitely agree with that idea. Isolating context by default seems the most logical approach. But allowing users to pass context variables to child component using "attrs" in any way must be preserved, because this is the key to define interesting components set based on a common logic but multiple concrete implementation (like \<c-icon.base> / \<c-icon.specific>).

Always passing messages, request, perms, etc. also sounds good to me, because they are injected via context_processors, and we are used to have these available anywhere in Django templates. Components templates should have the same abilities in my opinion.

That's said, this could be a major change in the behavior of the library, so maybe this should be customizable at some point (opt-in to isolated context by default) or may be integrated only in a new major version?

And then there is the context_processors. Which we may also be able to include by default, will need to do some tests and benchmarks around this but what are your thoughts around this?

I am not sure to understand your idea here. What are you talking about exactly (maybe you reference something I don't know in Django template engine ;) )?

wrabit commented 4 hours ago

@alorence what I meant is telling apart the view-set context to the items set by context processors, but in actual fact this is trivial.

I'm in agreement that this should be a major version + clear details on the change. The configurable way out of this is a decent approach and some supporting documents. The only can still be supported if the user needs further isolation.