From 653d3406683e2c71f8f7dd0e3aa3338085468557 Mon Sep 17 00:00:00 2001 From: David Nguyen Date: Wed, 25 Feb 2026 17:30:20 +1100 Subject: [PATCH] fix: virtualisation issues --- .../embed/authoring/configure-fields-view.tsx | 6 +- .../embed-direct-template-client-page.tsx | 6 +- .../embed/embed-document-fields.tsx | 4 +- .../embed/embed-document-signing-page-v1.tsx | 6 +- .../multi-sign-document-signing-view.tsx | 6 +- .../direct-template-signing-form.tsx | 6 +- .../document-signing-page-view-v1.tsx | 6 +- .../envelope-signing-complete-dialog.tsx | 8 +++ .../client-only/hooks/use-element-bounds.ts | 2 +- .../hooks/use-field-page-coords.ts | 43 ++++++++++++--- .../client-only/hooks/use-is-page-in-dom.ts | 34 ++++++++++++ packages/lib/utils/fields.ts | 35 +++++++++--- .../document/document-read-only-fields.tsx | 6 +- packages/ui/components/field/field.tsx | 24 +++++++- .../pdf-viewer/envelope-pdf-viewer.tsx | 6 +- .../ui/components/pdf-viewer/pdf-viewer.tsx | 6 +- .../pdf-viewer/use-scroll-to-page.ts | 46 ++++++++++++++++ .../virtual-list/use-virtual-list.ts | 55 +++++++++++++++++++ .../primitives/document-flow/field-item.tsx | 13 ++++- 19 files changed, 259 insertions(+), 59 deletions(-) create mode 100644 packages/lib/client-only/hooks/use-is-page-in-dom.ts create mode 100644 packages/ui/components/pdf-viewer/use-scroll-to-page.ts diff --git a/apps/remix/app/components/embed/authoring/configure-fields-view.tsx b/apps/remix/app/components/embed/authoring/configure-fields-view.tsx index 4fe96a850..e642b4539 100644 --- a/apps/remix/app/components/embed/authoring/configure-fields-view.tsx +++ b/apps/remix/app/components/embed/authoring/configure-fields-view.tsx @@ -178,8 +178,6 @@ export const ConfigureFieldsView = ({ name: 'fields', }); - const highestPageNumber = Math.max(...localFields.map((field) => field.pageNumber)); - const onFieldCopy = useCallback( (event?: KeyboardEvent | null, options?: { duplicate?: boolean; duplicateAll?: boolean }) => { const { duplicate = false, duplicateAll = false } = options ?? {}; @@ -556,9 +554,7 @@ export const ConfigureFieldsView = ({ scrollParentRef="window" /> - + {localFields.map((field, index) => { const recipientIndex = recipients.findIndex((r) => r.id === field.recipientId); diff --git a/apps/remix/app/components/embed/embed-direct-template-client-page.tsx b/apps/remix/app/components/embed/embed-direct-template-client-page.tsx index e487eb0bb..88b7aca2c 100644 --- a/apps/remix/app/components/embed/embed-direct-template-client-page.tsx +++ b/apps/remix/app/components/embed/embed-direct-template-client-page.tsx @@ -101,8 +101,6 @@ export const EmbedDirectTemplateClientPage = ({ localFields.filter((field) => field.inserted), ]; - const highestPendingPageNumber = Math.max(...pendingFields.map((field) => field.page)); - const hasSignatureField = localFields.some((field) => isSignatureFieldType(field.type)); const signatureValid = !hasSignatureField || (signature && signature.trim() !== ''); @@ -479,9 +477,7 @@ export const EmbedDirectTemplateClientPage = ({ - + {showPendingFieldTooltip && pendingFields.length > 0 && ( Click to insert field diff --git a/apps/remix/app/components/embed/embed-document-fields.tsx b/apps/remix/app/components/embed/embed-document-fields.tsx index 5c94f4127..dc1115226 100644 --- a/apps/remix/app/components/embed/embed-document-fields.tsx +++ b/apps/remix/app/components/embed/embed-document-fields.tsx @@ -50,10 +50,8 @@ export const EmbedDocumentFields = ({ onSignField, onUnsignField, }: EmbedDocumentFieldsProps) => { - const highestPageNumber = Math.max(...fields.map((field) => field.page)); - return ( - + {fields.map((field) => match(field.type) .with(FieldType.SIGNATURE, () => ( diff --git a/apps/remix/app/components/embed/embed-document-signing-page-v1.tsx b/apps/remix/app/components/embed/embed-document-signing-page-v1.tsx index 0ee9a8712..c5098ddb8 100644 --- a/apps/remix/app/components/embed/embed-document-signing-page-v1.tsx +++ b/apps/remix/app/components/embed/embed-document-signing-page-v1.tsx @@ -106,8 +106,6 @@ export const EmbedSignDocumentV1ClientPage = ({ fields.filter((field) => field.inserted), ]; - const highestPendingPageNumber = Math.max(...pendingFields.map((field) => field.page)); - const { mutateAsync: completeDocumentWithToken, isPending: isSubmitting } = trpc.recipient.completeDocumentWithToken.useMutation(); @@ -492,9 +490,7 @@ export const EmbedSignDocumentV1ClientPage = ({ - + {showPendingFieldTooltip && pendingFields.length > 0 && ( Click to insert field diff --git a/apps/remix/app/components/embed/multisign/multi-sign-document-signing-view.tsx b/apps/remix/app/components/embed/multisign/multi-sign-document-signing-view.tsx index 39087b77d..a2b64b338 100644 --- a/apps/remix/app/components/embed/multisign/multi-sign-document-signing-view.tsx +++ b/apps/remix/app/components/embed/multisign/multi-sign-document-signing-view.tsx @@ -95,8 +95,6 @@ export const MultiSignDocumentSigningView = ({ [], ]; - const highestPendingPageNumber = Math.max(...pendingFields.map((field) => field.page)); - const uninsertedFields = document?.fields.filter((field) => !field.inserted) ?? []; const onSignField = async (payload: TSignFieldWithTokenMutationSchema) => { @@ -370,9 +368,7 @@ export const MultiSignDocumentSigningView = ({ {hasDocumentLoaded && ( - + {showPendingFieldTooltip && pendingFields.length > 0 && ( field.page)); - const fieldsRequiringValidation = useMemo(() => { return localFields.filter((field) => isFieldUnsignedAndRequired(field)); }, [localFields]); @@ -250,9 +248,7 @@ export const DirectTemplateSigningForm = ({ - + {validateUninsertedFields && uninsertedFields[0] && ( Click to insert field diff --git a/apps/remix/app/components/general/document-signing/document-signing-page-view-v1.tsx b/apps/remix/app/components/general/document-signing/document-signing-page-view-v1.tsx index a39253ef8..4fb307554 100644 --- a/apps/remix/app/components/general/document-signing/document-signing-page-view-v1.tsx +++ b/apps/remix/app/components/general/document-signing/document-signing-page-view-v1.tsx @@ -162,8 +162,6 @@ export const DocumentSigningPageViewV1 = ({ : undefined; }, [document.documentMeta?.signingOrder, allRecipients, recipient.id]); - const highestPageNumber = Math.max(...fields.map((field) => field.page)); - const pendingFields = fieldsRequiringValidation.filter((field) => !field.inserted); const hasPendingFields = pendingFields.length > 0; @@ -401,9 +399,7 @@ export const DocumentSigningPageViewV1 = ({ )} - + {fields .filter( (field) => diff --git a/apps/remix/app/components/general/envelope-signing/envelope-signing-complete-dialog.tsx b/apps/remix/app/components/general/envelope-signing/envelope-signing-complete-dialog.tsx index ca1a80729..aefafea4c 100644 --- a/apps/remix/app/components/general/envelope-signing/envelope-signing-complete-dialog.tsx +++ b/apps/remix/app/components/general/envelope-signing/envelope-signing-complete-dialog.tsx @@ -71,6 +71,14 @@ export const EnvelopeSignerCompleteDialog = () => { if (fieldTooltip) { fieldTooltip.scrollIntoView({ behavior: 'smooth', block: 'center' }); + } else { + // Tooltip not in DOM (page virtualized away) — signal the PDF viewer + // to scroll to the correct page via the data attribute. + const pdfContent = document.querySelector('[data-pdf-content]'); + + if (pdfContent) { + pdfContent.setAttribute('data-scroll-to-page', String(nextField.page)); + } } }, isEnvelopeItemSwitch ? 150 : 50, diff --git a/packages/lib/client-only/hooks/use-element-bounds.ts b/packages/lib/client-only/hooks/use-element-bounds.ts index 560accca6..e25f5ba86 100644 --- a/packages/lib/client-only/hooks/use-element-bounds.ts +++ b/packages/lib/client-only/hooks/use-element-bounds.ts @@ -17,7 +17,7 @@ export const useElementBounds = (elementOrSelector: HTMLElement | string, withSc : elementOrSelector; if (!$el) { - throw new Error('Element not found'); + return { top: 0, left: 0, width: 0, height: 0 }; } if (withScroll) { diff --git a/packages/lib/client-only/hooks/use-field-page-coords.ts b/packages/lib/client-only/hooks/use-field-page-coords.ts index e212e7328..59d77b59e 100644 --- a/packages/lib/client-only/hooks/use-field-page-coords.ts +++ b/packages/lib/client-only/hooks/use-field-page-coords.ts @@ -57,23 +57,48 @@ export const useFieldPageCoords = ( }; }, [calculateCoords]); + // Watch for the page element to appear in the DOM (e.g. after a virtual list + // scroll) and recalculate coords. Also attach a ResizeObserver once the page + // element exists. useEffect(() => { - const $page = document.querySelector( - `${PDF_VIEWER_PAGE_SELECTOR}[data-page-number="${field.page}"]`, - ); + const pageSelector = `${PDF_VIEWER_PAGE_SELECTOR}[data-page-number="${field.page}"]`; - if (!$page) { - return; + let resizeObserver: ResizeObserver | null = null; + + const attachResizeObserver = ($page: HTMLElement) => { + resizeObserver?.disconnect(); + resizeObserver = new ResizeObserver(() => { + calculateCoords(); + }); + resizeObserver.observe($page); + }; + + // Try to attach immediately if the page already exists. + const existingPage = document.querySelector(pageSelector); + + if (existingPage) { + attachResizeObserver(existingPage); } - const observer = new ResizeObserver(() => { - calculateCoords(); + // Watch for DOM mutations to detect when the page element appears (e.g. + // after the virtual list scrolls to a new page and renders it). + const mutationObserver = new MutationObserver(() => { + const $page = document.querySelector(pageSelector); + + if ($page) { + calculateCoords(); + attachResizeObserver($page); + } }); - observer.observe($page); + mutationObserver.observe(document.body, { + childList: true, + subtree: true, + }); return () => { - observer.disconnect(); + mutationObserver.disconnect(); + resizeObserver?.disconnect(); }; }, [calculateCoords, field.page]); diff --git a/packages/lib/client-only/hooks/use-is-page-in-dom.ts b/packages/lib/client-only/hooks/use-is-page-in-dom.ts new file mode 100644 index 000000000..58ac61c76 --- /dev/null +++ b/packages/lib/client-only/hooks/use-is-page-in-dom.ts @@ -0,0 +1,34 @@ +import { useEffect, useState } from 'react'; + +import { PDF_VIEWER_PAGE_SELECTOR } from '@documenso/lib/constants/pdf-viewer'; + +/** + * Returns whether the PDF page element for the given page number is currently + * present in the DOM. With virtual list rendering only pages near the viewport + * are mounted, so this hook lets consumers skip rendering when their page is + * virtualised away. + */ +export const useIsPageInDom = (pageNumber: number) => { + const [isPageInDom, setIsPageInDom] = useState(false); + + useEffect(() => { + const selector = `${PDF_VIEWER_PAGE_SELECTOR}[data-page-number="${pageNumber}"]`; + + setIsPageInDom(document.querySelector(selector) !== null); + + const observer = new MutationObserver(() => { + setIsPageInDom(document.querySelector(selector) !== null); + }); + + observer.observe(document.body, { + childList: true, + subtree: true, + }); + + return () => { + observer.disconnect(); + }; + }, [pageNumber]); + + return isPageInDom; +}; diff --git a/packages/lib/utils/fields.ts b/packages/lib/utils/fields.ts index ec26192eb..e69282bab 100644 --- a/packages/lib/utils/fields.ts +++ b/packages/lib/utils/fields.ts @@ -23,25 +23,44 @@ export const sortFieldsByPosition = (fields: Field[]): Field[] => { */ export const validateFieldsInserted = (fields: Field[]): boolean => { const fieldCardElements = document.getElementsByClassName('field-card-container'); + const pdfContent = document.querySelector('[data-pdf-content]'); - // Attach validate attribute on all fields. + const uninsertedFields = sortFieldsByPosition(fields.filter((field) => !field.inserted)); + + // All fields are inserted — clear the validation signal. + if (uninsertedFields.length === 0) { + pdfContent?.removeAttribute('data-validate-fields'); + return true; + } + + // Attach validate attribute on all fields currently in the DOM. Array.from(fieldCardElements).forEach((element) => { element.setAttribute('data-validate', 'true'); }); - const uninsertedFields = sortFieldsByPosition(fields.filter((field) => !field.inserted)); + // Also set a signal on the PDF viewer container so that field elements that + // mount later (e.g. after the virtual list scrolls to a new page) can pick + // up the validation state. + pdfContent?.setAttribute('data-validate-fields', 'true'); const firstUninsertedField = uninsertedFields[0]; - const firstUninsertedFieldElement = - firstUninsertedField && document.getElementById(`field-${firstUninsertedField.id}`); + if (firstUninsertedField) { + // Try direct element scroll first (works if the field's page is currently rendered). + const firstUninsertedFieldElement = document.getElementById(`field-${firstUninsertedField.id}`); - if (firstUninsertedFieldElement) { - firstUninsertedFieldElement.scrollIntoView({ behavior: 'smooth', block: 'center' }); - return false; + if (firstUninsertedFieldElement) { + firstUninsertedFieldElement.scrollIntoView({ behavior: 'smooth', block: 'center' }); + } else { + // Field not in DOM (page virtualized away) — signal the PDF viewer to + // scroll to the correct page via the data attribute. + if (pdfContent) { + pdfContent.setAttribute('data-scroll-to-page', String(firstUninsertedField.page)); + } + } } - return uninsertedFields.length === 0; + return false; }; export const validateFieldsUninserted = (): boolean => { diff --git a/packages/ui/components/document/document-read-only-fields.tsx b/packages/ui/components/document/document-read-only-fields.tsx index 5d3cb2539..dde70f32a 100644 --- a/packages/ui/components/document/document-read-only-fields.tsx +++ b/packages/ui/components/document/document-read-only-fields.tsx @@ -98,10 +98,8 @@ export const DocumentReadOnlyFields = ({ setHiddenFieldIds((prev) => ({ ...prev, [fieldId]: true })); }; - const highestPageNumber = Math.max(...fields.map((field) => field.page)); - return ( - + {fields.map( (field) => !hiddenFieldIds[field.secondaryId] && ( @@ -163,7 +161,7 @@ export const DocumentReadOnlyFields = ({

-

+

{getRecipientDisplayText(field.recipient)}

diff --git a/packages/ui/components/field/field.tsx b/packages/ui/components/field/field.tsx index 2886230b3..444256f10 100644 --- a/packages/ui/components/field/field.tsx +++ b/packages/ui/components/field/field.tsx @@ -5,6 +5,7 @@ import { createPortal } from 'react-dom'; import { useElementBounds } from '@documenso/lib/client-only/hooks/use-element-bounds'; import { useFieldPageCoords } from '@documenso/lib/client-only/hooks/use-field-page-coords'; +import { useIsPageInDom } from '@documenso/lib/client-only/hooks/use-is-page-in-dom'; import { PDF_VIEWER_PAGE_SELECTOR } from '@documenso/lib/constants/pdf-viewer'; import { isFieldUnsignedAndRequired } from '@documenso/lib/utils/advanced-fields-helpers'; @@ -81,6 +82,8 @@ export function FieldRootContainer({ readonly, }: FieldRootContainerProps) { const [isValidating, setIsValidating] = useState(false); + const isPageInDom = useIsPageInDom(field.page); + const ref = React.useRef(null); useEffect(() => { @@ -88,6 +91,21 @@ export function FieldRootContainer({ return; } + // Check the validation signal on the PDF viewer container. When a field + // mounts after the virtual list scrolls to its page, the per-element + // `data-validate` attribute will not have been set yet. The signal on the + // `[data-pdf-content]` container bridges this gap so newly-rendered fields + // pick up the validation state immediately. + const pdfContent = document.querySelector('[data-pdf-content]'); + + if ( + pdfContent?.getAttribute('data-validate-fields') === 'true' && + isFieldUnsignedAndRequired(field) + ) { + ref.current.setAttribute('data-validate', 'true'); + setIsValidating(true); + } + const observer = new MutationObserver((_mutations) => { if (ref.current) { setIsValidating(ref.current.getAttribute('data-validate') === 'true'); @@ -101,7 +119,11 @@ export function FieldRootContainer({ return () => { observer.disconnect(); }; - }, []); + }, [isPageInDom]); + + if (!isPageInDom) { + return null; + } return ( diff --git a/packages/ui/components/pdf-viewer/envelope-pdf-viewer.tsx b/packages/ui/components/pdf-viewer/envelope-pdf-viewer.tsx index e4c5a4f16..79cbaa73c 100644 --- a/packages/ui/components/pdf-viewer/envelope-pdf-viewer.tsx +++ b/packages/ui/components/pdf-viewer/envelope-pdf-viewer.tsx @@ -16,6 +16,7 @@ import { Alert, AlertDescription, AlertTitle } from '@documenso/ui/primitives/al import type { ScrollTarget } from '../virtual-list/use-virtual-list'; import { useVirtualList } from '../virtual-list/use-virtual-list'; import { PdfViewerErrorState, PdfViewerLoadingState } from './pdf-viewer-states'; +import { useScrollToPage } from './use-scroll-to-page'; export type EnvelopePdfViewerProps = { className?: string; @@ -156,7 +157,7 @@ const VirtualizedPageList = ({ }: VirtualizedPageListProps) => { const contentRef = useRef(null); - const { virtualItems, totalSize, constraintWidth } = useVirtualList({ + const { virtualItems, totalSize, constraintWidth, scrollToItem } = useVirtualList({ scrollRef: scrollParentRef, constraintRef, contentRef, @@ -174,9 +175,12 @@ const VirtualizedPageList = ({ overscan: 10, }); + useScrollToPage(contentRef, scrollToItem); + return (
{ const contentRef = useRef(null); - const { virtualItems, totalSize, constraintWidth } = useVirtualList({ + const { virtualItems, totalSize, constraintWidth, scrollToItem } = useVirtualList({ scrollRef: scrollParentRef, constraintRef, contentRef, @@ -235,9 +236,12 @@ const VirtualizedPageList = ({ overscan: 5, }); + useScrollToPage(contentRef, scrollToItem); + return (
, + scrollToItem: (index: number) => void, +) => { + useEffect(() => { + const el = contentRef.current; + + if (!el) { + return; + } + + const observer = new MutationObserver((mutations) => { + for (const mutation of mutations) { + if (mutation.type === 'attributes' && mutation.attributeName === 'data-scroll-to-page') { + const raw = el.getAttribute('data-scroll-to-page'); + + if (raw) { + const pageNumber = parseInt(raw, 10); + + if (!isNaN(pageNumber) && pageNumber >= 1) { + // Pages are 1-indexed, virtual list items are 0-indexed. + scrollToItem(pageNumber - 1); + } + + el.removeAttribute('data-scroll-to-page'); + } + } + } + }); + + observer.observe(el, { attributes: true, attributeFilter: ['data-scroll-to-page'] }); + + return () => observer.disconnect(); + }, [contentRef, scrollToItem]); +}; diff --git a/packages/ui/components/virtual-list/use-virtual-list.ts b/packages/ui/components/virtual-list/use-virtual-list.ts index 532e8564a..21db2e5c9 100644 --- a/packages/ui/components/virtual-list/use-virtual-list.ts +++ b/packages/ui/components/virtual-list/use-virtual-list.ts @@ -34,6 +34,14 @@ export type VirtualListResult = { virtualItems: VirtualItem[]; totalSize: number; constraintWidth: number; + + /** + * Scroll the scroll container so that the item at the given index is visible. + * + * The scroll position is calculated from the precomputed item offsets and + * adjusted for any content offset (e.g. headers above the virtual list). + */ + scrollToItem: (index: number) => void; }; /** @@ -292,9 +300,56 @@ export const useVirtualList = (options: VirtualListOptions): VirtualListResult = findStartIndex, ]); + /** + * Imperatively scroll the scroll container so that the item at the given + * index is at the top of the viewport. + */ + const scrollToItem = useCallback( + (index: number) => { + if (index < 0 || index >= itemCount) { + return; + } + + const itemOffset = offsets[index] ?? 0; + + if (scrollRef === 'window') { + const contentEl = contentRef?.current; + const contentTop = contentEl ? contentEl.getBoundingClientRect().top + window.scrollY : 0; + + window.scrollTo({ + top: contentTop + itemOffset, + behavior: 'smooth', + }); + } else { + const scrollEl = scrollRef.current; + + if (!scrollEl) { + return; + } + + // Recalculate content offset to get the most up-to-date value. + const contentEl = contentRef?.current; + let contentOffset = 0; + + if (contentEl) { + const scrollRect = scrollEl.getBoundingClientRect(); + const contentRect = contentEl.getBoundingClientRect(); + contentOffset = contentRect.top - scrollRect.top + scrollEl.scrollTop; + } + + scrollEl.scrollTo({ + top: contentOffset + itemOffset, + behavior: 'smooth', + }); + } + }, + [scrollRef, contentRef, offsets, itemCount], + ); + return { virtualItems, totalSize, constraintWidth, + scrollToItem, }; }; diff --git a/packages/ui/primitives/document-flow/field-item.tsx b/packages/ui/primitives/document-flow/field-item.tsx index 0cb271ae9..1f3156b38 100644 --- a/packages/ui/primitives/document-flow/field-item.tsx +++ b/packages/ui/primitives/document-flow/field-item.tsx @@ -10,6 +10,7 @@ import { Rnd } from 'react-rnd'; import { useSearchParams } from 'react-router'; import { useElementBounds } from '@documenso/lib/client-only/hooks/use-element-bounds'; +import { useIsPageInDom } from '@documenso/lib/client-only/hooks/use-is-page-in-dom'; import { PDF_VIEWER_PAGE_SELECTOR } from '@documenso/lib/constants/pdf-viewer'; import type { TFieldMetaSchema } from '@documenso/lib/types/field-meta'; import { ZCheckboxFieldMeta, ZRadioFieldMeta } from '@documenso/lib/types/field-meta'; @@ -50,7 +51,17 @@ export type FieldItemProps = { /** * The item when editing fields?? */ -export const FieldItem = ({ +export const FieldItem = (props: FieldItemProps) => { + const isPageInDom = useIsPageInDom(props.field.pageNumber); + + if (!isPageInDom) { + return null; + } + + return ; +}; + +const FieldItemInner = ({ fieldClassName, field, passive,