Forráskód Böngészése

fix(client): streamline continuous PDF previews

imccyu 1 hónapja
szülő
commit
693621dcad

+ 7 - 8
apps/web/tests/document-preview.e2e.ts

@@ -253,29 +253,28 @@ describe.skipIf(MODE === 'record')('web e2e: document preview through Files', ()
     await expect.poll(() => viewer.innerText()).toBe('PDF')
     const canvas = preview.getByRole('img', { name: 'PDF page 1', exact: true })
     await canvas.waitFor({ state: 'visible', timeout: 30_000 })
-    expect(await preview.getByRole('spinbutton', { name: 'Page', exact: true }).inputValue()).toBe('1')
-    expect(await preview.getByText('of 2', { exact: true }).isVisible()).toBe(true)
+    expect(await preview.locator('[role="toolbar"]').count()).toBe(0)
+    expect(await preview.locator('[data-pdf-page]').count()).toBe(2)
     await expect.poll(() => canvasColor(canvas), { timeout: 30_000 }).toBe('red')
     const firstColor = await canvasColor(canvas)
     expect(firstColor).toBe('red')
     const workerNames = await Promise.all(page.workers().map(worker => worker.evaluate(() => self.name)))
     expect(workerNames).toContain('dsh-pdf')
-    await preview.getByRole('button', { name: 'Next page', exact: true }).click()
+    await preview.locator('[data-pdf-page="2"]').scrollIntoViewIfNeeded()
     const secondPage = preview.getByRole('img', { name: 'PDF page 2', exact: true })
     await secondPage.waitFor({ state: 'visible', timeout: 30_000 })
     await expect.poll(() => canvasColor(secondPage), { timeout: 30_000 }).toBe('blue')
     const secondColor = await canvasColor(secondPage)
     expect(secondColor).toBe('blue')
-    expect(await preview.getByRole('spinbutton', { name: 'Page', exact: true }).inputValue()).toBe('2')
+    expect(await body.evaluate(node => node.scrollWidth <= node.clientWidth)).toBe(true)
     const pdfTab = column.locator('[data-dockkit-tab]').filter({ has: page.getByText('smoke.pdf', { exact: true }) })
     const pdfTabId = await pdfTab.getAttribute('data-dockkit-tab')
     expect(pdfTabId).not.toBeNull()
     await filesTab.click()
     await column.locator('[data-files-state="tree"]').waitFor({ state: 'visible' })
     await pdfTab.click()
+    await preview.locator('[data-pdf-page="2"]').scrollIntoViewIfNeeded()
     await secondPage.waitFor({ state: 'visible', timeout: 30_000 })
-    const restoredPage = await preview.getByRole('spinbutton', { name: 'Page', exact: true }).inputValue()
-    expect(restoredPage).toBe('2')
     await expect.poll(() => canvasColor(secondPage), { timeout: 30_000 }).toBe('blue')
     const restoredColor = await canvasColor(secondPage)
     expect(restoredColor).toBe('blue')
@@ -285,9 +284,9 @@ describe.skipIf(MODE === 'record')('web e2e: document preview through Files', ()
       '## PDF', '',
       `- Viewer: ${await viewer.innerText()}`,
       `- Worker: ${workerNames.find(name => name === 'dsh-pdf')}`,
-      `- Page count: ${await preview.getByText('of 2', { exact: true }).innerText()}`,
+      `- Continuous pages: ${await preview.locator('[data-pdf-page]').count()}`,
+      `- Horizontal overflow: ${String(await body.evaluate(node => node.scrollWidth > node.clientWidth))}`,
       `- Canvas fills: ${[firstColor, secondColor, restoredColor].join(' -> ')}`,
-      `- Page after Files round trip: ${restoredPage}`,
       `- Same tab: ${String(await pdfTab.getAttribute('data-dockkit-tab') === pdfTabId)}`,
     ].join('\n'))
 

+ 3 - 0
packages/client/ui-sidebar-documentpreview/src/client/pdf/PdfBody.module.css

@@ -17,6 +17,9 @@
 }
 
 .placeholder {
+  display: flex;
+  align-items: center;
+  justify-content: center;
   width: 100%;
   aspect-ratio: 3 / 4;
 }

+ 24 - 12
packages/client/ui-sidebar-documentpreview/src/client/pdf/PdfBody.tsx

@@ -1,11 +1,11 @@
 /** PDF page presentation; binary content and tab information come from the document owner. */
