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

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

Open
MilyMilo opened this issue Feb 16, 2021 · 3 comments
Open

Comments

@MilyMilo
Copy link

MilyMilo commented Feb 16, 2021

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
Copy link
Author

MilyMilo commented Feb 16, 2021

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
Copy link
Contributor

davish commented Feb 18, 2021

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
Copy link
Contributor

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

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

3 participants