From dd7a018cb438157758172d438da3b4caa39d6c74 Mon Sep 17 00:00:00 2001 From: EthanHealy01 Date: Mon, 3 Nov 2025 01:30:52 +0000 Subject: [PATCH] cleanups and lint --- .../tools/compare/CompareDocumentPane.tsx | 147 ++++-------- .../tools/compare/CompareWorkbenchView.tsx | 101 +++------ .../core/components/tools/compare/compare.ts | 209 ++++++++++++++++++ .../components/tools/compare/compareView.css | 18 +- .../tools/compare/hooks/useComparePanZoom.ts | 38 +++- .../hooks/useCompareRightRailButtons.tsx | 19 +- frontend/src/core/styles/theme.css | 16 ++ frontend/src/core/tools/Compare.tsx | 6 +- frontend/src/core/types/compare.ts | 5 +- 9 files changed, 370 insertions(+), 189 deletions(-) create mode 100644 frontend/src/core/components/tools/compare/compare.ts diff --git a/frontend/src/core/components/tools/compare/CompareDocumentPane.tsx b/frontend/src/core/components/tools/compare/CompareDocumentPane.tsx index 046dcce3e..9579c0d4a 100644 --- a/frontend/src/core/components/tools/compare/CompareDocumentPane.tsx +++ b/frontend/src/core/components/tools/compare/CompareDocumentPane.tsx @@ -1,56 +1,12 @@ import { Group, Loader, Stack, Text } from '@mantine/core'; -import { useMemo, useRef, useState } from 'react'; +import { useMemo, useRef, useState, useEffect } from 'react'; import type { PagePreview } from '@app/types/compare'; import type { TokenBoundingBox, CompareDocumentPaneProps } from '@app/types/compare'; +import { mergeConnectedRects, normalizeRotation, groupWordRects, computePageLayoutMetrics } from '@app/components/tools/compare/compare'; import CompareNavigationDropdown from '@app/components/tools/compare/CompareNavigationDropdown'; import { useIsMobile } from '@app/hooks/useIsMobile'; -const toRgba = (hexColor: string, alpha: number): string => { - const hex = hexColor.replace('#', ''); - if (hex.length !== 6) { - return hexColor; - } - const r = parseInt(hex.slice(0, 2), 16); - const g = parseInt(hex.slice(2, 4), 16); - const b = parseInt(hex.slice(4, 6), 16); - return `rgba(${r}, ${g}, ${b}, ${alpha})`; -}; - -// Merge overlapping or touching rects into larger non-overlapping blocks. -// This is more robust across rotations (vertical "lines" etc.) and prevents dark spots. -const mergeConnectedRects = (rects: TokenBoundingBox[]): TokenBoundingBox[] => { - if (rects.length === 0) return rects; - const EPS = 0.004; // small tolerance in normalized page coords - const sorted = rects.slice().sort((a, b) => (a.top !== b.top ? a.top - b.top : a.left - b.left)); - const merged: TokenBoundingBox[] = []; - const overlapsOrTouches = (a: TokenBoundingBox, b: TokenBoundingBox) => { - const aR = a.left + a.width; - const aB = a.top + a.height; - const bR = b.left + b.width; - const bB = b.top + b.height; - // Overlap or touch within EPS gap - return !(b.left > aR + EPS || bR < a.left - EPS || b.top > aB + EPS || bB < a.top - EPS); - }; - for (const r of sorted) { - let mergedIntoExisting = false; - for (let i = 0; i < merged.length; i += 1) { - const m = merged[i]; - if (overlapsOrTouches(m, r)) { - const left = Math.min(m.left, r.left); - const top = Math.min(m.top, r.top); - const right = Math.max(m.left + m.width, r.left + r.width); - const bottom = Math.max(m.top + m.height, r.top + r.height); - merged[i] = { left, top, width: Math.max(0, right - left), height: Math.max(0, bottom - top) }; - mergedIntoExisting = true; - break; - } - } - if (!mergedIntoExisting) { - merged.push({ ...r }); - } - } - return merged; -}; +// utilities moved to compare.ts const CompareDocumentPane = ({ pane, @@ -58,9 +14,6 @@ const CompareDocumentPane = ({ scrollRef, peerScrollRef, handleScrollSync, - beginPan, - continuePan, - endPan, handleWheelZoom, handleWheelOverscroll, onTouchStart, @@ -68,7 +21,6 @@ const CompareDocumentPane = ({ onTouchEnd, isPanMode, zoom, - pan, title, dropdownPlaceholder, changes, @@ -77,15 +29,11 @@ const CompareDocumentPane = ({ processingMessage, pages, pairedPages, - getRowHeightPx, wordHighlightMap, metaIndexToGroupId, documentLabel, pageLabel, altLabel, - pageInputValue, - onPageInputChange, - maxSharedPages, onVisiblePageChange, }: CompareDocumentPaneProps) => { const isMobileViewport = useIsMobile(); @@ -97,8 +45,7 @@ const CompareDocumentPane = ({ return map; }, [pairedPages]); - const HIGHLIGHT_COLOR = pane === 'base' ? '#ff6b6b' : '#51cf66'; // red for base (removals), green for comparison (additions) - const HIGHLIGHT_OPACITY = pane === 'base' ? 0.45 : 0.35; + const HIGHLIGHT_BG_VAR = pane === 'base' ? 'var(--spdf-compare-removed-bg)' : 'var(--spdf-compare-added-bg)'; const OFFSET_PIXELS = pane === 'base' ? 4 : 2; const cursorStyle = isPanMode && zoom > 1 ? 'grab' : 'auto'; const pagePanRef = useRef>(new Map()); @@ -108,6 +55,24 @@ const CompareDocumentPane = ({ const imageLoadedRef = useRef>(new Map()); const [, forceRerender] = useState(0); const visiblePageRafRef = useRef(null); + const lastReportedVisiblePageRef = useRef(null); + const pageNodesRef = useRef(null); + const groupedRectsByPage = useMemo(() => { + const out = new Map>(); + for (const p of pages) { + const rects = wordHighlightMap.get(p.pageNumber) ?? []; + out.set(p.pageNumber, groupWordRects(rects, metaIndexToGroupId, pane)); + } + return out; + }, [pages, wordHighlightMap, metaIndexToGroupId, pane]); + + // When zoom returns to 1 (reset), clear per-page pan state so content is centered again + useEffect(() => { + if (zoom <= 1) { + pagePanRef.current.clear(); + forceRerender(v => v + 1); + } + }, [zoom]); return (
@@ -135,14 +100,19 @@ const CompareDocumentPane = ({ onScroll={(event) => { handleScrollSync(event.currentTarget, peerScrollRef.current); // Notify parent about the currently visible page (throttled via rAF) - if (visiblePageRafRef.current != null) cancelAnimationFrame(visiblePageRafRef.current); + if (visiblePageRafRef.current != null) return; + if (!onVisiblePageChange || pages.length === 0) return; visiblePageRafRef.current = requestAnimationFrame(() => { const container = scrollRef.current; if (!container) return; const mid = container.scrollTop + container.clientHeight * 0.5; let bestPage = pages[0]?.pageNumber ?? 1; let bestDist = Number.POSITIVE_INFINITY; - const nodes = Array.from(container.querySelectorAll('.compare-diff-page')) as HTMLElement[]; + let nodes = pageNodesRef.current; + if (!nodes || nodes.length !== pages.length) { + nodes = Array.from(container.querySelectorAll('.compare-diff-page')) as HTMLElement[]; + pageNodesRef.current = nodes; + } for (const el of nodes) { const top = el.offsetTop; const height = el.clientHeight || 1; @@ -155,9 +125,11 @@ const CompareDocumentPane = ({ if (!Number.isNaN(pn)) bestPage = pn; } } - if (typeof onVisiblePageChange === 'function') { + if (typeof onVisiblePageChange === 'function' && bestPage !== lastReportedVisiblePageRef.current) { + lastReportedVisiblePageRef.current = bestPage; onVisiblePageChange(pane, bestPage); } + visiblePageRafRef.current = null; }); }} onMouseDown={undefined} @@ -181,45 +153,21 @@ const CompareDocumentPane = ({ {pages.map((page) => { const peerPage = pairedPageMap.get(page.pageNumber); - const targetHeight = peerPage ? Math.max(page.height, peerPage.height) : page.height; - const fit = targetHeight / page.height; - const rowHeightPx = getRowHeightPx(page.pageNumber); - const highlightOffset = OFFSET_PIXELS / page.height; - const rotationNorm = ((page.rotation ?? 0) % 360 + 360) % 360; - const isPortrait = rotationNorm === 0 || rotationNorm === 180; - const isLandscape = rotationNorm === 90 || rotationNorm === 270; - const isStackedPortrait = layout === 'stacked' && isPortrait; - const isStackedLandscape = layout === 'stacked' && isLandscape; const viewportWidth = typeof window !== 'undefined' ? window.innerWidth : 1200; - const containerW = scrollRef.current?.clientWidth ?? viewportWidth; - const stackedWidth = isMobileViewport - ? Math.max(320, Math.round(containerW)) - : Math.max(320, Math.round(viewportWidth * 0.5)); - const stackedHeight = Math.round(stackedWidth * 1.4142); + const metrics = computePageLayoutMetrics({ + page, + peerPage: peerPage ?? null, + layout, + isMobileViewport, + scrollRefWidth: scrollRef.current?.clientWidth ?? null, + viewportWidth, + zoom, + offsetPixels: OFFSET_PIXELS, + }); - const wordRects = wordHighlightMap.get(page.pageNumber) ?? []; - const groupedRects = new Map(); - for (const { rect, metaIndex } of wordRects) { - const id = metaIndexToGroupId.get(metaIndex) ?? `${pane}-token-${metaIndex}`; - const current = groupedRects.get(id) ?? []; - current.push(rect); - groupedRects.set(id, current); - } - const preloadMarginPx = Math.max(rowHeightPx * 5, 1200); // render several pages ahead to hide loading flashes + const { highlightOffset, baseWidth, baseHeight, containerWidth, containerHeight, innerScale } = metrics; - const baseWidth = isStackedPortrait - ? stackedWidth - : Math.round(page.width * fit); - const baseHeight = isStackedPortrait - ? stackedHeight - : Math.round(targetHeight); - const desiredWidth = Math.max(1, Math.round(baseWidth * Math.max(0.1, zoom))); - const desiredHeight = Math.max(1, Math.round(baseHeight * Math.max(0.1, zoom))); - const containerMaxW = scrollRef.current?.clientWidth ?? (typeof window !== 'undefined' ? window.innerWidth : desiredWidth); - const containerWidth = Math.min(desiredWidth, Math.max(120, containerMaxW)); - const containerHeight = Math.round(baseHeight * (containerWidth / baseWidth)); - const innerScale = Math.max(1, desiredWidth / containerWidth); - const currentPan = pagePanRef.current.get(page.pageNumber) || { x: 0, y: 0 }; + const groupedRects = groupedRectsByPage.get(page.pageNumber) ?? new Map(); return ( <> @@ -280,7 +228,7 @@ const CompareDocumentPane = ({
@@ -289,6 +237,7 @@ const CompareDocumentPane = ({ src={page.url ?? ''} alt={altLabel} loading="lazy" + decoding="async" className="compare-diff-page__image" onLoad={() => { if (!imageLoadedRef.current.get(page.pageNumber)) { @@ -305,7 +254,7 @@ const CompareDocumentPane = ({ )} {[...groupedRects.entries()].flatMap(([id, rects]) => mergeConnectedRects(rects).map((rect, index) => { - const rotation = ((page.rotation ?? 0) % 360 + 360) % 360; + const rotation = normalizeRotation(page.rotation); const verticalOffset = rotation === 180 ? -highlightOffset : highlightOffset; return ( ); diff --git a/frontend/src/core/components/tools/compare/CompareWorkbenchView.tsx b/frontend/src/core/components/tools/compare/CompareWorkbenchView.tsx index 1259a9dbd..5790b54ef 100644 --- a/frontend/src/core/components/tools/compare/CompareWorkbenchView.tsx +++ b/frontend/src/core/components/tools/compare/CompareWorkbenchView.tsx @@ -2,13 +2,19 @@ import { useEffect, useMemo, useRef, useState, useCallback } from 'react'; import { Loader, Stack } from '@mantine/core'; import { useTranslation } from 'react-i18next'; import { useIsMobile } from '@app/hooks/useIsMobile'; +import { + mapChangesForDropdown, + getFileFromSelection, + getStubFromSelection, + computeShowProgressBanner, + computeProgressPct, + computeCountsText, + computeMaxSharedPages, +} from '@app/components/tools/compare/compare'; import { CompareResultData, CompareWorkbenchData, - CompareChangeOption, } from '@app/types/compare'; -import type { FileId } from '@app/types/file'; -import type { StirlingFile } from '@app/types/fileContext'; import { useFileContext } from '@app/contexts/file/fileHooks'; import { useRightRailButtons } from '@app/hooks/useRightRailButtons'; import CompareDocumentPane from '@app/components/tools/compare/CompareDocumentPane'; @@ -25,26 +31,7 @@ interface CompareWorkbenchViewProps { data: CompareWorkbenchData | null; } -const getFileFromSelection = ( - explicit: StirlingFile | null | undefined, - fileId: FileId | null, - selectors: ReturnType['selectors'], -) => { - if (explicit) return explicit; - if (!fileId) return null; - return selectors.getFile(fileId) ?? null; -}; - -const getStubFromSelection = ( - fileId: FileId | null, - selectors: ReturnType['selectors'], -) => { - if (!fileId) return null; - return selectors.getStirlingFileStub(fileId) ?? null; -}; - -const mapChangesForDropdown = (changes: CompareChangeOption[]) => - changes.map(({ value, label, pageNumber }) => ({ value, label, pageNumber })); +// helpers moved to compare.ts const CompareWorkbenchView = ({ data }: CompareWorkbenchViewProps) => { const { t } = useTranslation(); @@ -58,8 +45,8 @@ const CompareWorkbenchView = ({ data }: CompareWorkbenchViewProps) => { const baseFile = getFileFromSelection(data?.baseLocalFile, baseFileId, selectors); const comparisonFile = getFileFromSelection(data?.comparisonLocalFile, comparisonFileId, selectors); - const baseStub = getStubFromSelection(baseFileId, selectors); - const comparisonStub = getStubFromSelection(comparisonFileId, selectors); + const baseStub = getStubFromSelection(baseFileId, selectors) as any; + const comparisonStub = getStubFromSelection(comparisonFileId, selectors) as any; const processedAt = result?.totals.processedAt ?? null; @@ -81,9 +68,6 @@ const CompareWorkbenchView = ({ data }: CompareWorkbenchViewProps) => { baseScrollRef, comparisonScrollRef, handleScrollSync, - beginPan, - continuePan, - endPan, handleWheelZoom, handleWheelOverscroll, onTouchStart, @@ -92,11 +76,10 @@ const CompareWorkbenchView = ({ data }: CompareWorkbenchViewProps) => { isPanMode, setIsPanMode, baseZoom, - setBaseZoom, + setBaseZoom, comparisonZoom, setComparisonZoom, - basePan, - comparisonPan, + setPanToTopLeft, centerPanForZoom, clampPanForZoom, clearScrollLinkDelta, @@ -149,6 +132,7 @@ const CompareWorkbenchView = ({ data }: CompareWorkbenchViewProps) => { comparisonZoom, setBaseZoom, setComparisonZoom, + setPanToTopLeft, centerPanForZoom, clampPanForZoom, clearScrollLinkDelta, @@ -156,6 +140,8 @@ const CompareWorkbenchView = ({ data }: CompareWorkbenchViewProps) => { isScrollLinked, setIsScrollLinked, zoomLimits, + baseScrollRef, + comparisonScrollRef, }); useRightRailButtons(rightRailButtons); @@ -163,17 +149,11 @@ const CompareWorkbenchView = ({ data }: CompareWorkbenchViewProps) => { // Rendering progress toast for very large PDFs const LARGE_PAGE_THRESHOLD = 400; // show banner when one or both exceed threshold const totalsKnown = (baseTotal ?? 0) > 0 && (compTotal ?? 0) > 0; - const showProgressBanner = useMemo(() => { - if (!totalsKnown) return false; // avoid premature 100% before totals are known - const totals = [baseTotal!, compTotal!]; - return Math.max(...totals) >= LARGE_PAGE_THRESHOLD && (baseLoading || comparisonLoading); - }, [totalsKnown, baseTotal, compTotal, baseLoading, comparisonLoading]); + const showProgressBanner = useMemo(() => ( + computeShowProgressBanner(totalsKnown, baseTotal, compTotal, baseLoading, comparisonLoading, LARGE_PAGE_THRESHOLD) + ), [totalsKnown, baseTotal, compTotal, baseLoading, comparisonLoading]); - const totalCombined = totalsKnown ? (baseTotal! + compTotal!) : 0; - const renderedCombined = baseRendered + compRendered; - const progressPct = totalsKnown && totalCombined > 0 - ? Math.min(100, Math.round((renderedCombined / totalCombined) * 100)) - : 0; + const progressPct = computeProgressPct(totalsKnown, baseTotal, compTotal, baseRendered, compRendered); const progressToastIdRef = useRef(null); const completionTimerRef = useRef(null); @@ -208,7 +188,14 @@ const CompareWorkbenchView = ({ data }: CompareWorkbenchViewProps) => { return; } - const countsText = `${baseRendered}/${baseTotal || basePages.length} • ${compRendered}/${compTotal || comparisonPages.length}`; + const countsText = computeCountsText( + baseRendered, + baseTotal, + basePages.length, + compRendered, + compTotal, + comparisonPages.length, + ); if (!allDone) { // Create toast if missing if (!progressToastIdRef.current) { @@ -266,13 +253,9 @@ const CompareWorkbenchView = ({ data }: CompareWorkbenchViewProps) => { }, [showProgressBanner, allDone, progressPct, baseRendered, compRendered, baseTotal, compTotal, basePages.length, comparisonPages.length, t]); // Shared page navigation state/input - const maxSharedPages = useMemo(() => { - const baseMax = baseTotal || basePages.length || 0; - const compMax = compTotal || comparisonPages.length || 0; - const minKnown = Math.min(baseMax || Infinity, compMax || Infinity); - if (!Number.isFinite(minKnown)) return 0; - return Math.max(0, minKnown); - }, [baseTotal, compTotal, basePages.length, comparisonPages.length]); + const maxSharedPages = useMemo(() => ( + computeMaxSharedPages(baseTotal, compTotal, basePages.length, comparisonPages.length) + ), [baseTotal, compTotal, basePages.length, comparisonPages.length]); const [pageInputValue, setPageInputValue] = useState('1'); const typingTimerRef = useRef(null); @@ -343,14 +326,6 @@ const CompareWorkbenchView = ({ data }: CompareWorkbenchViewProps) => { }, 300); }, [maxSharedPages, scrollBothToPage]); - const handleVisiblePageChange = useCallback((pane: 'base' | 'comparison', page: number) => { - // Reflect scroll position in the input, but do not trigger navigation - if (isTypingRef.current) return; // ignore during typing debounce window - if (page <= 0) return; - const display = String(Math.min(maxSharedPages || page, page)); - setPageInputValue(display); - }, [maxSharedPages]); - return ( @@ -364,9 +339,6 @@ const CompareWorkbenchView = ({ data }: CompareWorkbenchViewProps) => { scrollRef={baseScrollRef} peerScrollRef={comparisonScrollRef} handleScrollSync={handleScrollSync} - beginPan={beginPan} - continuePan={continuePan} - endPan={endPan} handleWheelZoom={handleWheelZoom} handleWheelOverscroll={handleWheelOverscroll} onTouchStart={onTouchStart} @@ -374,10 +346,9 @@ const CompareWorkbenchView = ({ data }: CompareWorkbenchViewProps) => { onTouchEnd={onTouchEnd} isPanMode={isPanMode} zoom={baseZoom} - pan={basePan} title={baseTitle} dropdownPlaceholder={baseDropdownPlaceholder} - changes={mapChangesForDropdown(baseWordChanges)} + changes={mapChangesForDropdown(baseWordChanges)} onNavigateChange={(value, pageNumber) => handleChangeNavigation(value, 'base', pageNumber)} isLoading={isOperationLoading || baseLoading} processingMessage={processingMessage} @@ -392,7 +363,6 @@ const CompareWorkbenchView = ({ data }: CompareWorkbenchViewProps) => { pageInputValue={pageInputValue} onPageInputChange={handleTypingChange} maxSharedPages={maxSharedPages} - onVisiblePageChange={handleVisiblePageChange} /> { scrollRef={comparisonScrollRef} peerScrollRef={baseScrollRef} handleScrollSync={handleScrollSync} - beginPan={beginPan} - continuePan={continuePan} - endPan={endPan} handleWheelZoom={handleWheelZoom} handleWheelOverscroll={handleWheelOverscroll} onTouchStart={onTouchStart} @@ -410,7 +377,6 @@ const CompareWorkbenchView = ({ data }: CompareWorkbenchViewProps) => { onTouchEnd={onTouchEnd} isPanMode={isPanMode} zoom={comparisonZoom} - pan={comparisonPan} title={comparisonTitle} dropdownPlaceholder={comparisonDropdownPlaceholder} changes={mapChangesForDropdown(comparisonWordChanges)} @@ -428,7 +394,6 @@ const CompareWorkbenchView = ({ data }: CompareWorkbenchViewProps) => { pageInputValue={pageInputValue} onPageInputChange={handleTypingChange} maxSharedPages={maxSharedPages} - onVisiblePageChange={handleVisiblePageChange} />
diff --git a/frontend/src/core/components/tools/compare/compare.ts b/frontend/src/core/components/tools/compare/compare.ts new file mode 100644 index 000000000..8dd7a0d85 --- /dev/null +++ b/frontend/src/core/components/tools/compare/compare.ts @@ -0,0 +1,209 @@ +import type { TokenBoundingBox, WordHighlightEntry } from '@app/types/compare'; +import type { FileId } from '@app/types/file'; +import type { StirlingFile } from '@app/types/fileContext'; +import type { PagePreview } from '@app/types/compare'; + +/** Convert hex color (#rrggbb) to rgba() string with alpha; falls back to input if invalid. */ +export const toRgba = (hexColor: string, alpha: number): string => { + const hex = hexColor.replace('#', ''); + if (hex.length !== 6) return hexColor; + const r = parseInt(hex.slice(0, 2), 16); + const g = parseInt(hex.slice(2, 4), 16); + const b = parseInt(hex.slice(4, 6), 16); + return `rgba(${r}, ${g}, ${b}, ${alpha})`; +}; + +/** Normalize rotation to [0, 360). */ +export const normalizeRotation = (deg: number | undefined | null): number => { + const n = ((deg ?? 0) % 360 + 360) % 360; + return n; +}; + +/** + * Merge overlapping or touching rectangles into larger non-overlapping blocks. + * Robust across rotations (vertical groups) and prevents dark spots from overlaps. + */ +export const mergeConnectedRects = (rects: TokenBoundingBox[]): TokenBoundingBox[] => { + if (rects.length === 0) return rects; + const EPS = 0.004; // small tolerance in normalized page coords + const sorted = rects + .slice() + .sort((a, b) => (a.top !== b.top ? a.top - b.top : a.left - b.left)); + const merged: TokenBoundingBox[] = []; + + const overlapsOrTouches = (a: TokenBoundingBox, b: TokenBoundingBox) => { + const aR = a.left + a.width; + const aB = a.top + a.height; + const bR = b.left + b.width; + const bB = b.top + b.height; + return !(b.left > aR + EPS || bR < a.left - EPS || b.top > aB + EPS || bB < a.top - EPS); + }; + + for (const r of sorted) { + let mergedIntoExisting = false; + for (let i = 0; i < merged.length; i += 1) { + const m = merged[i]; + if (overlapsOrTouches(m, r)) { + const left = Math.min(m.left, r.left); + const top = Math.min(m.top, r.top); + const right = Math.max(m.left + m.width, r.left + r.width); + const bottom = Math.max(m.top + m.height, r.top + r.height); + merged[i] = { + left, + top, + width: Math.max(0, right - left), + height: Math.max(0, bottom - top), + }; + mergedIntoExisting = true; + break; + } + } + if (!mergedIntoExisting) merged.push({ ...r }); + } + return merged; +}; + +/** Group word rectangles by change id using metaIndexToGroupId. */ +export const groupWordRects = ( + wordRects: WordHighlightEntry[], + metaIndexToGroupId: Map, + pane: 'base' | 'comparison' +): Map => { + const groupedRects = new Map(); + for (const { rect, metaIndex } of wordRects) { + const id = metaIndexToGroupId.get(metaIndex) ?? `${pane}-token-${metaIndex}`; + const current = groupedRects.get(id) ?? []; + current.push(rect); + groupedRects.set(id, current); + } + return groupedRects; +}; + +/** Compute derived layout metrics for a page render, given environment and zoom. */ +export const computePageLayoutMetrics = (args: { + page: PagePreview; + peerPage?: PagePreview | null; + layout: 'side-by-side' | 'stacked'; + isMobileViewport: boolean; + scrollRefWidth: number | null; + viewportWidth: number; + zoom: number; + offsetPixels: number; // highlight offset in px relative to original page height +}) => { + const { page, peerPage, layout, isMobileViewport, scrollRefWidth, viewportWidth, zoom, offsetPixels } = args; + const targetHeight = peerPage ? Math.max(page.height, peerPage.height) : page.height; + const fit = targetHeight / page.height; + const highlightOffset = offsetPixels / page.height; + const rotationNorm = normalizeRotation(page.rotation); + const isPortrait = rotationNorm === 0 || rotationNorm === 180; + const isStackedPortrait = layout === 'stacked' && isPortrait; + + const containerW = scrollRefWidth ?? viewportWidth; + const stackedWidth = isMobileViewport + ? Math.max(320, Math.round(containerW)) + : Math.max(320, Math.round(viewportWidth * 0.5)); + const stackedHeight = Math.round(stackedWidth * 1.4142); + + const baseWidth = isStackedPortrait ? stackedWidth : Math.round(page.width * fit); + const baseHeight = isStackedPortrait ? stackedHeight : Math.round(targetHeight); + const containerMaxW = scrollRefWidth ?? viewportWidth; + const containerWidth = Math.min(baseWidth, Math.max(120, containerMaxW)); + const containerHeight = Math.round(baseHeight * (containerWidth / baseWidth)); + const innerScale = Math.max(1, zoom); + + return { + targetHeight, + fit, + highlightOffset, + rotationNorm, + isPortrait, + isStackedPortrait, + baseWidth, + baseHeight, + containerMaxW, + containerWidth, + containerHeight, + innerScale, + }; +}; + +/** Map changes to dropdown options tuple. */ +export const mapChangesForDropdown = ( + changes: Array<{ value: string; label: string; pageNumber: number }> +) => changes.map(({ value, label, pageNumber }) => ({ value, label, pageNumber })); + +/** File selection helpers */ +export const getFileFromSelection = ( + explicit: StirlingFile | null | undefined, + fileId: FileId | null, + selectors: { getFile: (id: FileId) => StirlingFile | undefined | null } +): StirlingFile | null => { + if (explicit) return explicit; + if (!fileId) return null; + return (selectors.getFile(fileId) as StirlingFile | undefined | null) ?? null; +}; + +export const getStubFromSelection = ( + fileId: FileId | null, + selectors: { getStirlingFileStub: (id: FileId) => unknown } +): unknown | null => { + if (!fileId) return null; + const stub = selectors.getStirlingFileStub(fileId); + return stub ?? null; +}; + +/** Progress banner computations */ +export const computeShowProgressBanner = ( + totalsKnown: boolean, + baseTotal: number | null | undefined, + compTotal: number | null | undefined, + baseLoading: boolean, + compLoading: boolean, + threshold: number = 400 +): boolean => { + if (!totalsKnown) return false; + const totals = [baseTotal ?? 0, compTotal ?? 0]; + return Math.max(...totals) >= threshold && (baseLoading || compLoading); +}; + +export const computeProgressPct = ( + totalsKnown: boolean, + baseTotal: number | null | undefined, + compTotal: number | null | undefined, + baseRendered: number, + compRendered: number +): number => { + const totalCombined = totalsKnown ? ((baseTotal ?? 0) + (compTotal ?? 0)) : 0; + const renderedCombined = baseRendered + compRendered; + return totalsKnown && totalCombined > 0 + ? Math.min(100, Math.round((renderedCombined / totalCombined) * 100)) + : 0; +}; + +export const computeCountsText = ( + baseRendered: number, + baseTotal: number | null | undefined, + baseLength: number, + compRendered: number, + compTotal: number | null | undefined, + compLength: number +): string => { + const baseTotalShown = baseTotal || baseLength; + const compTotalShown = compTotal || compLength; + return `${baseRendered}/${baseTotalShown} • ${compRendered}/${compTotalShown}`; +}; + +export const computeMaxSharedPages = ( + baseTotal: number | null | undefined, + compTotal: number | null | undefined, + baseLen: number, + compLen: number +): number => { + const baseMax = baseTotal || baseLen || 0; + const compMax = compTotal || compLen || 0; + const minKnown = Math.min(baseMax || Infinity, compMax || Infinity); + if (!Number.isFinite(minKnown)) return 0; + return Math.max(0, minKnown); +}; + + diff --git a/frontend/src/core/components/tools/compare/compareView.css b/frontend/src/core/components/tools/compare/compareView.css index 7fbe4c3ce..78e562859 100644 --- a/frontend/src/core/components/tools/compare/compareView.css +++ b/frontend/src/core/components/tools/compare/compareView.css @@ -44,6 +44,7 @@ flex: 1; min-height: 0; overflow: auto; + overscroll-behavior: contain; } .compare-pane__content { @@ -99,8 +100,8 @@ /* Dropdown badge-like style - only style the dropdowns, not titles */ .compare-changes-select { - background: rgba(255, 59, 48, 0.15) !important; - color: #b91c1c !important; + background: var(--spdf-compare-removed-badge-bg) !important; + color: var(--spdf-compare-removed-badge-fg) !important; border: none !important; border-radius: 8px !important; font-weight: 500 !important; @@ -126,8 +127,8 @@ } .compare-changes-select--comparison { - background: rgba(52, 199, 89, 0.18) !important; - color: #1b5e20 !important; + background: var(--spdf-compare-added-badge-bg) !important; + color: var(--spdf-compare-added-badge-fg) !important; border: none !important; border-radius: 8px !important; font-weight: 500 !important; @@ -227,11 +228,11 @@ } .compare-changes-select .mantine-Combobox-option:hover { - background-color: rgba(255, 59, 48, 0.1) !important; + background-color: var(--spdf-compare-removed-badge-bg) !important; } .compare-changes-select--comparison .mantine-Combobox-option:hover { - background-color: rgba(52, 199, 89, 0.15) !important; + background-color: var(--spdf-compare-added-badge-bg) !important; } /* Style the search input */ @@ -374,6 +375,7 @@ max-width: 100%; background-color: #fff; /* ensure stable white backing during load */ border: 1px solid var(--border-subtle); + will-change: transform; } .compare-diff-page__image { @@ -470,10 +472,10 @@ padding: 0.05rem 0.15rem; } .compare-inline--removed { - background-color: rgba(255, 59, 48, 0.25); + background-color: var(--spdf-compare-inline-removed-bg); } .compare-inline--added { - background-color: rgba(52, 199, 89, 0.25); + background-color: var(--spdf-compare-inline-added-bg); } .compare-pane-header { diff --git a/frontend/src/core/components/tools/compare/hooks/useComparePanZoom.ts b/frontend/src/core/components/tools/compare/hooks/useComparePanZoom.ts index a059d4ba3..e86592cb6 100644 --- a/frontend/src/core/components/tools/compare/hooks/useComparePanZoom.ts +++ b/frontend/src/core/components/tools/compare/hooks/useComparePanZoom.ts @@ -87,6 +87,10 @@ export const useComparePanZoom = ({ [basePages, comparisonPages] ); + // rAF-coalesced follower scroll writes + const syncRafRef = useRef<{ base: number | null; comparison: number | null }>({ base: null, comparison: null }); + const desiredTopRef = useRef<{ base: number | null; comparison: number | null }>({ base: null, comparison: null }); + const canonicalLayout = useMemo(() => { const baseMap = new Map(); const compMap = new Map(); @@ -312,6 +316,14 @@ export const useComparePanZoom = ({ [getPanBounds] ); + const setPanToTopLeft = useCallback((pane: Pane) => { + if (pane === 'base') { + setBasePan({ x: 0, y: 0 }); + } else { + setComparisonPan({ x: 0, y: 0 }); + } + }, []); + const clampPanForZoom = useCallback( (pane: Pane, zoomValue: number) => { const bounds = getPanBounds(pane, zoomValue); @@ -367,11 +379,24 @@ export const useComparePanZoom = ({ return; } - isSyncingRef.current = true; - target.scrollTop = desiredTop; - requestAnimationFrame(() => { - isSyncingRef.current = false; - }); + const targetIsBase = target === baseScrollRef.current; + const key = targetIsBase ? 'base' : 'comparison'; + + desiredTopRef.current[key] = desiredTop; + if (syncRafRef.current[key] == null) { + syncRafRef.current[key] = requestAnimationFrame(() => { + const el = targetIsBase ? baseScrollRef.current : comparisonScrollRef.current; + const top = desiredTopRef.current[key] ?? 0; + if (el) { + isSyncingRef.current = true; + el.scrollTop = top; + } + syncRafRef.current[key] = null; + requestAnimationFrame(() => { + isSyncingRef.current = false; + }); + }); + } }, [isScrollLinked, mapScrollTopBetweenPanes] ); @@ -394,6 +419,8 @@ export const useComparePanZoom = ({ // Heuristic: clear the flag shortly after scroll events settle let timeout: number | null = null; const onScroll = () => { + // Ignore programmatic scrolls to avoid feedback loops and unnecessary syncing work + if (isSyncingRef.current) return; onStart(); if (timeout != null) window.clearTimeout(timeout); timeout = window.setTimeout(onEnd, 120); @@ -872,6 +899,7 @@ export const useComparePanZoom = ({ setComparisonZoom, basePan, comparisonPan, + setPanToTopLeft, centerPanForZoom, clampPanForZoom, handleScrollSync, diff --git a/frontend/src/core/components/tools/compare/hooks/useCompareRightRailButtons.tsx b/frontend/src/core/components/tools/compare/hooks/useCompareRightRailButtons.tsx index 200177594..f87e43ae1 100644 --- a/frontend/src/core/components/tools/compare/hooks/useCompareRightRailButtons.tsx +++ b/frontend/src/core/components/tools/compare/hooks/useCompareRightRailButtons.tsx @@ -1,4 +1,5 @@ import { useMemo } from 'react'; +import type React from 'react'; import { useTranslation } from 'react-i18next'; import LocalIcon from '@app/components/shared/LocalIcon'; import { alert } from '@app/components/toast'; @@ -17,6 +18,7 @@ export interface UseCompareRightRailButtonsOptions { comparisonZoom: number; setBaseZoom: (value: number) => void; setComparisonZoom: (value: number) => void; + setPanToTopLeft: (pane: Pane) => void; centerPanForZoom: (pane: Pane, zoom: number) => void; clampPanForZoom: (pane: Pane, zoom: number) => void; clearScrollLinkDelta: () => void; @@ -24,6 +26,8 @@ export interface UseCompareRightRailButtonsOptions { isScrollLinked: boolean; setIsScrollLinked: (value: boolean) => void; zoomLimits: { min: number; max: number; step: number }; + baseScrollRef?: React.RefObject; + comparisonScrollRef?: React.RefObject; } export const useCompareRightRailButtons = ({ @@ -35,6 +39,7 @@ export const useCompareRightRailButtons = ({ comparisonZoom, setBaseZoom, setComparisonZoom, + setPanToTopLeft, centerPanForZoom, clampPanForZoom, clearScrollLinkDelta, @@ -42,6 +47,8 @@ export const useCompareRightRailButtons = ({ isScrollLinked, setIsScrollLinked, zoomLimits, + baseScrollRef, + comparisonScrollRef, }: UseCompareRightRailButtonsOptions): RightRailButtonWithAction[] => { const { t } = useTranslation(); const isMobile = useIsMobile(); @@ -111,9 +118,16 @@ export const useCompareRightRailButtons = ({ onClick: () => { setBaseZoom(1); setComparisonZoom(1); - centerPanForZoom('base', 1); - centerPanForZoom('comparison', 1); + setPanToTopLeft('base'); + setPanToTopLeft('comparison'); clearScrollLinkDelta(); + // Reset scrollTop for both panes to realign view + if (baseScrollRef?.current) { + baseScrollRef.current.scrollTop = 0; + } + if (comparisonScrollRef?.current) { + comparisonScrollRef.current.scrollTop = 0; + } }, }, { @@ -163,6 +177,7 @@ export const useCompareRightRailButtons = ({ setComparisonZoom, centerPanForZoom, clampPanForZoom, + setPanToTopLeft, clearScrollLinkDelta, captureScrollLinkDelta, isScrollLinked, diff --git a/frontend/src/core/styles/theme.css b/frontend/src/core/styles/theme.css index 934d94600..75a05a34e 100644 --- a/frontend/src/core/styles/theme.css +++ b/frontend/src/core/styles/theme.css @@ -1,3 +1,19 @@ +:root { + /* Compare highlight colors (same in light/dark) */ + --spdf-compare-removed-bg: rgba(255, 107, 107, 0.45); /* #ff6b6b @ 0.45 */ + --spdf-compare-added-bg: rgba(81, 207, 102, 0.35); /* #51cf66 @ 0.35 */ + + /* Badge colors for dropdowns */ + --spdf-compare-removed-badge-bg: rgba(255, 59, 48, 0.15); + --spdf-compare-removed-badge-fg: #b91c1c; + --spdf-compare-added-badge-bg: rgba(52, 199, 89, 0.18); + --spdf-compare-added-badge-fg: #1b5e20; + + /* Inline highlights in summary */ + --spdf-compare-inline-removed-bg: rgba(255, 59, 48, 0.25); + --spdf-compare-inline-added-bg: rgba(52, 199, 89, 0.25); +} + /* CSS variables for Tailwind + Mantine integration */ :root { diff --git a/frontend/src/core/tools/Compare.tsx b/frontend/src/core/tools/Compare.tsx index dc9c86bd6..41d9bd801 100644 --- a/frontend/src/core/tools/Compare.tsx +++ b/frontend/src/core/tools/Compare.tsx @@ -500,10 +500,10 @@ const Compare = (props: BaseToolProps) => { variant="filled" onClick={() => { setClearConfirmOpen(false); - try { base.operation.cancelOperation(); } catch {} - try { base.operation.resetResults(); } catch {} + try { base.operation.cancelOperation(); } catch {console.error('Failed to cancel operation');} + try { base.operation.resetResults(); } catch {console.error('Failed to reset results');} base.params.setParameters(prev => ({ ...prev, baseFileId: null, comparisonFileId: null })); - try { fileActions.clearSelections(); } catch {} + try { fileActions.clearSelections(); } catch {console.error('Failed to clear selections');} clearCustomWorkbenchViewData(CUSTOM_VIEW_ID); navigationActions.setWorkbench(getDefaultWorkbench()); }} diff --git a/frontend/src/core/types/compare.ts b/frontend/src/core/types/compare.ts index a66422ea6..b06e852a6 100644 --- a/frontend/src/core/types/compare.ts +++ b/frontend/src/core/types/compare.ts @@ -143,9 +143,6 @@ export interface CompareDocumentPaneProps { scrollRef: React.RefObject; peerScrollRef: React.RefObject; handleScrollSync: (source: HTMLDivElement | null, target: HTMLDivElement | null) => void; - beginPan: (pane: 'base' | 'comparison', event: React.MouseEvent) => void; - continuePan: (event: React.MouseEvent) => void; - endPan: () => void; handleWheelZoom: (pane: 'base' | 'comparison', event: React.WheelEvent) => void; handleWheelOverscroll: (pane: 'base' | 'comparison', event: React.WheelEvent) => void; onTouchStart: (pane: 'base' | 'comparison', event: React.TouchEvent) => void; @@ -153,7 +150,6 @@ export interface CompareDocumentPaneProps { onTouchEnd: (event: React.TouchEvent) => void; isPanMode: boolean; zoom: number; - pan?: { x: number; y: number }; title: string; dropdownPlaceholder?: React.ReactNode; changes: Array<{ value: string; label: string; pageNumber?: number }>; @@ -261,6 +257,7 @@ export interface UseComparePanZoomReturn { setComparisonZoom: (value: number) => void; basePan: PanState; comparisonPan: PanState; + setPanToTopLeft: (pane: ComparePane) => void; centerPanForZoom: (pane: ComparePane, zoom: number) => void; clampPanForZoom: (pane: ComparePane, zoom: number) => void; handleScrollSync: (source: HTMLDivElement | null, target: HTMLDivElement | null) => void;