Remove flickering when preview is being updated

Unassigning ticket for now (may need some BE help)

@fazilitehreem Why does this need back-end help? AFAICT this is a 100% client-side challenge? 🤔 What am I missing? 🙈

Is this a duplicate of ✨ Partial preview updates: update preview of modified component only, not entire component tree Active ? Or is the thinking to implement some workaround in this issue that removes the flicker without solving the full scope of ✨ Partial preview updates: update preview of modified component only, not entire component tree Active ?

I don't know if ✨ Partial preview updates: update preview of modified component only, not entire component tree Active would solve this. Another question is that would we still have to do full update of the preview for some actions? Would we for example be able to do partial updates for undos and sorting?

#5 + #6: AFAICT:

• This issue can/should be 100% client-side.
• ✨ Partial preview updates: update preview of modified component only, not entire component tree Active is about having A) less data sent+received, B) hence speed up updates

#3462360: Partial preview updates: update preview of modified component only, not entire component tree is about having A) less data sent+received, B) hence speed up updates

I think it's also about C) replace the full iframe content replacement with code that reaches into the iframe's document and only replaces the modified component.

My initial thought when I wrote #5 is that the flicker is due to something about the fact that we're currently replacing the iframe's entire srcdoc, including its head section that references all the CSS.

However, looking at the GIF again in this issue's summary, it seems like maybe the flicker is actually something else as only a subset of borders on a subset of elements are flickering.

Addressing #4

Why does this need back-end help? AFAICT this is a 100% client-side challenge?

During my investigation, I observed that the values appear to be rendered through the backend. It closely resembles an AJAX call, as the entire srcdoc of the iframe is being replaced when the value is updated. This led me to believe that backend support might be necessary to understand how the values within the srcdoc are being updated. I would like to determine whether this process is handled entirely within the React frontend or if backend logic is involved.

@effulgentsia in #9:

[...] looking at the GIF again in this issue's summary, it seems like maybe the flicker is actually something else as only a subset of borders on a subset of elements are flickering.

I think those might be scrollbars? I'm not seeing those in my environment. I do see the entire element flickering.

I think it's also about C) replace the iframe srcdoc replacement with code that reaches into the iframe's document and only replaces the modified component.

I think this is the key here. I believe any time we replace the string that is the srcdoc, we're telling the browser that there is a new document to deal with, so it needs to construct a new DOM, compute the layout, load assets etc.

