Skip to content
This repository has been archived by the owner on Dec 26, 2023. It is now read-only.

Proposal: turbo_frame tag. #5

Open
blopker opened this issue Dec 26, 2020 · 8 comments
Open

Proposal: turbo_frame tag. #5

blopker opened this issue Dec 26, 2020 · 8 comments

Comments

@blopker
Copy link
Member

blopker commented Dec 26, 2020

Hey team, I'm pretty excited about the prospect of writing less JavaScript and so, am pretty excited about this project.

After wrapping my head around the Rails version of the frames tag (https://github.com/hotwired/turbo-rails/blob/main/app/helpers/turbo/frames_helper.rb), I've come up with what I think is the equivalent in Django land. Let me know what you think!

Simplest version

This renders a Django template inline, passing the parent template context to the child template like an include tag would. The tag would wrap the child context with <turbo-frame>, like the Rails version does. This simple version would auto-generate a frame ID based on this template name, in this case myapp-mytemplate. It could also add that ID to the parent context (turbo_id) as it passes the context to the child template.

Tag:

{% turbo_frame 'myapp/mytemplate.html' %}

Template:

<div>My frame with ID {{turbo_id}}!</div>

Output:

<turbo-frame id="myapp-mytemplate">  
<div>My frame with ID myapp-mytemplate!</div>
</turbo-frame>

Tag with arguments

This demonstrates overriding default behavior to the previous tag. Note all arguments are prefixed with turbo to avoid collisions in the lazy tags below.
turbo_target: Custom target. Optional.
turbo_id: Custom frame ID. Works for both lazy and non-lazy frames. Optional.

Tag:

{% turbo_frame 'myapp/mytemplate.html' turbo_target='_top' turbo_id='my_id' %}

Template:

<div>My frame with ID {{turbo_id}}!</div>

Output:

<turbo-frame id="my_id" target="_top">  
<div>My frame with ID my_id!</div>
</turbo-frame>

Lazy frame

This demonstrates using the tag with lazy loading. The first argument is still a template path, but this will be replaced once the remote content loads. In a lazy frame the template argument is also optional. turbo_src is either a URL or a named URL and makes the frame "lazy". The idea is to be similar to how the url tag works. Unknown arguments are passed to URL resolver, just like with the builtin url tag. Arguments prefixed with turbo are used to configure the frame. This adds a new option, turbo_loader, that only works with lazy frames. The previous options work here too. For lazy frames the auto generated ID is based on turbo_src and the arguments passed.
turbo_src: Named URL or URL. Turns the frame into a lazy frame. Optional.

Tag:

{% turbo_frame 'my_spinner.html' turbo_src='myview' arg1=v1 arg2=v2 %}

Template:

<div>Loading {{turbo_id}}!</div>

Output:

<turbo-frame id="myview-v1-v2" src="https://myapp.com/myurl/1/2">  
<div>Loading myview-v1-v2!</div>
</turbo-frame>
@davish
Copy link
Contributor

davish commented Dec 26, 2020

Hey @blopker, thanks so much for putting this proposal together! Overall, I think this makes a lot of sense. The one part that I wanted to ask about was the specific API around lazy-loaded frames. As far as I know, there's nothing stopping you from naming a url route myview.html, and so the logic behind inferring whether a given frame tag has a lazy-loaded URL as its first argument, or a URL route, might be a bit ambiguous.

I imagine it would be cleaner to either separate the functionality into a separate tag, or have the API for the turbo_frame tag be a bit more explicit –– possibly, the "loader" template remains the first positional argument, while turbo_src would designate a lazy-loaded frame, similar to the underlying turbo_frame element itself.

Hope that makes sense! Thanks again for writing this up, and happy holidays!

@blopker
Copy link
Member Author

blopker commented Dec 27, 2020

Thanks for the feedback! You're spot on about the laziness being ambiguous. I was trying to be too clever... I like both your suggestions. I'm leaning towards using turbo_src just because it feels more consistent with the Rails version. I'll edit the proposal to see how it feels.

@davish
Copy link
Contributor

davish commented Dec 27, 2020

This proposal looks great to me as it stands right now! Tagging @C4ptainCrunch in case they'd like to take a look. @blopker, would you like to work on the implementation for this? If not, I can try and take a crack at it sometime in the next few days, just let me know!

@danjac
Copy link

danjac commented Dec 27, 2020

You would probably need to have the ability to pass in variables as with an {% include %} tag. I'm wondering what the benefit is here vs just using plain <turbo-frame> with an include; it's just a couple lines of markup and you would essentially be replicating what include does.

@blopker
Copy link
Member Author

blopker commented Dec 27, 2020

Yeah, I got it all working with the example chat app, both static and lazy frames, but you've read my mind. We don't get much from having it. One thing we can do is auto-generate the frame ID like the Rails version. It's one less thing to think about in the simple case I guess. The other advantage is we get a facade to defend our users from breaking changes in the underlying library, or in case we do add extra features later. Such as some kind of error checking. Users will get it for free.

Implementation as of now (no ID generation):

# turbo/templatetags/turbo.py
from django import template
from django.template import Template
from django.urls import reverse

register = template.Library()
turbo_frame_tpl = Template(
"""
<turbo-frame {{ turbo_args|safe }}>
{% include turbo_template %}
</turbo-frame>
""")

@register.inclusion_tag(turbo_frame_tpl, takes_context=True)
def turbo_frame(context, turbo_template=Template(''), turbo_id=None, turbo_target=None, turbo_src=None, **kwargs):
    turbo_args = f'id="{turbo_id}"'
    if turbo_src is not None:
        if '/' not in turbo_src:
            turbo_src = reverse(turbo_src, kwargs=kwargs)
        turbo_args += f' src="{turbo_src}"'
    if turbo_target is not None:
        turbo_args += f' target="{turbo_target}"'
    params = {'turbo_template': turbo_template, 'turbo_args': turbo_args}
    params.update(context.flatten())
    return params

@danjac
Copy link

danjac commented Dec 27, 2020

OK, but this makes anything beyond the very simple use case difficult: for example, let's say we have a row of items, and each item has it's own frame. Here's the non-template tag version:

{% for item in items %}
<div>
{{ item.title }}
<turbo-frame id="item-subscribe-{{ item.id }}">
 {% include "subscribe.html" with css_class="in-list" %}  
</turbo-frame>
</div>
{% endfor %}

This is a bit contrived, but a couple things: first it's easy to build a frame ID as it's just interpolated in-template markup. Second I can use an include, which means I can pass in scoped variables using "with". While these are doable in a template tag, it adds more complexity than it takes away. Doing the same thing with a tag:

{% for item in items %}
<div>
{{ item.title }}
{% with dom_id="item-subscribe-"|add:item.id|stringformat:"s" css_class="in-list" %}
{% turbo_frame "subscribe.html" dom_id %}
{% endwith %
</div>
{% endfor %}

I'd argue the first example is easier to write and maintain, plus I can keep that "css_class" argument scoped to the include template.

While there may be some changes to the markup, I'd argue this is a good argument for not adding a template tag, as we don't know how much the underlying semantics and behavior will change as well. Hotwire is very beta and as more people start using it and making PRs as they run into limitations it's likely to change quite a bit (right now it's really only used in serious production by one team on one project), so we should avoid too much premature abstraction while it all shakes out.

@davish
Copy link
Contributor

davish commented Dec 27, 2020

One of the reasons I think some kind of abstraction over the Turbo elements is useful is parity with turbo-rails. Since it's maintained alongside Turbo at Basecamp, as the library and official documentation evolves, developers looking to use turbo-django will be looking for analogs to turbo-rails's constructs in Django. You're right in saying the Turbo's APIs will probably take a while to stabilize, but I don't think that means that building these abstractions won't be useful. I imagine that turbo-django versions will be pinned to versions of the turbo library for the forseeable future, and won't be able to be declared "stable" until the underlying Turbo library is stable. An added benefit of abstractions over the base elements is that if the underlying API does change, we may be able to provide more of an upgrade path than Turbo itself. Obviously the idioms and features of Rails and Django don't map one-to-one, and I see the issues you've brought up with the template tag as proposed right now. It's possible the template-tag level abstraction isn't a useful one in Django, and we should focus more on helpers for Views and Forms.

Passing context down to the child template will definitely be necessary, and might be able to be accomplished by subclassing the include tag or using some more advanced custom template features.

I tend to agree though with your comment in #4 that most of the benefits that turbo-django can bring to the table are abstractions at the View (including request/response) and Form layers to handle the plumbing of building pages with Frames and Streams. I just also think that these markup helpers provide some benefits as well, and are worth thinking through.

@JulianFeinauer
Copy link
Contributor

Hey @blopker, thanks so much for putting this proposal together! Overall, I think this makes a lot of sense. The one part that I wanted to ask about was the specific API around lazy-loaded frames. As far as I know, there's nothing stopping you from naming a url route myview.html, and so the logic behind inferring whether a given frame tag has a lazy-loaded URL as its first argument, or a URL route, might be a bit ambiguous.

I imagine it would be cleaner to either separate the functionality into a separate tag, or have the API for the turbo_frame tag be a bit more explicit –– possibly, the "loader" template remains the first positional argument, while turbo_src would designate a lazy-loaded frame, similar to the underlying turbo_frame element itself.

Hope that makes sense! Thanks again for writing this up, and happy holidays!

Hey, regarding lazy loading... I wanted to share my work so far here.
I put together this little Project https://github.com/hotwire-django/turbo-lazy where I made a small set of tags around lazy loading based on Turbo-Frames.

An Example would look like that in code:

{% lazy 'apps.core.partial_views._machine_card' poll_status.machine_id %}
    {% include 'core/partials/_machine_card_loading.html' with name=poll_status.name machine_id=poll_status.machine_id %}
{% endlazy %}

So the inner part would be rendered initially and the part given in the lazy part would be placed as src tag (and wrapped accordingly).
I also addad a "proxy" view that has to be wired as URL then you can also lazy load views that you did not wire up with a separate url. So something I would call "rich partials" (because they can get their own context as well).

What do you think about this approach to lazy loading @blopker and @davish ?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants