-
Notifications
You must be signed in to change notification settings - Fork 11
feat(workspaces): reveal PDF citations at cited pages #685
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
4c71950
cedc887
49d82b7
b7e0338
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,6 +28,7 @@ import { | |
| ScrollPluginPackage, | ||
| ScrollStrategy, | ||
| useScroll, | ||
| useScrollCapability, | ||
| } from "@embedpdf/plugin-scroll/react"; | ||
| import { | ||
| SelectionPluginPackage, | ||
|
|
@@ -44,7 +45,10 @@ import { | |
| WorkspaceCaptureViewerFrame, | ||
| } from "#/features/workspaces/components/WorkspaceCaptureChrome"; | ||
| import { useFileItemToolbar } from "#/features/workspaces/components/WorkspaceItemToolbarSlot"; | ||
| import { useWorkspacePaneHotkey } from "#/features/workspaces/components/WorkspacePaneRuntime"; | ||
| import { | ||
| useWorkspacePaneHotkey, | ||
| useWorkspacePaneRuntime, | ||
| } from "#/features/workspaces/components/WorkspacePaneRuntime"; | ||
| import { WorkspacePdfAskSelectionMenu } from "#/features/workspaces/components/WorkspacePdfAskSelectionMenu"; | ||
| import { | ||
| WorkspacePdfCaptureInteractionMode, | ||
|
|
@@ -55,6 +59,7 @@ import { WorkspacePdfPageControl } from "#/features/workspaces/components/Worksp | |
| import { useWorkspaceViewCapabilities } from "#/features/workspaces/components/workspace-view-policy"; | ||
| import { createCaptureAttachmentFile } from "#/features/workspaces/components/workspace-region-capture"; | ||
| import { stageCaptureAttachmentToComposerWithFeedback } from "#/features/workspaces/composer/workspace-composer-actions"; | ||
| import { useWorkspacePdfPageRevealRequest } from "#/features/workspaces/locations/workspace-location-context"; | ||
| import type { WorkspaceItem } from "#/features/workspaces/model/types"; | ||
| import { getWorkspaceFileContentUrl } from "#/features/workspaces/model/workspace-file"; | ||
| import { | ||
|
|
@@ -102,11 +107,11 @@ type WorkspacePdfInteractions = { | |
|
|
||
| export default function WorkspacePdfViewer({ | ||
| item, | ||
| toolbarSlotId, | ||
| viewInstanceId, | ||
| workspaceId, | ||
| }: { | ||
| item: WorkspaceItem; | ||
| toolbarSlotId?: string; | ||
| viewInstanceId: string; | ||
| workspaceId: string; | ||
| }) { | ||
| const fileUrl = getWorkspaceFileContentUrl(workspaceId, item.id); | ||
|
|
@@ -131,7 +136,7 @@ export default function WorkspacePdfViewer({ | |
| : undefined, | ||
| fileName: item.name, | ||
| fileUrl, | ||
| slotId: toolbarSlotId ?? item.id, | ||
| slotId: viewInstanceId, | ||
| }); | ||
|
|
||
| return ( | ||
|
|
@@ -155,6 +160,7 @@ export default function WorkspacePdfViewer({ | |
| itemId={item.id} | ||
| onCaptureModeExit={exitCapture} | ||
| onCaptureModeToggle={toggleCapture} | ||
| viewInstanceId={viewInstanceId} | ||
| workspaceId={workspaceId} | ||
| /> | ||
| ) : ( | ||
|
|
@@ -177,6 +183,7 @@ function WorkspacePdfDocumentLoader({ | |
| itemId, | ||
| onCaptureModeExit, | ||
| onCaptureModeToggle, | ||
| viewInstanceId, | ||
| workspaceId, | ||
| }: { | ||
| activeDocumentId: string | null; | ||
|
|
@@ -187,14 +194,19 @@ function WorkspacePdfDocumentLoader({ | |
| itemId: string; | ||
| onCaptureModeExit: () => void; | ||
| onCaptureModeToggle: () => void; | ||
| viewInstanceId: string; | ||
| workspaceId: string; | ||
| }) { | ||
| const { provides: documentManager } = useDocumentManagerCapability(); | ||
| const { provides: scrollCapability } = useScrollCapability(); | ||
| const paneRuntime = useWorkspacePaneRuntime(); | ||
| const { consume, request } = useWorkspacePdfPageRevealRequest(viewInstanceId); | ||
| const [openError, setOpenError] = useState<{ | ||
| documentId: string; | ||
| message: string; | ||
| } | null>(null); | ||
| const currentOpenError = openError?.documentId === documentId ? openError.message : null; | ||
| const isActive = paneRuntime?.isActive ?? true; | ||
|
Comment on lines
+197
to
+209
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Check whether `@embedpdf/plugin-scroll` exposes a synchronous "current layout/ready" query
# that can be checked immediately (avoiding reliance on a fresh onLayoutReady firing).
fd -HI 'plugin-scroll' node_modules/@embedpdf 2>/dev/null
rg -n 'isInitial|onLayoutReady|getState|totalPages' node_modules/@embedpdf/plugin-scroll/dist 2>/dev/null | head -50Repository: ThinkEx-OSS/thinkex Length of output: 157 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Repo files of interest:"
git ls-files | rg '(^|/)WorkspacePdfViewer\.tsx$|WorkspaceStandardTabPanes|package\.json|yarn\.lock|pnpm-lock\.yaml|package-lock\.json'
echo
echo "Dependency versions:"
if [ -f package.json ]; then jq '.dependencies, .devDependencies' package.json | rg -n 'embedpdf|react-pdf|pdf|plugin-scroll' -C 2; fi
echo
echo "Inspect WorkspacePdfViewer outline and relevant sections:"
ast-grep outline src/features/workspaces/components/WorkspacePdfViewer.tsx 2>/dev/null || true
wc -l src/features/workspaces/components/WorkspacePdfViewer.tsx
sed -n '1,340p' src/features/workspaces/components/WorkspacePdfViewer.tsx
echo
echo "Inspect WorkspaceStandardTabPanes:"
fd -HI 'WorkspaceStandardTabPanes' . -x sh -c 'echo "FILE: $1"; wc -l "$1"; sed -n "1,220p" "$1"' sh {}
echo
echo "Search revealWorkspaceLocation and related types:"
rg -n "revealWorkspaceLocation|WorkspacePdfPage|useWorkspacePdfPageRevealRequest|onLayoutReady|viewInstanceId|handleScroll" src package.jsonRepository: ThinkEx-OSS/thinkex Length of output: 20236 🌐 Web query:
💡 Result: In the EmbedPDF viewer, the onLayoutReady event is part of the Scroll plugin and is used to perform actions—such as programmatic scrolling—once a document's layout has been fully initialized and is ready for interaction [1][2][3]. The isInitial property is a boolean flag included in the payload of the onLayoutReady event [1][4]. It serves to distinguish between different layout stages: - isInitial is true only on the first layout immediately following the initial document load [1][5]. - isInitial is false on subsequent layouts, such as when switching between tabs or re-rendering the layout [1][5]. Because documents load asynchronously, developers should use the onLayoutReady event rather than attempting to call navigation methods like scrollToPage immediately after initialization [2][6][3]. Checking that isInitial is true ensures that your logic (e.g., jumping to a specific page on load) only executes when the document is first ready [2][4][5]. This pattern was introduced as part of a update that removed the global initialPage configuration option in favor of more flexible, programmatic control [4][5]. Citations:
🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Inspect workspace navigation / revealWorkspaceLocation:"
wc -l src/features/workspaces/navigation/useWorkspaceNavigation.ts
sed -n '180,290p' src/features/workspaces/navigation/useWorkspaceNavigation.ts
echo
echo "Inspect workspace location context:"
wc -l src/features/workspaces/locations/workspace-location-context.tsx
sed -n '1,120p' src/features/workspaces/locations/workspace-location-context.tsx
echo
echo "Inspect workspace content around pdf viewer path:"
sed -n '380,530p' src/features/workspaces/components/WorkspaceContent.tsx
echo
echo "Search pdfPage references:"
rg -n "pdf-page|pdfPage|pdf-page|locationPage|pageNumber|revealWorkspaceLocation|matchingTab|activeItem" src -C 2Repository: ThinkEx-OSS/thinkex Length of output: 50375 🌐 Web query:
💡 Result: In the EmbedPDF viewer, the onLayoutReady event handler within the scroll plugin is used to handle scrolling after a document has loaded [1][2]. Because document loading is asynchronous, this event ensures that the document structure is calculated and the viewer is ready for interaction before attempting to navigate to a specific position [1][3]. The onLayoutReady event provides a payload containing documentId and isInitial properties [4][5]. The isInitial boolean is a key feature used to differentiate between the first layout following a document's initial load (where isInitial is true) and subsequent layout events, such as those triggered by switching tabs or document re-renders (where isInitial is false) [6][7]. This mechanism was introduced as part of a move to remove the global initialPage configuration option [6][7]. By checking if isInitial is true, developers can safely trigger actions—like scrolling to a specific page—only once during the initial load cycle [1][2]. Example usage in React: const unsubscribe = scrollCapability.onLayoutReady((event) => { if (event.documentId === documentId && event.isInitial) { scrollCapability.forDocument(documentId).scrollToPage({ pageNumber: initialPage, behavior: 'instant', }); } }); Citations:
🌐 Web query:
💡 Result: The terms onLayoutReady and isInitial refer to the API provided by the Citations:
🌐 Web query:
💡 Result: In the Citations:
Check EmbedPDF layout readiness synchronously before relying on For reused inactive PDF tabs, 🤖 Prompt for AI Agents |
||
|
|
||
| useEffect(() => { | ||
| if (!documentManager || documentManager.isDocumentOpen(documentId)) { | ||
|
|
@@ -240,6 +252,46 @@ function WorkspacePdfDocumentLoader({ | |
| }; | ||
| }, [documentId, documentManager, fileName, fileUrl]); | ||
|
|
||
| useEffect(() => { | ||
| if (!request) { | ||
| return; | ||
| } | ||
|
|
||
| if (currentOpenError || request.location.itemId !== documentId) { | ||
| consume(request); | ||
| return; | ||
| } | ||
|
|
||
| if (!isActive || activeDocumentId !== documentId || !scrollCapability) { | ||
| return; | ||
| } | ||
|
|
||
| let handled = false; | ||
|
|
||
| return scrollCapability.onLayoutReady((event) => { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a citation targets a PDF that is already open and laid out, this effect subscribes to Useful? React with 👍 / 👎. |
||
| if (handled || event.documentId !== documentId) { | ||
| return; | ||
| } | ||
|
|
||
| handled = true; | ||
| if (request.location.pageNumber <= event.totalPages) { | ||
| scrollCapability.forDocument(documentId).scrollToPage({ | ||
| behavior: "instant", | ||
| pageNumber: request.location.pageNumber, | ||
| }); | ||
| } | ||
| consume(request); | ||
| }); | ||
| }, [ | ||
| activeDocumentId, | ||
| consume, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: The Prompt for AI agents |
||
| currentOpenError, | ||
| documentId, | ||
| isActive, | ||
| request, | ||
| scrollCapability, | ||
| ]); | ||
|
|
||
| if (currentOpenError) { | ||
| return ( | ||
| <WorkspacePdfLoadFailure fileName={fileName} fileUrl={fileUrl}> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: Citations opened while the workspace is in split presentation will not reveal the page in the target pane: this line identifies the PDF viewer with
pane.id, but the navigation callback stores a workspace tab ID in the reveal request. The request consequently matches no mountedWorkspacePdfViewer. The navigation/reveal flow would need to resolve and return the actual rendered pane instance (or otherwise map the tab to that pane) for split/maximized presentations.Prompt for AI agents