From fd7ec28e3d2ef6745c57ed986d814f7a6dd9b07e Mon Sep 17 00:00:00 2001
From: Sage Abdullah <sage.abdullah@torchbox.com>
Date: Tue, 11 Feb 2025 11:32:11 +0000
Subject: [PATCH] Force preview panel scroll behavior to instant to avoid
 flickering

---
 .../src/controllers/PreviewController.test.js | 49 +++++++++++--------
 client/src/controllers/PreviewController.ts   | 12 +++--
 2 files changed, 36 insertions(+), 25 deletions(-)

diff --git a/client/src/controllers/PreviewController.test.js b/client/src/controllers/PreviewController.test.js
index dc2d28d7bb..29d155dcc9 100644
--- a/client/src/controllers/PreviewController.test.js
+++ b/client/src/controllers/PreviewController.test.js
@@ -209,6 +209,10 @@ describe('PreviewController', () => {
     expect(newIframe.src).toEqual(expectedUrl);
     expect(newIframe.classList.contains('w-preview__proxy')).toBe(true);
 
+    // Pretend the old iframe has scrolled
+    oldIframe.contentWindow.scrollX = 200;
+    oldIframe.contentWindow.scrollY = 100;
+
     // Simulate the iframe loading
     const mockScroll = jest.fn();
     newIframe.contentWindow.scroll = mockScroll;
@@ -221,10 +225,11 @@ describe('PreviewController', () => {
     expect(newIframe.id).toEqual(oldIframeId);
     expect(newIframe.src).toEqual(expectedUrl);
     expect(newIframe.getAttribute('style')).toBeNull();
-    expect(newIframe.contentWindow.scroll).toHaveBeenCalledWith(
-      oldIframe.contentWindow.scrollX,
-      oldIframe.contentWindow.scrollY,
-    );
+    expect(newIframe.contentWindow.scroll).toHaveBeenCalledWith({
+      top: oldIframe.contentWindow.scrollY,
+      left: oldIframe.contentWindow.scrollX,
+      behavior: 'instant',
+    });
 
     // Clear the fetch call history
     fetch.mockClear();
@@ -443,10 +448,11 @@ describe('PreviewController', () => {
       expect(iframes[0]).toBe(newIframe);
       expect(newIframe.src).toEqual(expectedUrl);
       expect(newIframe.getAttribute('style')).toBeNull();
-      expect(newIframe.contentWindow.scroll).toHaveBeenCalledWith(
-        oldIframe.contentWindow.scrollX,
-        oldIframe.contentWindow.scrollY,
-      );
+      expect(newIframe.contentWindow.scroll).toHaveBeenCalledWith({
+        top: oldIframe.contentWindow.scrollY,
+        left: oldIframe.contentWindow.scrollX,
+        behavior: 'instant',
+      });
 
       // Should set the device width property to the selected size (the default)
       const element = document.querySelector('[data-controller="w-preview"]');
@@ -564,10 +570,11 @@ describe('PreviewController', () => {
       expect(iframes[0]).toBe(newIframe);
       expect(newIframe.src).toEqual(expectedUrl);
       expect(newIframe.getAttribute('style')).toBeNull();
-      expect(newIframe.contentWindow.scroll).toHaveBeenCalledWith(
-        oldIframe.contentWindow.scrollX,
-        oldIframe.contentWindow.scrollY,
-      );
+      expect(newIframe.contentWindow.scroll).toHaveBeenCalledWith({
+        top: oldIframe.contentWindow.scrollY,
+        left: oldIframe.contentWindow.scrollX,
+        behavior: 'instant',
+      });
 
       // Should set the has-errors class on the controlled element
       expect(element.classList).toContain('w-preview--has-errors');
@@ -715,10 +722,11 @@ describe('PreviewController', () => {
       expect(iframes[0]).toBe(newIframe);
       expect(newIframe.src).toEqual(expectedUrl);
       expect(newIframe.getAttribute('style')).toBeNull();
-      expect(newIframe.contentWindow.scroll).toHaveBeenCalledWith(
-        oldIframe.contentWindow.scrollX,
-        oldIframe.contentWindow.scrollY,
-      );
+      expect(newIframe.contentWindow.scroll).toHaveBeenCalledWith({
+        top: oldIframe.contentWindow.scrollY,
+        left: oldIframe.contentWindow.scrollX,
+        behavior: 'instant',
+      });
 
       // Should set the has-errors class on the controlled element
       expect(element.classList).toContain('w-preview--has-errors');
@@ -915,10 +923,11 @@ describe('PreviewController', () => {
       expect(iframes[0]).toBe(newIframe);
       expect(newIframe.src).toEqual(expectedUrl);
       expect(newIframe.getAttribute('style')).toBeNull();
-      expect(newIframe.contentWindow.scroll).toHaveBeenCalledWith(
-        oldIframe.contentWindow.scrollX,
-        oldIframe.contentWindow.scrollY,
-      );
+      expect(newIframe.contentWindow.scroll).toHaveBeenCalledWith({
+        top: oldIframe.contentWindow.scrollY,
+        left: oldIframe.contentWindow.scrollX,
+        behavior: 'instant',
+      });
       // The spinner should be hidden after the iframe loads
       expect(spinnerElement.hidden).toBe(true);
 
diff --git a/client/src/controllers/PreviewController.ts b/client/src/controllers/PreviewController.ts
index ab55361dd8..4e14db2af7 100644
--- a/client/src/controllers/PreviewController.ts
+++ b/client/src/controllers/PreviewController.ts
@@ -714,11 +714,13 @@ export class PreviewController extends Controller<HTMLElement> {
     // not run the replacement logic in this case.
     if (!newIframe.src) return;
 
-    // Restore scroll position
-    newIframe.contentWindow?.scroll(
-      this.iframeTarget.contentWindow?.scrollX as number,
-      this.iframeTarget.contentWindow?.scrollY as number,
-    );
+    // Restore scroll position with instant scroll to avoid flickering if the
+    // previewed page has scroll-behavior: smooth.
+    newIframe.contentWindow?.scroll({
+      top: this.iframeTarget.contentWindow?.scrollY as number,
+      left: this.iframeTarget.contentWindow?.scrollX as number,
+      behavior: 'instant',
+    });
 
     // Remove any other existing iframes. Normally there are two iframes at this
     // point, the old one and the new one. However, the `load` event may be fired