I did this small experiment where I have two iframes, the content for both lives in srcdoc, and by clicking a button we replace the contents of the srcdoc attribute to see how the re-rendering looks with some randomized changes. The first iframe only has inline styles, the second runs Tailwind CSS from their Play CDN (I say runs it, because it's not just a stylesheet include, it's JavaScript that generates the CSS classes based on the HTML markup — so it's a bit heaver processing for the experiment.)

You can see the experiment here: https://ztph97.csb.app
(source code).

I didn't end up learning from this as much as I hoped, but I did observe one thing: I can easily see the flicker issue in Chrome for both iframes, even with the one with inline styles. That is, however, not the case in Safari or Firefox, not even with the iframe loading the Tailwind script once it's cached. So I went back to the XB app, and noticed the same: basically no flicker in Safari or Firefox as far as I can tell!

We could dig into the problem from this angle, and try to find out if there's a remedy for Chrome. Though I would still be worried about other browsers, other operating systems etc.

I agree with #9 that we may need to consider backend support. Having access to structured data that tells us the markup for each layout node would allow us to build and maintain the DOM elements for the preview like it was mentioned in #8. ✨ Partial preview updates: update preview of modified component only, not entire component tree Active would make that performant.

Addressing @laurii's question from #6:

[...] would we still have to do full update of the preview for some actions? Would we for example be able to do partial updates for undos and sorting?

I don't think we actually need a preview update when sorting happens, except for adding new components. It happens now because we update the layout data in the Redux store, which dispatches the preview update because adding a new component has the same effect, and we don't differentiate. For undo/redo, it can get quite complex to avoid a full preview update.

no flicker in Safari or Firefox

Nice!

find out if there's a remedy for Chrome

Does the flicker go away if you go to chrome://settings/system and turn off "graphics acceleration"?

I think another experiment that would be informative is to try instead of replacing the srcdoc to parse out just the part of the srcdoc string between <body> and </body>, and replace the iframe's body.innerHTML with that. We wouldn't necessarily be able to actually do this (e.g., the new srcdoc's <head> might actually be different and/or it might have onLoad handlers that need to run) but it would be instructive to know if simply not changing the srcdoc is sufficient to remove the flicker.

Will try to get this fixed.

I tried what's mentioned in #12 but it does not solve the problem.Is there anything that i can try apart from this?

Unassigning it from me as this requires fairly substantial refactor of the preview system as discussed with Jesse.

I tried what's mentioned in #12 but it does not solve the problem.

Thank you, @utkarsh_33. That's really helpful to know that something about what's being changed within the <body> is causing the flickering rather than it being solely due to a full document reload. Can you share the code that you used for testing this (either an issue fork or a patch file, whichever is easier for you)? And if possible, also a screen recording with the code applied that shows the flicker?

it seems like maybe the flicker is actually something else as only a subset of borders on a subset of elements are flickering

Sorry, looking at the issue summary's animated gif more closely, it's not a subset of elements, it's the two different iframes: the desktop sized one and the mobile sized one.

...I think those might be scrollbars?

They don't look like scrollbars to me, but it's hard to tell. For the desktop sized iframe, it looks like just the bottom and right borders, which would be consistent with a scrollbar issue. For the mobile sized iframe, it looks like the entire black bar at the bottom is flickering, which seems like more height than would be explained by a scrollbar.

If it is a scrollbar issue (or something else about the bottom and right borders), does that point to any specific HTML or CSS that's coming in at a slightly later time than everything else? Is Big Pipe module enabled, and if so, does uninstalling it change anything?

@effulgentsia — Yes, it's hard to tell from a gif, so I recorded a video to more accurately show how things look now:
https://balintbrews.com/video/xb-preview-flicker

At 0:26 I switch from Chrome to Safari. You can see that the flicker almost always happens in Chrome, whereas it never does in Safari. This is consistent with what my previous experiment shows: https://ztph97.csb.app.

Uninstalling the BigPipe module didn't make a difference, neither did disabling graphics acceleration in Chrome.

I think what you suggested in #12 should be what we try next. Maybe @utkarsh_33 already did? I would love to see some code, if that's the case. 🙂 We can even try that in my CodeSandbox, https://ztph97.csb.app, where forking is supported.

Oh hey, starting with Chrome 127, view transitions are supported in iframes.

I wonder if all we need to do is to add <style>@view-transition {navigation: auto;}</style> inside of the <head> of the string we're about to set as the iframe's srcDoc before setting it?

I've done this quick test, and it didn't help, but we may need to tweak the CSS — if someone has time to play with that, that would be great:

diff --git a/ui/src/features/layout/preview/Preview.tsx b/ui/src/features/layout/preview/Preview.tsx
index 12a472c..59bc0f6 100644
--- a/ui/src/features/layout/preview/Preview.tsx
+++ b/ui/src/features/layout/preview/Preview.tsx
@@ -42,7 +42,14 @@ const Preview: React.FC<PreviewProps> = () => {
         // Trigger the mutation
         const result = await postPreview({ layout, model }).unwrap();
         // Handle the successful response here
-        setFrameSrcDoc(result.html);
+        const additionalStyles = `
+          <style>@view-transition {navigation: auto;}</style>
+        `;
+        const modifiedHtml = result.html.replace(
+          '</head>',
+          `${additionalStyles}</head>`,
+        );
+        setFrameSrcDoc(modifiedHtml);
       } catch (err) {
         showBoundary(err);
       }

Here a quick explanation of what i tried according to #12.
In the current state of the MR the flickering is gone if we select a component and edit any props value in the contextual form.But the problem is that because of the updated useEffect we are not able to select any other component which makes sense as we have an if condition that is causing it to behave this way.
I just wanted to highlight that if we proceed with some more related changes to #12 we might come close to solution(not sure though).

I also tried to keep a local state inside the viewPort.tsx just to keep the track of changes in the srcDoc and alter the if condition accordingly but that also didn't work.

I have attached a video demonstrating that the flickering issue is gone when we update the props value inside the contextual form.

Marking it NR so that we can have an update on what's next that someone can try in order to get this fixed.

Also one thing to notice is when we have our browser console open then the flickering is less as compared to what it is normally.It makes me think that can there be something related to CSS that's causing the issue as mentioned in #20 and #21.

After some (😅) trial and error, I've finally got the tests all passing and seemingly not flakey (famous last words) and the linting all sorted.

Ready for review!

Code reviewed, and Jesse and I discussed possible places where this could lead to potential test failures and no red flags. Thus, MR is approved and RTBC.

It works great - glad to see this happening.

wim leers → closed merge request !285

YAY!

All that's missing and we should still add: a GIF or video demonstrating how well it works now :)

Here's a video of the latest in 0.x:

https://youtu.be/lycpMtHT2Qs

@kristen pol Thanks for the video, but it's not clear to me what your point is? Are you confirming it works well now? Or are you pointing out there is a problem?

🤩 Thanks, @jessebaker!

It works well. I just re-noticed an unrelated issue when testing. Sorry that I didn’t make that clear. I should create an issue for that before I forget.

Automatically closed - issue fixed for 2 weeks with no activity.