[FIX] Image attachment re-renders on message update

[Kailash0311]

Closes #5028 and #14092

Issue and research: On researching and playing around with codebase a bit I found out that message is re-created everytime someone updates the message, (e.g: Starring or adding a reaction as mentioned in the issues that this PR solves.)Since it is meteor's default reactivity system because of which it is happening, my resolution is a simple work-around.

Resolution: Add a onDestroy function to the lazyloadtemplate(This template is responsible for showing the preview of the attachment.) This template gets destroyed and recreated whenever there is some update in the message (e.g: Starring or adding a reaction). Workaround is a simple Template.onDestroy call that stores the value of loaded state before being destroyed because of the update in the message. So whenever the template is recreated, now this.loaded has the same state as what was before updating a message(or being destroyed).

[geekgonecrazy]

@RocketChat/core if this solves... this would be amazing to include. An extremely annoying issue

[geekgonecrazy]

Does this work for others? Like say youtube embed?

[Kailash0311]

Does this work for others? Like say youtube embed?

No, it doesn't because youtube embed doesn't depend upon lazyloadImage package. But I guess the strategy would be the same there too.

[rodrigok]

@Kailash0311 Ok, but with your solution ALL images will share the same control variable right? Doesn't that defeats the purpose?

[rodrigok]

A possible solution may be creating a Set to act as a cache where you store the src and every time you create the template instance you define the loaded initial state as true if the src already exists in Set.

[rodrigok]

To be clear, this will not stop the re render @geekgonecrazy , will just prevent the render of the preview/base64 image and then the original image, so it may blink less, but videos would back to the initial state due to the re render. To solve that we need to remove NRR or move to another template language,

[geekgonecrazy]

Ah! So this is only preventing the blury base64 to regular. Since the other image is already downloaded it re-renders fast enough you don't notice it.

Drats.. I was hoping solving it really was this simple as its an extremely annoying issue.

Avoiding the blurry to clear flicker still would be a welcome change though

[Kailash0311]

@Kailash0311 Ok, but with your solution ALL images will share the same control variable right? Doesn't that defeats the purpose?

Oh, yes. How could I not see that! That is why new images were never loading the preview image.
Thanks!

[Kailash0311]

A possible solution may be creating a Set to act as a cache where you store the src and every time you create the template instance you define the loaded initial state as true if the src already exists in Set.

@rodrigok Do you mean using set method in Session package that meteor provides? Or a normal array of objects?

[Kailash0311]

To be clear, this will not stop the re render @geekgonecrazy , will just prevent the render of the preview/base64 image and then the original image, so it may blink less, but videos would back to the initial state due to the re render. To solve that we need to remove NRR or move to another template language,

Yes, this will not stop re-render as the message itself is being re-rendered altogether. This will just skip the loading of the preview image and directly load the main image.

[rodrigok]

@Kailash0311 I mean the Set class https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set

[rodrigok]

With this changes ALL images will share the same control variable.

[Kailash0311]

With this changes ALL images will share the same control variable.

Yes, I understood it.

[Kailash0311]

@rodrigok In the recent changes, the PREVIEW image will not be re-loaded when the message is re-rendered.