-import { useEffect, useRef, useState, type ReactNode } from 'react'
+import { useCallback, useEffect, useRef, useState, type ReactNode } from 'react'
 import { Button } from '@deepseek-ai/dsh-client-ui-primitives'
 import type { PropsLocale, PropsStore } from '@deepseek-ai/dsh-client-ui-slots'
 import type { TabId } from '@deepseek-ai/dsh-client-ui-dockkit'
 import type { DocumentPreviewProps } from '../document/contract.ts'
 import { LoadingIndicator } from '../LoadingIndicator.tsx'
-import type { PdfStore } from './store.ts'
+import { DEFAULT_PDF_VIEW, type PdfStore } from './store.ts'
 import { renderPdfPage, type PdfDocument } from './document.ts'
 import { openPdf } from './runtime.ts'
 import { PdfWorkerFailure } from './errors.ts'
@@ -36,10 +36,14 @@ type LoadState =
  */
 export function PdfBody(props: PdfBodyProps): ReactNode {
   const { tab } = props.useTabInfo()
+  const view = props.useStore(state => state.byTab[tab.id] ?? DEFAULT_PDF_VIEW)
   const data = props.content.kind === 'bytes' ? props.content.data : undefined
   const [load, setLoad] = useState<LoadState>()
   const [attempt, setAttempt] = useState(0)
-  const { retainTab, t } = props
+  const { retainTab, actions, t } = props
+  const pageVisible = useCallback((page: number): void => {
+    actions.page(tab.id, page)
+  }, [actions, tab.id])
 
   useEffect(() => { retainTab(tab.id, tab.signal) }, [retainTab, tab.id, tab.signal])
   useEffect(() => {
@@ -70,38 +74,45 @@ export function PdfBody(props: PdfBodyProps): ReactNode {
   }
   return <section className={css.body} data-pdf-preview>
     {Array.from({ length: load.document.numPages }, (_, index) => (
-      <PdfPage key={index} document={load.document} page={index + 1} signal={tab.signal} t={t} />
+      <PdfPage key={index} document={load.document} page={index + 1}
+        requested={index === 0 || view.page === index + 1} onVisible={pageVisible} signal={tab.signal} t={t} />
     ))}
   </section>
 }
 
-function PdfPage({ document, page, signal, t }: {
+function PdfPage({ document, page, requested: initiallyRequested, onVisible, signal, t }: {
   readonly document: PdfDocument
   readonly page: number
+  readonly requested: boolean
+  readonly onVisible: (page: number) => void
   readonly signal: AbortSignal
 } & PropsLocale<'sidebarPdf'>): ReactNode {
   const host = useRef<HTMLDivElement>(null)
   const canvas = useRef<HTMLCanvasElement>(null)
-  const [requested, setRequested] = useState(page === 1)
+  const [requested, setRequested] = useState(initiallyRequested)
   const [state, setState] = useState<'loading' | 'ready'>('loading')
   const [failure, setFailure] = useState<{ readonly error: unknown }>()
   const [attempt, setAttempt] = useState(0)
   useEffect(() => {
-    if (requested) return
     const node = host.current as HTMLDivElement
     if (typeof IntersectionObserver === 'undefined') {
       setRequested(true)
       return
     }
+    let disposed = false
     let observer: IntersectionObserver | undefined
     observer = new IntersectionObserver((entries) => {
-      if (!entries.some(entry => entry.isIntersecting)) return
+      if (disposed || !entries.some(entry => entry.isIntersecting)) return
       setRequested(true)
+      onVisible(page)
       observer?.disconnect()
     }, { rootMargin: '100% 0px' })
     observer.observe(node)
-    return () => { observer?.disconnect() }
-  }, [requested])
+    return () => {
+      disposed = true
+      observer?.disconnect()
+    }
+  }, [page, onVisible])
   useEffect(() => {
     if (!requested) return
     // The canvas is unconditional; this effect runs after its ref is committed.
@@ -117,8 +128,9 @@ function PdfPage({ document, page, signal, t }: {
     return () => { lifetime.abort() }
   }, [document, page, requested, signal, attempt])
   return <div ref={host} className={css.page} data-pdf-page={page}>
-    {!requested && <div className={css.placeholder} />}
-    {requested && failure === undefined && state === 'loading' && <LoadingIndicator className={css.status} label={t('rendering')} />}
+    {failure === undefined && state !== 'ready' && <div className={css.placeholder}>
+      {requested && <LoadingIndicator className={css.status} label={t('rendering')} />}
+    </div>}
     {failure !== undefined && <div className={css.status} role="alert">
       <span>{failureText(failure.error, t)}</span>
       <Button size="sm" onClick={() => { setAttempt(value => value + 1) }}>{t('retry')}</Button>

+ 1 - 1
packages/client/ui-sidebar-documentpreview/tests/document-seat.client.spec.tsx

@@ -41,7 +41,7 @@ async function boot() {
   rt.ctx.provide('locale', locale)
   rt.slots.installLocale(locale)
   await rt.declare({
-    rightbar: { kind: 'single', scope: 'session' },
+    rightbar: { kind: 'single', scope: 'root' },
     'conversation.session.header.corner': { kind: 'single', scope: 'session' },
   })
   await rt.sessions.add({ id: SESSION })

+ 48 - 59
packages/client/ui-sidebar-documentpreview/tests/pdf-body.client.spec.tsx

@@ -33,7 +33,28 @@ beforeEach(() => {
   engine.render.mockReset().mockResolvedValue({ width: 100, height: 100 })
 })
 
-afterEach(() => { cleanup() })
+afterEach(() => {
+  cleanup()
+  vi.unstubAllGlobals()
+})
+
+class IntersectionObserverStub {
+  static instances: IntersectionObserverStub[] = []
+  readonly observed = new Set<Element>()
+  disconnected = false
+
+  constructor(private readonly callback: IntersectionObserverCallback) {
+    IntersectionObserverStub.instances.push(this)
+  }
+
+  observe(element: Element): void { this.observed.add(element) }
+  unobserve(element: Element): void { this.observed.delete(element) }
+  disconnect(): void { this.disconnected = true; this.observed.clear() }
+  takeRecords(): IntersectionObserverEntry[] { return [] }
+  intersect(element: Element, isIntersecting: boolean): void {
+    this.callback([{ target: element, isIntersecting } as IntersectionObserverEntry], this as unknown as IntersectionObserver)
+  }
+}
 
 function harness() {
   const instance = createPdfStore().create()
@@ -75,25 +96,19 @@ describe('PDF body', () => {
     expect(engine.open).not.toHaveBeenCalled()
   })
 
-  it('shows loading, navigates pages, zooms, and restores the tab view after remount', async () => {
+  it('shows loading, omits the paging toolbar, and renders a continuous page sequence', async () => {
     const h = harness()
-    const mounted = render(<h.View />)
+    const view = render(<h.View />)
     expect(screen.getByRole('status').textContent).toBe('Opening PDF…')
     expect(screen.getByRole('status').hasAttribute('data-document-loading')).toBe(true)
     await act(async () => { loads[0]!.deferred.resolve(documentOf()) })
-    expect(screen.getByRole('button', { name: 'Previous page' }).hasAttribute('disabled')).toBe(true)
-    fireEvent.click(screen.getByRole('button', { name: 'Next page' }))
-    fireEvent.click(screen.getByRole('button', { name: 'Zoom in' }))
-    expect(h.instance.getSnapshot().byTab[h.tabId]).toEqual({ page: 2, zoom: 1.25 })
-    mounted.unmount()
-    expect(loads[0]!.dispose).toHaveBeenCalledOnce()
-    render(<h.View />)
-    await act(async () => { loads[1]!.deferred.resolve(documentOf()) })
-    expect(screen.getByRole('spinbutton', { name: 'Page' }).getAttribute('value')).toBe('2')
-    expect(screen.getByRole('button', { name: 'Reset zoom' }).textContent).toBe('125%')
-    fireEvent.change(screen.getByRole('spinbutton', { name: 'Page' }), { target: { value: '99' } })
-    expect(h.instance.getSnapshot().byTab[h.tabId]?.page).toBe(3)
-    expect(screen.getByRole('button', { name: 'Next page' }).hasAttribute('disabled')).toBe(true)
+    await act(async () => {})
+    expect(view.container.querySelector('[role="toolbar"]')).toBeNull()
+    expect([...view.container.querySelectorAll('[data-pdf-page]')].map(page => page.getAttribute('data-pdf-page')))
+      .toEqual(['1', '2', '3'])
+    expect(screen.getAllByRole('img').map(image => image.getAttribute('aria-label')))
+      .toEqual(['PDF page 1', 'PDF page 2', 'PDF page 3'])
+    expect(engine.render.mock.calls.map(([, page, zoom]) => [page, zoom])).toEqual([[1, 1], [2, 1], [3, 1]])
   })
 
   it('keeps the replacement document when the previous load settles late', async () => {
@@ -104,7 +119,7 @@ describe('PDF body', () => {
     const latest = documentOf(2)
     await act(async () => { loads[1]!.deferred.resolve(latest) })
     await act(async () => { loads[0]!.deferred.resolve(documentOf(99)) })
-    expect(screen.getByText('of 2')).toBeTruthy()
+    expect(screen.getAllByRole('img')).toHaveLength(2)
     expect(engine.render.mock.calls.every(([document]) => document === latest)).toBe(true)
   })
 
@@ -129,37 +144,23 @@ describe('PDF body', () => {
     expect(screen.getByRole('alert').textContent).toContain(en.password)
   })
 
-  it('cancels an old page render on navigation and ignores its late rejection', async () => {
-    const drawing = Promise.withResolvers<{ width: number; height: number }>()
-    engine.render.mockImplementationOnce(() => drawing.promise)
-    const h = harness()
-    render(<h.View />)
-    await act(async () => { loads[0]!.deferred.resolve(documentOf(2)) })
-    const oldSignal = engine.render.mock.calls[0]![4]
-    fireEvent.click(screen.getByRole('button', { name: 'Next page' }))
-    expect(oldSignal.aborted).toBe(true)
-    await act(async () => { drawing.reject(new Error('cancelled old page')) })
-    expect(screen.queryByRole('alert')).toBeNull()
-    expect(screen.getByRole('img', { name: 'PDF page 2' })).toBeTruthy()
-  })
-
-  it('handles previous page, blank page input, zoom-out, reset, and the zoom limits', async () => {
+  it('renders later pages only when they approach the viewport and records the reached page', async () => {
+    IntersectionObserverStub.instances = []
+    vi.stubGlobal('IntersectionObserver', IntersectionObserverStub)
     const h = harness()
-    render(<h.View />)
+    const view = render(<h.View />)
     await act(async () => { loads[0]!.deferred.resolve(documentOf(3)) })
-    fireEvent.click(screen.getByRole('button', { name: 'Next page' }))
-    fireEvent.click(screen.getByRole('button', { name: 'Previous page' }))
-    fireEvent.change(screen.getByRole('spinbutton', { name: 'Page' }), { target: { value: '' } })
-    expect(h.instance.getSnapshot().byTab[h.tabId]?.page).toBe(1)
-    for (let i = 0; i < 3; i++) fireEvent.click(screen.getByRole('button', { name: 'Zoom out' }))
-    expect(h.instance.getSnapshot().byTab[h.tabId]?.zoom).toBe(0.25)
-    expect(screen.getByRole('button', { name: 'Zoom out' }).hasAttribute('disabled')).toBe(true)
-    fireEvent.click(screen.getByRole('button', { name: 'Reset zoom' }))
-    expect(h.instance.getSnapshot().byTab[h.tabId]?.zoom).toBe(1)
-    for (let i = 0; i < 12; i++) fireEvent.click(screen.getByRole('button', { name: 'Zoom in' }))
-    expect(h.instance.getSnapshot().byTab[h.tabId]?.zoom).toBe(4)
-    expect(screen.getByRole('button', { name: 'Zoom in' }).hasAttribute('disabled')).toBe(true)
-    await act(async () => {})
+    expect(engine.render.mock.calls.map(([, page]) => page)).toEqual([1])
+    const second = view.container.querySelector('[data-pdf-page="2"]') as HTMLElement
+    const observer = IntersectionObserverStub.instances.find(instance => instance.observed.has(second))!
+    act(() => { observer.intersect(second, false) })
+    expect(engine.render.mock.calls.map(([, page]) => page)).toEqual([1])
+    await act(async () => { observer.intersect(second, true) })
+    expect(engine.render.mock.calls.map(([, page]) => page)).toEqual([1, 2])
+    expect(h.instance.getSnapshot().byTab[h.tabId]?.page).toBe(2)
+    expect(screen.getByRole('img', { name: 'PDF page 2' })).toBeTruthy()
+    view.unmount()
+    expect(IntersectionObserverStub.instances.every(instance => instance.disconnected)).toBe(true)
   })
 
   it('renders a structured Worker failure through its own locale', async () => {
@@ -181,19 +182,7 @@ describe('PDF body', () => {
       engine.open.mock.calls[0]![2](new PdfWorkerFailure(new ErrorEvent('error')))
     })
     expect(screen.queryByRole('alert')).toBeNull()
-    expect(screen.getByText('of 2')).toBeTruthy()
-  })
-
-  it('keeps a newer page visible when the cancelled render resolves late', async () => {
-    const drawing = Promise.withResolvers<{ width: number; height: number }>()
-    engine.render.mockImplementationOnce(() => drawing.promise)
-    const h = harness()
-    render(<h.View />)
-    await act(async () => { loads[0]!.deferred.resolve(documentOf(2)) })
-    fireEvent.click(screen.getByRole('button', { name: 'Next page' }))
-    await act(async () => { drawing.resolve({ width: 100, height: 100 }) })
-    expect(screen.queryByRole('img', { name: 'PDF page 1' })).toBeNull()
-    expect(screen.getByRole('img', { name: 'PDF page 2' })).toBeTruthy()
+    expect(screen.getAllByRole('img')).toHaveLength(2)
   })
 
   it('shows a foreign render rejection and retries that page', async () => {