Address web SDK demo review feedback

Author: MahmoudElsayad

web/viewer/web-sdk-demo/src/App.tsx

@@ -1,4 +1,4 @@
-import { useCallback, useEffect, useState } from 'react'
+import { useCallback, useEffect, useRef, useState } from 'react'
 import { FileExplorer } from './FileExplorer'
 import { NutrientViewer } from './NutrientViewer'
 import { Toolbar } from './Toolbar'
@@ -36,6 +36,7 @@ const BUILTIN_FILE: FileEntry = {
 
 export function App() {
   const [files, setFiles] = useState<FileEntry[]>([BUILTIN_FILE])
+  const filesRef = useRef<FileEntry[]>([BUILTIN_FILE])
   const [activeId, setActiveId] = useState<string>(BUILTIN_FILE.id)
   const [instance, setInstance] = useState<SDKInstance | null>(null)
 
@@ -81,13 +82,16 @@ export function App() {
     [activeId],
   )
 
+  useEffect(() => {
+    filesRef.current = files
+  }, [files])
+
   useEffect(() => {
     return () => {
-      files.forEach((f) => {
+      filesRef.current.forEach((f) => {
         if (!f.isBuiltin) URL.revokeObjectURL(f.url)
       })
     }
-    // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [])
 
   // ── Place a field at a specific page-space rect (used by drag-drop + click) ──
@@ -160,7 +164,9 @@ export function App() {
 
         if (!formField) {
           console.warn('SDK FormField constructor not found for field type:', type)
-          await instance.create(widget)
+          window.alert(
+            'Could not create this form field because the SDK form-field constructor is unavailable.',
+          )
           return
         }
 
@@ -297,15 +303,20 @@ export function App() {
         const Rect = sdk.Geometry?.Rect ?? sdk.Rect
         if (!ImageAnnotation || !Rect) {
           console.warn('SDK ImageAnnotation/Rect not found — signature not inserted.')
+          window.alert(
+            'Could not insert the signature because image annotations are unavailable.',
+          )
           return
         }
 
         const res = await fetch(dataUrl)
         const blob = await res.blob()
         const contentType = blob.type || getDataUrlContentType(dataUrl) || 'image/png'
-        const attachmentId = instance.createAttachment
-          ? await instance.createAttachment(blob)
-          : null
+        if (!instance.createAttachment) {
+          window.alert('Could not insert the signature because attachments are unavailable.')
+          return
+        }
+        const attachmentId = await instance.createAttachment(blob)
         const { pageIndex, boundingBox } = await resolveSignatureInsertionTarget(
           instance,
           signingModal,
@@ -322,8 +333,10 @@ export function App() {
         await instance.create(annotation)
       } catch (err) {
         console.error('Failed to insert signature', err)
+        window.alert('Could not insert the signature.')
+      } finally {
+        setSigningModal(null)
       }
-      setSigningModal(null)
     },
     [instance, refreshSavedSignatures, signingModal],
   )

web/viewer/web-sdk-demo/src/Toolbar.tsx

@@ -32,25 +32,42 @@ export function Toolbar({
   const [zoom, setZoom] = useState(1)
   const [pageInput, setPageInput] = useState('1')
   const [signMenuOpen, setSignMenuOpen] = useState(false)
+  const [operationError, setOperationError] = useState<string | null>(null)
+
+  const syncPageState = () => {
+    if (!instance) return
+    const nextPageCount = instance.totalPageCount
+    const nextPageIndex = Math.min(
+      instance.viewState.currentPageIndex,
+      Math.max(nextPageCount - 1, 0),
+    )
+    setPageIndex(nextPageIndex)
+    setPageCount(nextPageCount)
+    setZoom(instance.currentZoomLevel ?? 1)
+    setPageInput(String(nextPageIndex + 1))
+  }
 
   useEffect(() => {
     if (!instance) {
       setPageIndex(0)
       setPageCount(0)
       setZoom(1)
       setPageInput('1')
+      setOperationError(null)
       return
     }
 
-    setPageIndex(instance.viewState.currentPageIndex)
-    setPageCount(instance.totalPageCount)
-    setZoom(instance.currentZoomLevel ?? 1)
-    setPageInput(String(instance.viewState.currentPageIndex + 1))
+    syncPageState()
 
     const onViewState = (vs: unknown) => {
       const state = vs as SDKInstance['viewState']
-      setPageIndex(state.currentPageIndex)
-      setPageInput(String(state.currentPageIndex + 1))
+      const nextPageIndex = Math.min(
+        state.currentPageIndex,
+        Math.max(instance.totalPageCount - 1, 0),
+      )
+      setPageIndex(nextPageIndex)
+      setPageCount(instance.totalPageCount)
+      setPageInput(String(nextPageIndex + 1))
     }
     const onZoom = (z: unknown) => setZoom(z as number)
 
@@ -62,6 +79,12 @@ export function Toolbar({
     }
   }, [instance])
 
+  useEffect(() => {
+    if (!operationError) return
+    const timeout = window.setTimeout(() => setOperationError(null), 4000)
+    return () => window.clearTimeout(timeout)
+  }, [operationError])
+
   const apply = (transform: (vs: SDKInstance['viewState']) => SDKInstance['viewState']) => {
     if (!instance) return
     instance.setViewState(transform)
@@ -81,33 +104,55 @@ export function Toolbar({
   const fitWidth = () =>
     apply((vs) => vs.set('zoom', window.NutrientViewer!.ZoomMode.FIT_TO_WIDTH))
 
-  const rotatePages = async (rotateBy: 90 | -90) => {
-    if (!instance) return
-    await instance.applyOperations([
-      { type: 'rotatePages', pageIndexes: [pageIndex], rotateBy },
-    ])
+  const runOperation = async (
+    label: string,
+    operation: (viewer: SDKInstance) => Promise<unknown>,
+    refreshPages = true,
+  ) => {
+    if (!instance) return false
+    setOperationError(null)
+    try {
+      await operation(instance)
+      if (refreshPages) syncPageState()
+      return true
+    } catch (err) {
+      console.error(`${label} failed`, err)
+      setOperationError(`${label} failed`)
+      return false
+    }
   }
-  const deletePage = async () => {
-    if (!instance || pageCount <= 1) return
-    await instance.applyOperations([
-      { type: 'removePages', pageIndexes: [pageIndex] },
-    ])
+
+  const rotatePages = (rotateBy: 90 | -90) => {
+    return runOperation('Rotate page', (viewer) =>
+      viewer.applyOperations([
+        { type: 'rotatePages', pageIndexes: [pageIndex], rotateBy },
+      ]),
+    )
   }
-  const addPage = async () => {
-    if (!instance) return
-    await instance.applyOperations([
-      {
-        type: 'addPage',
-        afterPageIndex: pageIndex,
-        backgroundColor: window.NutrientViewer!.Color
-          ? new (window.NutrientViewer!.Color as new (...args: unknown[]) => unknown)({
-              r: 255,
-              g: 255,
-              b: 255,
-            })
-          : undefined,
-      },
-    ])
+  const deletePage = () => {
+    if (pageCount <= 1) return
+    return runOperation('Delete page', (viewer) =>
+      viewer.applyOperations([
+        { type: 'removePages', pageIndexes: [pageIndex] },
+      ]),
+    )
+  }
+  const addPage = () => {
+    return runOperation('Add page', (viewer) =>
+      viewer.applyOperations([
+        {
+          type: 'addPage',
+          afterPageIndex: pageIndex,
+          backgroundColor: window.NutrientViewer!.Color
+            ? new (window.NutrientViewer!.Color as new (...args: unknown[]) => unknown)({
+                r: 255,
+                g: 255,
+                b: 255,
+              })
+            : undefined,
+        },
+      ]),
+    )
   }
 
   const search = () => setMode(window.NutrientViewer!.InteractionMode.SEARCH)
@@ -123,61 +168,94 @@ export function Toolbar({
     const fromIndex = fromOneBased - 1
     if (fromIndex < 0 || fromIndex >= pageCount) return
 
-    // The user's mental model: "After page N" means the moved page should end up
-    // at slot N+1 in the resulting document. Compute that target final index and
-    // translate it into the SDK's afterPageIndex/beforePageIndex (which reference
-    // the *original* page positions).
+    const afterIndex = afterOneBased - 1
+    if (afterOneBased > pageCount || afterIndex === fromIndex) {
+      setMovePopoverOpen(false)
+      return
+    }
+
     const targetIndex =
-      afterOneBased <= 0 ? 0 : Math.min(afterOneBased, pageCount - 1)
+      afterOneBased <= 0
+        ? 0
+        : fromIndex < afterIndex
+          ? afterIndex
+          : afterIndex + 1
 
     if (targetIndex === fromIndex) {
       setMovePopoverOpen(false)
       return
     }
 
     const op =
-      targetIndex === 0
+      afterOneBased <= 0
         ? { type: 'movePages', pageIndexes: [fromIndex], beforePageIndex: 0 }
         : {
             type: 'movePages',
             pageIndexes: [fromIndex],
-            // Forward move: removing the source shifts later pages left by one,
-            // so the original-numbering anchor equals the desired final index.
-            // Backward move: source removal doesn't affect indices before it,
-            // so the anchor is target − 1 (insert "after that" → at target).
-            afterPageIndex: targetIndex > fromIndex ? targetIndex : targetIndex - 1,
+            afterPageIndex: afterIndex,
           }
 
-    await instance.applyOperations([op])
-    instance.setViewState((vs) => vs.set('currentPageIndex', targetIndex))
-    setMovePopoverOpen(false)
+    const moved = await runOperation('Move page', async (viewer) => {
+      await viewer.applyOperations([op])
+      viewer.setViewState((vs) => vs.set('currentPageIndex', targetIndex))
+    })
+    if (moved) setMovePopoverOpen(false)
   }
 
-  const insertImage = async () => {
+  const insertImage = () => {
     if (!instance) return
+    const sdk = window.NutrientViewer
+    const ImageAnnotation = sdk?.Annotations?.ImageAnnotation ?? sdk?.ImageAnnotation
+    const Rect = sdk?.Geometry?.Rect ?? sdk?.Rect
+    if (!sdk || !ImageAnnotation || !Rect || !instance.createAttachment) {
+      setOperationError('Insert image unavailable')
+      return
+    }
+
     const input = document.createElement('input')
     input.type = 'file'
     input.accept = 'image/*'
     input.onchange = () => {
-      // Triggering image insertion requires deeper SDK plumbing (attachments +
-      // image annotations). For this demo, we just enter PAN mode and log.
-      console.info('TODO: wire insertImage to instance.create()', input.files)
+      const file = input.files?.[0]
+      if (!file) return
+      void runOperation(
+        'Insert image',
+        async (viewer) => {
+          const attachmentId = await viewer.createAttachment(file)
+          const pageIndex = viewer.viewState.currentPageIndex
+          const annotation = new ImageAnnotation({
+            id: sdk.generateInstantId?.(),
+            pageIndex,
+            boundingBox: new Rect({ left: 80, top: 80, width: 180, height: 120 }),
+            description: file.name || 'Inserted image',
+            imageAttachmentId: attachmentId,
+            contentType: file.type || 'image/png',
+          })
+          await viewer.create(annotation)
+        },
+        false,
+      )
     }
     input.click()
   }
 
-  const download = async () => {
-    if (!instance) return
-    const buffer = await instance.exportPDF()
-    const blob = new Blob([buffer], { type: 'application/pdf' })
-    const url = URL.createObjectURL(blob)
-    const a = document.createElement('a')
-    a.href = url
-    a.download = fileName ?? 'document.pdf'
-    document.body.appendChild(a)
-    a.click()
-    document.body.removeChild(a)
-    URL.revokeObjectURL(url)
+  const download = () => {
+    return runOperation(
+      'Download',
+      async (viewer) => {
+        const buffer = await viewer.exportPDF()
+        const blob = new Blob([buffer], { type: 'application/pdf' })
+        const url = URL.createObjectURL(blob)
+        const a = document.createElement('a')
+        a.href = url
+        a.download = fileName ?? 'document.pdf'
+        document.body.appendChild(a)
+        a.click()
+        document.body.removeChild(a)
+        window.setTimeout(() => URL.revokeObjectURL(url), 0)
+      },
+      false,
+    )
   }
 
   const disabled = !instance
@@ -413,6 +491,12 @@ export function Toolbar({
         </Popover>
       </div>
 
+      {operationError && (
+        <span className="toolbar__status" role="status">
+          {operationError}
+        </span>
+      )}
+
       <div className="toolbar__group toolbar__group--right">
         <IconButton
           label="Search"

web/viewer/web-sdk-demo/src/form-creator/index.ts

@@ -225,15 +225,19 @@ async function getFormFieldForAnnotation(
   instance: SDKInstance,
   annotation: WidgetLikeAnnotation,
 ) {
-  const fields = await instance.getFormFields?.()
-  const allFields = fields?.toArray?.() ?? []
+  const fields = await instance.getFormFields()
+  const allFields = fields.toArray()
   return allFields.find((field) => field.name === annotation.formFieldName) ?? null
 }
 
 async function deleteField(instance: SDKInstance, annotation: WidgetLikeAnnotation) {
   try {
     const formField = await getFormFieldForAnnotation(instance, annotation)
-    const target = formField?.id ?? formField ?? annotation.id
+    if (!formField) {
+      console.warn('No backing form field found; skipping widget-only delete.')
+      return
+    }
+    const target = formField.id ?? formField
     if (!target) return
     await instance.delete(target)
   } catch (error) {
@@ -250,8 +254,13 @@ async function renameField(
     const updates: unknown[] = []
     const formField = await getFormFieldForAnnotation(instance, annotation)
 
-    if (formField?.set) updates.push(formField.set('name', nextName))
-    if (annotation.set) updates.push(annotation.set('formFieldName', nextName))
+    if (!formField?.set || !annotation.set) {
+      console.warn('No backing form field found; skipping partial field rename.')
+      return
+    }
+
+    updates.push(formField.set('name', nextName))
+    updates.push(annotation.set('formFieldName', nextName))
 
     if (updates.length > 0) await instance.update(updates)
   } catch (error) {