Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make the MessageEditModal close *after* there's a response from the server #36

Open
smellyshovel opened this issue Sep 28, 2023 · 1 comment

Comments

@smellyshovel
Copy link
Contributor

          > > Most probably that's due to the fact that patching the messages changes some parts of it which are used as the `key` somewhere in the messages list.

Yes, that's possible. Patching will refetch the message.

You could try changing the parts of message.js store where it sets the message in list, so that for pre-existing messages, instead of overwriting the whole object (which might trigger reactivity where the id is used as a key) we set every property except id. That might stop reactivity happening.

However this might apply to anything where we use a reactive value as part of the key. That happens a lot with lots of stores, often using id. If that is the issue then we need to think about it some more. I think normally that would just result in extra re-renders which we wouldn't notice, but your change exposes it as an issue.

Let's see if that's the problem first.

OK. What I'll do then is just close the MessageEditModal before the await statement (that's while the modal for sure still exists). That works, though, obviously, not ideal. It's just that I really wouldn't like to mess around with the stores under the scope of this PR. Let's move it to another issue instead. From this PR's perspective the modal should close, and with my fix it does. The fact that it doesn't close exactly when it should is a bug and we'll address it separately.

Originally posted by @smellyshovel in #32 (comment)

@smellyshovel smellyshovel changed the title Most probably that's due to the fact that patching the messages changes some parts of it which are used as the key somewhere in the messages list. Make the MessageEditModal close *after* there's a response from the server Sep 28, 2023
@edwh
Copy link
Member

edwh commented Sep 28, 2023

That works, though, obviously, not ideal.

Yes, that's fine.

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

No branches or pull requests

2 participants