hotwire-django / turbo-django

Unmaintained // An early stage integration of Hotwire Turbo with Django
https://discuss.hotwire.dev/t/django-backend-support-for-hotwire/1570/
Other
384 stars 20 forks source link

Update specific instance instead of the entire container with dom_target #30

Open MilyMilo opened 3 years ago

MilyMilo commented 3 years ago

Hey guys! First of all thank you for the work you've done on this implementation, it's a great starting point to implementing turbo with django and I think It'll only get better.

I have a question concerning the dom_target that generated in get_dom_target in BroadcastableMixin. I have recreated the simple chat application, this is my model setup:

class Room(BroadcastableMixin, models.Model):
    name = models.CharField(max_length=50)

    def get_absolute_url(self):
        return reverse_lazy('chat:room_detail', kwargs={'name': self.name})

    def __str__(self):
        return self.name

class Message(BroadcastableMixin, models.Model):
    broadcasts_to = ['room']
    broadcast_self = False

    room = models.ForeignKey(Room, on_delete=models.CASCADE, related_name='messages')
    content = models.TextField()
    posted_at = models.DateTimeField(default=timezone.now)

    def __str__(self):
        return self.content

And these are the templates:

# room.html
{% extends "base.html" %}
{% load turbo_streams %}

{% block content %}
    {% turbo_stream_from room %}
    <h3>Room {{ room.name }}:</h3>

    <div id="messages">
        {% for message in room.messages.all %}
            {% include "chat/message.html" %}
        {% endfor %}
    </div>

    <hr>
    <form method="POST">
        {% csrf_token %}
        {{ message_form.as_p }}
        <button type="submit">Add Message</button>
    </form>
{% endblock %}

# message.html
{% load humanize turbo_streams %}
<p id="{% stream_id message %}">{{ message.posted_at|naturaltime }}: {{ message }} (#{{message.pk}})</p>

The problem I am encountering is, whenever a message changes, there's a signal sent with domtarget pointing at messages. I don't think that should be the case, I'd like to target. specific `message` to avoid re-rendering all the messages (which btw don't render with that setup, all my messages get replaced with just a single one - the updated one).

An example signal that gets sent, as you see dom_target is messages not message_9:

{'signed_channel_name': 'broadcast-chat.room-1:xPgfqh9WzMQHO_y8OdfE7-0H1ePopB0vfWRZIWtg1hQ', 'data': '<turbo-stream action="replace" target="messages">\n  <template>\n      \n          \n<p id="message_9">a minute ago: Welp Welp (#9)</p>\n      \n  </template>\n</turbo-stream>'}

I think get_dom_target should by default return f"{self._meta.verbose_name.lower()}_{self.pk}", I don't really see how messages is helpful. Am I missing something or is this some sort of a bug? If you need more I could share my playground repo with you.

MilyMilo commented 3 years ago

I just confirmed the exact same thing happens in the experiments chat demo. Entire messages container gets replaced with the singular edited message, whenever a message is changed.

davish commented 3 years ago

Huh, this must have been a but that snuck in there in a more recent commit. I'll try and take a look this weekend and get back to you, hopefully with a fix. Thanks for reporting this!

JoiRichi commented 2 years ago

While using streams, the whole loop gets re-rendered in templates which changes the position of the updated instance in the template. I think this needs an urgent fix