turbo-django icon indicating copy to clipboard operation
turbo-django copied to clipboard

Update specific instance instead of the entire container with dom_target

Open MilyMilo opened this issue 4 years ago • 3 comments

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 dom_target pointing at messages. I don't think that should be the case, I'd like to target. specific message_<pk> 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 avatar Feb 16 '21 18:02 MilyMilo

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.

MilyMilo avatar Feb 16 '21 20:02 MilyMilo

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!

davish avatar Feb 18 '21 18:02 davish

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

JoiRichi avatar Jan 16 '22 04:01 JoiRichi