From 474e4f4d87a40b4b7e276eb09aeeccdd76d3c437 Mon Sep 17 00:00:00 2001 From: Alimedhat000 Date: Wed, 26 Aug 2026 03:52:28 +0300 Subject: [PATCH 1/2] fix(client): race-free scroll sync stops split-view tearing in Firefox The synced-scroll toggle suppressed mirrored-pane echoes with a flag reset in a fresh requestAnimationFrame. Chromium dispatches the mirrored pane's scroll event before that frame, but Firefox delivers it after the reset: the echo then passes the guard and the next genuine update is swallowed, so the preview trails and snaps back continuously while scrolling. Replace the timing flag with position-based echo suppression (an event at the last-written offset is an echo) and batch mirror writes to one per animation frame. Handlers are now stable callbacks, so listeners stop resubscribing on every render. Covered by interaction stories: baseline mirroring both directions plus a deterministic late-echo regression test. --- .../DocumentMain/document-main.stories.tsx | 147 ++++++++++++++++++ .../components/DocumentMain/document-main.tsx | 61 +------- .../DocumentMain/use-scroll-sync.ts | 100 ++++++++++++ 3 files changed, 253 insertions(+), 55 deletions(-) create mode 100644 client/src/features/DocumentPage/components/DocumentMain/use-scroll-sync.ts diff --git a/client/src/features/DocumentPage/components/DocumentMain/document-main.stories.tsx b/client/src/features/DocumentPage/components/DocumentMain/document-main.stories.tsx index c399e6b..1c16acd 100644 --- a/client/src/features/DocumentPage/components/DocumentMain/document-main.stories.tsx +++ b/client/src/features/DocumentPage/components/DocumentMain/document-main.stories.tsx @@ -85,3 +85,150 @@ export const ReadOnly: Story = { await expect(canvas.queryByTitle('Bold')).toBeNull(); }, }; + +/** + * Provider stub seeding enough content that both split panes overflow and + * can actually scroll (the short default seed cannot). + */ +const longProviderFactory: CollabProviderFactory = (options) => { + const ytext = options.document.getText('content'); + if (!ytext.length) { + const paragraphs = Array.from( + { length: 120 }, + (_, i) => + `\n\nParagraph ${i + 1}: lorem ipsum dolor sit amet, consectetur adipiscing elit.`, + ).join(''); + ytext.insert(0, `# Long Document${paragraphs}`); + } + return { destroy: () => {} }; +}; + +/** + * Resolves after n animation frames so effects and rAF callbacks have run. + */ +const rafFrames = (n: number) => + new Promise((resolve) => { + const step = () => (--n <= 0 ? resolve() : requestAnimationFrame(step)); + requestAnimationFrame(step); + }); + +/** + * Resolves on the element's next scroll event; rejects after timeoutMs so a + * swallowed mirror update fails fast instead of hanging the run. + */ +const nextScrollEvent = (el: HTMLElement, timeoutMs = 1000) => + new Promise((resolve, reject) => { + const timer = setTimeout(() => { + el.removeEventListener('scroll', onScroll); + reject(new Error(`no scroll event within ${timeoutMs}ms`)); + }, timeoutMs); + const onScroll = () => { + clearTimeout(timer); + el.removeEventListener('scroll', onScroll); + resolve(); + }; + el.addEventListener('scroll', onScroll); + }); + +const renderSplitWithLongDoc: Story['render'] = function RenderedStory() { + return ( +
+ +
+ ); +}; + +/** Enables synced scrolling via the handle overlay button. */ +async function enableSyncScroll(canvasElement: HTMLElement) { + const toggle = canvasElement.querySelector( + '[data-panel-resize-handle-id] div.absolute', + ) as HTMLElement | null; + if (!toggle) throw new Error('scroll-sync toggle not found'); + toggle.click(); + await rafFrames(3); +} + +export const SplitSyncMirrorsScroll: Story = { + render: renderSplitWithLongDoc, + play: async ({ canvasElement }) => { + await enableSyncScroll(canvasElement); + const editor = canvasElement + .querySelector('.cm-editor') + ?.closest('div.custom-scrollbar.overflow-y-scroll') as HTMLElement; + const preview = canvasElement.querySelector( + '.markdown-previewer', + ) as HTMLElement; + + // Editor -> preview. Expectations are computed against live geometry: + // CodeMirror's scrollHeight can still grow during deep scrolls. + const edMax = editor.scrollHeight - editor.clientHeight; + const pvMax = preview.scrollHeight - preview.clientHeight; + + editor.scrollTop = edMax * 0.4; + await nextScrollEvent(preview); + await rafFrames(2); + expect( + Math.abs( + preview.scrollTop - + (editor.scrollTop / (editor.scrollHeight - editor.clientHeight)) * + (preview.scrollHeight - preview.clientHeight), + ), + ).toBeLessThan(pvMax * 0.05); + + // Preview -> editor + const pvMax2 = preview.scrollHeight - preview.clientHeight; + preview.scrollTop = pvMax2 * 0.8; + await nextScrollEvent(editor); + await rafFrames(2); + expect( + Math.abs( + editor.scrollTop - + (preview.scrollTop / (preview.scrollHeight - preview.clientHeight)) * + (editor.scrollHeight - editor.clientHeight), + ), + ).toBeLessThan(edMax * 0.05); + }, +}; + +export const SplitSyncSurvivesLateEcho: Story = { + render: renderSplitWithLongDoc, + play: async ({ canvasElement }) => { + await enableSyncScroll(canvasElement); + const editor = canvasElement + .querySelector('.cm-editor') + ?.closest('div.custom-scrollbar.overflow-y-scroll') as HTMLElement; + const preview = canvasElement.querySelector( + '.markdown-previewer', + ) as HTMLElement; + + const edMax = editor.scrollHeight - editor.clientHeight; + const pvMax = preview.scrollHeight - preview.clientHeight; + + // A normal mirrored scroll leaves both panes aligned... + editor.scrollTop = edMax * 0.5; + await nextScrollEvent(preview); + await rafFrames(2); + + // ...then the mirrored pane's own scroll event arrives one frame LATE + // (Firefox delivers it after the syncing flag was already reset). It must + // be recognized as an echo, not consume the suppression state. + preview.dispatchEvent(new Event('scroll')); + + // And a genuine editor scroll in the SAME task must still be mirrored. + editor.scrollTop = edMax * 0.75; + await rafFrames(5); + expect( + Math.abs( + preview.scrollTop - + (editor.scrollTop / (editor.scrollHeight - editor.clientHeight)) * + (preview.scrollHeight - preview.clientHeight), + ), + ).toBeLessThan(pvMax * 0.05); + }, +}; diff --git a/client/src/features/DocumentPage/components/DocumentMain/document-main.tsx b/client/src/features/DocumentPage/components/DocumentMain/document-main.tsx index ca676af..da163b3 100644 --- a/client/src/features/DocumentPage/components/DocumentMain/document-main.tsx +++ b/client/src/features/DocumentPage/components/DocumentMain/document-main.tsx @@ -10,6 +10,7 @@ import { cn } from '@/utils/cn'; import { MarkdownEditor } from './MarkdownEditor'; import { MarkdownPreview } from './MarkdownPreview'; +import { useScrollSync } from './use-scroll-sync'; /** * Wires collaboration state into either MarkdownEditor or MarkdownPreview per view mode. @@ -44,8 +45,12 @@ export function DocumentMain({ ); const editorScrollRef = useRef(null); const previewScrollRef = useRef(null); - const isSyncingRef = useRef(false); const [syncScroll, setSyncScroll] = useState(false); + const { handleEditorScroll, handlePreviewScroll } = useScrollSync({ + enabled: syncScroll, + editorRef: editorScrollRef, + previewRef: previewScrollRef, + }); useEffect(() => { if (doc && text !== doc.content) { @@ -70,60 +75,6 @@ export function DocumentMain({ percent * (previewEl.scrollHeight - previewEl.clientHeight); }, [syncScroll]); - const handleEditorScroll = () => { - if (!syncScroll) return; - if ( - !syncScroll || - isSyncingRef.current || - !editorScrollRef.current || - !previewScrollRef.current - ) { - return; - } - - isSyncingRef.current = true; - - const editor = editorScrollRef.current; - const preview = previewScrollRef.current; - - const scrollRatio = - editor.scrollTop / (editor.scrollHeight - editor.clientHeight); - preview.scrollTop = - scrollRatio * (preview.scrollHeight - preview.clientHeight); - - // Use a shorter timeout and requestAnimationFrame - requestAnimationFrame(() => { - isSyncingRef.current = false; - }); - }; - - const handlePreviewScroll = () => { - if (!syncScroll) return; - - if ( - !syncScroll || - isSyncingRef.current || - !editorScrollRef.current || - !previewScrollRef.current - ) { - return; - } - - isSyncingRef.current = true; - - const editor = editorScrollRef.current; - const preview = previewScrollRef.current; - - const scrollRatio = - preview.scrollTop / (preview.scrollHeight - preview.clientHeight); - editor.scrollTop = - scrollRatio * (editor.scrollHeight - editor.clientHeight); - - // Use a shorter timeout and requestAnimationFrame - requestAnimationFrame(() => { - isSyncingRef.current = false; - }); - }; if (!docId || !isReady || !ydoc || !ytext || !provider) { return (
diff --git a/client/src/features/DocumentPage/components/DocumentMain/use-scroll-sync.ts b/client/src/features/DocumentPage/components/DocumentMain/use-scroll-sync.ts new file mode 100644 index 0000000..4acc6b4 --- /dev/null +++ b/client/src/features/DocumentPage/components/DocumentMain/use-scroll-sync.ts @@ -0,0 +1,100 @@ +import { useCallback, useEffect, useRef } from 'react'; + +type PaneKey = 'editor' | 'preview'; + +/** + * Race-free bidirectional scroll syncing between two panes. + * + * Mirrors scroll positions once per animation frame (latest wins) and + * recognizes its own mirrored writes by position: an event whose target is + * already at the last-written offset is an echo and is ignored. This replaces + * timing-flag suppression, which browsers deliver in different orders + * (Firefox dispatches the mirrored pane's scroll event after the reset frame, + * swallowing every other genuine update). + * + * @param args - Hook arguments. + * @param args.enabled - Mirrors only run while true. + * @param args.editorRef - Scrollable editor container. + * @param args.previewRef - Scrollable preview container. + * @returns Stable scroll handlers to attach to each pane's container. + */ +export function useScrollSync(args: { + /** Mirrors only run while true. */ + enabled: boolean; + /** Scrollable editor container. */ + editorRef: React.RefObject; + /** Scrollable preview container. */ + previewRef: React.RefObject; +}) { + const { enabled, editorRef, previewRef } = args; + const enabledRef = useRef(enabled); + const lastWritten = useRef>({ + editor: Number.NaN, + preview: Number.NaN, + }); + const frameRef = useRef(null); + const pendingSource = useRef(null); + + useEffect(() => { + enabledRef.current = enabled; + if (!enabled) { + if (frameRef.current !== null) cancelAnimationFrame(frameRef.current); + frameRef.current = null; + pendingSource.current = null; + lastWritten.current = { editor: Number.NaN, preview: Number.NaN }; + } + return () => { + if (frameRef.current !== null) cancelAnimationFrame(frameRef.current); + frameRef.current = null; + }; + }, [enabled]); + + const applyMirror = useCallback(() => { + frameRef.current = null; + const source = pendingSource.current; + pendingSource.current = null; + if (!source || !enabledRef.current) return; + + const from = source === 'editor' ? editorRef.current : previewRef.current; + const to = source === 'editor' ? previewRef.current : editorRef.current; + if (!from || !to) return; + + const fromRange = from.scrollHeight - from.clientHeight; + if (fromRange <= 0) return; + + const top = + (from.scrollTop / fromRange) * (to.scrollHeight - to.clientHeight); + lastWritten.current[source === 'editor' ? 'preview' : 'editor'] = top; + to.scrollTop = top; + }, [editorRef, previewRef]); + + const queueMirror = useCallback( + (which: PaneKey) => { + if (!enabledRef.current) return; + const el = which === 'editor' ? editorRef.current : previewRef.current; + if (!el) return; + + const written = lastWritten.current[which]; + if (!Number.isNaN(written) && Math.abs(el.scrollTop - written) < 1) { + return; + } + + pendingSource.current = which; + if (frameRef.current === null) { + frameRef.current = requestAnimationFrame(applyMirror); + } + }, + [editorRef, previewRef, applyMirror], + ); + + const handleEditorScroll = useCallback( + () => queueMirror('editor'), + [queueMirror], + ); + const handlePreviewScroll = useCallback( + () => queueMirror('preview'), + [queueMirror], + ); + + return { handleEditorScroll, handlePreviewScroll }; +} From 7b7116c92904b4730f6500ef35ac0deade2937af Mon Sep 17 00:00:00 2001 From: Alimedhat000 Date: Wed, 26 Aug 2026 04:08:01 +0300 Subject: [PATCH 2/2] test(client): cover latest-wins mirroring under same-frame scroll bursts Chromium coalesces same-frame scroll events, so no existing story failed on the old guard when two updates landed in one frame. Deliver the first update, then add a second within the same frame (Firefox's per-write delivery): the old code swallows the second and the mirror settles stale; the new batched mirror converges on the latest position. --- .../DocumentMain/document-main.stories.tsx | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/client/src/features/DocumentPage/components/DocumentMain/document-main.stories.tsx b/client/src/features/DocumentPage/components/DocumentMain/document-main.stories.tsx index 1c16acd..62b43b7 100644 --- a/client/src/features/DocumentPage/components/DocumentMain/document-main.stories.tsx +++ b/client/src/features/DocumentPage/components/DocumentMain/document-main.stories.tsx @@ -232,3 +232,47 @@ export const SplitSyncSurvivesLateEcho: Story = { ).toBeLessThan(pvMax * 0.05); }, }; + +export const SplitSyncMirrorsLatestUnderBurst: Story = { + render: renderSplitWithLongDoc, + play: async ({ canvasElement }) => { + await enableSyncScroll(canvasElement); + const editor = canvasElement + .querySelector('.cm-editor') + ?.closest('div.custom-scrollbar.overflow-y-scroll') as HTMLElement; + const preview = canvasElement.querySelector( + '.markdown-previewer', + ) as HTMLElement; + + const edMax = editor.scrollHeight - editor.clientHeight; + const pvMax = preview.scrollHeight - preview.clientHeight; + + // Two scroll updates land within one frame but are delivered separately + // (Firefox dispatches a scroll event per wheel-tick write instead of + // coalescing them). The second must not be swallowed by suppression + // state left behind by the first: the mirror has to end up at the + // LATEST position once the frame flushes. + editor.scrollTop = edMax * 0.2; + await nextScrollEvent(editor); + editor.scrollTop = edMax * 0.6; + await rafFrames(3); + expect( + Math.abs( + preview.scrollTop - + (editor.scrollTop / (editor.scrollHeight - editor.clientHeight)) * + (preview.scrollHeight - preview.clientHeight), + ), + ).toBeLessThan(pvMax * 0.05); + + // ...and a third update after the burst still mirrors. + editor.scrollTop = edMax * 0.85; + await rafFrames(3); + expect( + Math.abs( + preview.scrollTop - + (editor.scrollTop / (editor.scrollHeight - editor.clientHeight)) * + (preview.scrollHeight - preview.clientHeight), + ), + ).toBeLessThan(pvMax * 0.05); + }, +};