diff --git a/frontend/editor/public/locales/en-GB/translation.toml b/frontend/editor/public/locales/en-GB/translation.toml index dba8fa41cc..67779cc07d 100644 --- a/frontend/editor/public/locales/en-GB/translation.toml +++ b/frontend/editor/public/locales/en-GB/translation.toml @@ -8003,6 +8003,8 @@ zoomIn = "Zoom In" zoomOut = "Zoom Out" [viewer.attachments] +addAttachment = "Add attachment" +close = "Close attachments" empty = "No attachments in this document" loading = "Loading attachments..." noDocument = "Open a PDF to view its attachments." @@ -8016,6 +8018,7 @@ addComment = "Add comment" addCommentPlaceholder = "Add comment..." addLink = "Add link" addReplyPlaceholder = "Add reply..." +close = "Close comments" deleteAnnotationAndComment = "Delete annotation & comment" deleteDescription = "This annotation has a comment attached. You can remove just the comment from the sidebar while keeping the annotation, or delete everything." deleteTitle = "Remove annotation from comments?" @@ -8026,6 +8029,7 @@ moreActions = "More actions" nComments_one = "{{count}} comment" nComments_other = "{{count}} comments" pageLabel = "Page {{page}}" +placingHint = "Click a page to place… (cancel)" removeCommentOnly = "Remove comment only" saveReply = "Save reply" title = "Comments" diff --git a/frontend/editor/public/locales/en-US/translation.toml b/frontend/editor/public/locales/en-US/translation.toml index 5440077934..c549a6f9a1 100644 --- a/frontend/editor/public/locales/en-US/translation.toml +++ b/frontend/editor/public/locales/en-US/translation.toml @@ -8027,6 +8027,8 @@ zoomIn = "Zoom In" zoomOut = "Zoom Out" [viewer.attachments] +addAttachment = "Add attachment" +close = "Close attachments" empty = "No attachments in this document" loading = "Loading attachments..." noDocument = "Open a PDF to view its attachments." @@ -8044,6 +8046,7 @@ cancelClearAll = "Cancel" clearAll = "Clear all comments" clearAllDescription = "This removes comments and replies from the sidebar while keeping any attached annotations in the document." clearAllTitle = "Clear all comments?" +close = "Close comments" deleteAnnotationAndComment = "Delete annotation & comment" deleteDescription = "This annotation has a comment attached. You can remove just the comment from the sidebar while keeping the annotation, or delete everything." deleteTitle = "Remove annotation from comments?" @@ -8054,6 +8057,7 @@ moreActions = "More actions" nComments_one = "{{count}} comment" nComments_other = "{{count}} comments" pageLabel = "Page {{page}}" +placingHint = "Click a page to place… (cancel)" removeCommentOnly = "Remove comment only" saveReply = "Save reply" title = "Comments" diff --git a/frontend/editor/src/core/components/shared/FileSidebar.tsx b/frontend/editor/src/core/components/shared/FileSidebar.tsx index 3a14b0fb59..1eb51b759b 100644 --- a/frontend/editor/src/core/components/shared/FileSidebar.tsx +++ b/frontend/editor/src/core/components/shared/FileSidebar.tsx @@ -989,6 +989,35 @@ const FileSidebar = forwardRef( ) : filteredFileStubs.length > 0 ? (
+ {filteredFileStubs.map((stub) => { const workbenchFileId = state.files.ids.find( (id) => (id as string) === (stub.id as string), diff --git a/frontend/editor/src/core/components/viewer/AttachmentSidebar.tsx b/frontend/editor/src/core/components/viewer/AttachmentSidebar.tsx index 29f9806342..a858498bb6 100644 --- a/frontend/editor/src/core/components/viewer/AttachmentSidebar.tsx +++ b/frontend/editor/src/core/components/viewer/AttachmentSidebar.tsx @@ -4,12 +4,14 @@ import { ScrollArea, Text, ActionIcon, + Button, Loader, Stack, TextInput, } from "@mantine/core"; import LocalIcon from "@app/components/shared/LocalIcon"; import { useViewer } from "@app/contexts/ViewerContext"; +import { useToolWorkflow } from "@app/contexts/ToolWorkflowContext"; import { PdfAttachmentObject } from "@embedpdf/models"; import AttachmentIcon from "@mui/icons-material/AttachmentRounded"; import DownloadIcon from "@mui/icons-material/DownloadRounded"; @@ -52,7 +54,9 @@ export const AttachmentSidebar = ({ preloadCacheKeys = [], }: AttachmentSidebarProps) => { const { t } = useTranslation(); - const { attachmentActions, hasAttachmentSupport } = useViewer(); + const { attachmentActions, hasAttachmentSupport, toggleAttachmentSidebar } = + useViewer(); + const { handleToolSelectForced } = useToolWorkflow(); const [searchTerm, setSearchTerm] = useState(""); const [attachmentSupport, setAttachmentSupport] = useState(() => hasAttachmentSupport(), @@ -139,16 +143,28 @@ export const AttachmentSidebar = ({ const key = documentCacheKey; const cached = cacheRef.current.get(key); - if ( - cached && - (cached.status === "loading" || cached.status === "success") - ) { + // Only short-circuit on a finalised success cache. Skipping when + // cached.status === "loading" caused the sidebar to get stuck: if + // the previous fetch was cancelled (by a parent re-render that + // changed the attachmentActions reference - createViewerActions + // builds a new object every viewer render), the cache still says + // "loading" but no live fetch is in flight. On the re-run we'd + // early-return and never refetch, so the UI would sit on the + // "Loading attachments..." state forever. Same change applied in + // BookmarkSidebar. + if (cached && cached.status === "success") { return; } let cancelled = false; + // Don't write "loading" into the cache - keep the cache for + // terminal states (success/error) only, so a cancelled run can + // never leave a stale "loading" entry behind. The visible + // sidebar state still goes through setActiveEntry below. const updateEntry = (entry: AttachmentCacheEntry) => { - cacheRef.current.set(key, entry); + if (entry.status === "success" || entry.status === "error") { + cacheRef.current.set(key, entry); + } if (!cancelled && currentKeyRef.current === key) { setActiveEntry(entry); } @@ -163,10 +179,20 @@ export const AttachmentSidebar = ({ ); const fetchWithRetry = async () => { - const maxAttempts = 10; + // See BookmarkSidebar - matching change. After a file swap the + // attachment bridge briefly unregisters and the action returns + // null until the new document is loaded; without retrying on + // null we'd cache an empty success and miss freshly-added + // attachments. + const maxAttempts = 30; for (let attempt = 0; attempt < maxAttempts; attempt++) { try { const result = await attachmentActions.getAttachments(); + if (result === null) { + if (attempt === maxAttempts - 1) return []; + await new Promise((resolve) => setTimeout(resolve, 50)); + continue; + } return Array.isArray(result) ? result : []; } catch (error: any) { const message = @@ -239,6 +265,14 @@ export const AttachmentSidebar = ({ attachmentActions.downloadAttachment(attachment); }; + const handleAddAttachment = useCallback(() => { + // Close the attachment sidebar before opening the tool so the user + // doesn't end up looking at two stacked side panels (the sidebar on + // the right + the tool's settings on the left). + toggleAttachmentSidebar(); + handleToolSelectForced("addAttachments"); + }, [handleToolSelectForced, toggleAttachmentSidebar]); + const filteredAttachments = useMemo(() => { const attachments = Array.isArray(activeEntry.attachments) ? activeEntry.attachments @@ -352,6 +386,18 @@ export const AttachmentSidebar = ({ {t("viewer.attachments.title", "Attachments")}
+ + + + + + + {t( "viewer.attachments.empty", "No attachments in this document", )} - + + )} {showAttachmentList && ( -
- {renderAttachments(filteredAttachments)} -
+ <> + +
+ {renderAttachments(filteredAttachments)} +
+ )} {showSearchEmpty && ( diff --git a/frontend/editor/src/core/components/viewer/BookmarkSidebar.tsx b/frontend/editor/src/core/components/viewer/BookmarkSidebar.tsx index c87c783bdf..40bd9d3045 100644 --- a/frontend/editor/src/core/components/viewer/BookmarkSidebar.tsx +++ b/frontend/editor/src/core/components/viewer/BookmarkSidebar.tsx @@ -7,10 +7,18 @@ import { Loader, Stack, TextInput, + NumberInput, Button, + Group, + UnstyledButton, } from "@mantine/core"; import LocalIcon from "@app/components/shared/LocalIcon"; import { useViewer } from "@app/contexts/ViewerContext"; +import { useToolWorkflow } from "@app/contexts/ToolWorkflowContext"; +import { useFileContext } from "@app/contexts/FileContext"; +import { isStirlingFile, type FileId } from "@app/types/fileContext"; +import { createStirlingFilesAndStubs } from "@app/services/fileStubHelpers"; +import apiClient from "@app/services/apiClient"; import { PdfBookmarkObject, PdfActionType } from "@embedpdf/models"; import BookmarksIcon from "@mui/icons-material/BookmarksRounded"; import "@app/components/viewer/SidebarBase.css"; @@ -75,9 +83,25 @@ export const BookmarkSidebar = ({ documentCacheKey, preloadCacheKeys = [], }: BookmarkSidebarProps) => { - const { bookmarkActions, scrollActions, hasBookmarkSupport } = useViewer(); + const { + bookmarkActions, + scrollActions, + hasBookmarkSupport, + activeFileId, + activeFileIndex, + setActiveFileId, + getScrollState, + toggleBookmarkSidebar, + } = useViewer(); + const { handleToolSelectForced } = useToolWorkflow(); + const { selectors, actions: fileActions } = useFileContext(); const [expanded, setExpanded] = useState>({}); const [searchTerm, setSearchTerm] = useState(""); + const [isAddingBookmark, setIsAddingBookmark] = useState(false); + const [newBookmarkTitle, setNewBookmarkTitle] = useState(""); + const [newBookmarkPage, setNewBookmarkPage] = useState(1); + const [isSavingBookmark, setIsSavingBookmark] = useState(false); + const [addBookmarkError, setAddBookmarkError] = useState(null); const [bookmarkSupport, setBookmarkSupport] = useState(() => hasBookmarkSupport(), ); @@ -164,16 +188,23 @@ export const BookmarkSidebar = ({ const key = documentCacheKey; const cached = cacheRef.current.get(key); - if ( - cached && - (cached.status === "loading" || cached.status === "success") - ) { + // Only short-circuit on a finalised success cache. Skipping when + // cached.status === "loading" causes the sidebar to get stuck if + // the previous fetch was cancelled by a parent re-render (the + // bookmarkActions reference changes every viewer render because + // createViewerActions rebuilds the object). See matching change + // in AttachmentSidebar. + if (cached && cached.status === "success") { return; } let cancelled = false; + // Don't write "loading" into the cache - cache only terminal + // states so a cancelled run can't poison the cache. const updateEntry = (entry: BookmarkCacheEntry) => { - cacheRef.current.set(key, entry); + if (entry.status === "success" || entry.status === "error") { + cacheRef.current.set(key, entry); + } if (!cancelled && currentKeyRef.current === key) { setActiveEntry(entry); } @@ -188,10 +219,24 @@ export const BookmarkSidebar = ({ ); const fetchWithRetry = async () => { - const maxAttempts = 10; + // 30 × 50ms = 1.5s window. After consumeFiles swaps the file the + // embedpdf bookmark plugin tears down for the old document and + // re-registers for the new one; until the bridge is back the + // action returns null. Without retrying on null we'd cache an + // empty "success" and the just-added bookmark would never show + // up in the sidebar. + const maxAttempts = 30; for (let attempt = 0; attempt < maxAttempts; attempt++) { try { const result = await bookmarkActions.fetchBookmarks(); + if (result === null) { + // Bridge not registered yet (document still loading). Wait + // and retry instead of caching this as a successful empty + // list. + if (attempt === maxAttempts - 1) return []; + await new Promise((resolve) => setTimeout(resolve, 50)); + continue; + } return Array.isArray(result) ? result : []; } catch (error: any) { const message = @@ -256,6 +301,143 @@ export const BookmarkSidebar = ({ setFetchNonce((value) => value + 1); }, [documentCacheKey, bookmarkActions]); + const handleOpenAddBookmark = useCallback(() => { + setAddBookmarkError(null); + setNewBookmarkTitle(""); + // Default the new bookmark's target page to whatever page the user is + // currently viewing - matches Acrobat / Foxit behaviour. + const currentPage = getScrollState?.()?.currentPage ?? 1; + setNewBookmarkPage(currentPage); + setIsAddingBookmark(true); + }, [getScrollState]); + + const handleCancelAddBookmark = useCallback(() => { + setIsAddingBookmark(false); + setAddBookmarkError(null); + setNewBookmarkTitle(""); + }, []); + + // Fallback: open the full Edit Table of Contents tool when inline add is + // not viable (e.g. the active file is a preview / unmanaged file we + // cannot consume + replace via FileContext). + const handleFallbackToTool = useCallback(() => { + handleToolSelectForced("editTableOfContents"); + }, [handleToolSelectForced]); + + const handleSubmitAddBookmark = useCallback(async () => { + const title = newBookmarkTitle.trim(); + if (!title) { + setAddBookmarkError("Bookmark title is required"); + return; + } + // Resolve the file the viewer is currently displaying. activeFileId + // is only set explicitly (user clicked a thumbnail / a tool ran); + // on a fresh /read upload it stays null and the viewer falls back + // to activeFileIndex - so we mirror that here. Without this, Save + // would silently route to the full editor every time on a fresh + // upload. + const allFiles = selectors.getFiles(); + const resolvedFile = activeFileId + ? allFiles.find((f) => isStirlingFile(f) && f.fileId === activeFileId) + : (allFiles[activeFileIndex] ?? allFiles[0]); + const resolvedFileId = + resolvedFile && isStirlingFile(resolvedFile) + ? (resolvedFile.fileId as FileId) + : null; + if (!resolvedFileId) { + handleFallbackToTool(); + return; + } + const fileId = resolvedFileId; + const file = selectors.getFile(fileId); + const parentStub = selectors.getStirlingFileStub(fileId); + if (!file || !parentStub) { + handleFallbackToTool(); + return; + } + + setIsSavingBookmark(true); + setAddBookmarkError(null); + try { + // Convert existing PDF bookmarks (from embedpdf) to the backend's + // payload shape, then append the new one. + const toPayload = ( + b: PdfBookmarkObject, + ): { + title: string; + pageNumber: number; + children: any[]; + } => ({ + title: b.title ?? "", + pageNumber: resolvePageNumber(b) ?? 1, + children: (b.children ?? []).map(toPayload), + }); + const existing = (activeEntry.bookmarks ?? []).map(toPayload); + const bookmarkData = [ + ...existing, + { title, pageNumber: newBookmarkPage, children: [] }, + ]; + + const formData = new FormData(); + formData.append("fileInput", file); + formData.append("replaceExisting", "true"); + formData.append("bookmarkData", JSON.stringify(bookmarkData)); + + const response = await apiClient.post( + "/api/v1/general/edit-table-of-contents", + formData, + { responseType: "blob" }, + ); + + const newFile = new File([response.data as Blob], file.name, { + type: "application/pdf", + }); + const { stirlingFiles, stubs } = await createStirlingFilesAndStubs( + [newFile], + parentStub, + "editTableOfContents", + ); + const outputFileIds = await fileActions.consumeFiles( + [fileId], + stirlingFiles, + stubs, + ); + + // Point the viewer at the new file. Without this the viewer's + // activeFileId-removed effect nulls activeFileId (old file is + // gone) and the activeFileIndex falls back to 0, which races + // against the embedpdf plugin reloading - the bookmark / + // attachment bridges can end up stuck in a "loading" state. + // useToolOperation does the same thing after consumeFiles. + if (outputFileIds.length === 1) { + setActiveFileId(outputFileIds[0]); + } + + // Reset form. The cache is keyed by documentCacheKey (== fileId); + // the new fileId triggers our document-switch effect, which + // resets state and re-fetches once the embedpdf bookmark + // capability has the new document loaded. + setIsAddingBookmark(false); + setNewBookmarkTitle(""); + } catch (error) { + const message = + error instanceof Error ? error.message : "Failed to save bookmark"; + setAddBookmarkError(message); + } finally { + setIsSavingBookmark(false); + } + }, [ + newBookmarkTitle, + newBookmarkPage, + activeFileId, + activeFileIndex, + selectors, + fileActions, + setActiveFileId, + activeEntry.bookmarks, + handleFallbackToTool, + ]); + const bookmarksWithIds = useMemo(() => { const assignIds = ( nodes: PdfBookmarkObject[], @@ -293,34 +475,6 @@ export const BookmarkSidebar = ({ })); }; - const expandAll = useCallback(() => { - const allExpanded: Record = {}; - const expandRecursive = (nodes: BookmarkNode[]) => { - nodes.forEach((node) => { - if (node.children && node.children.length > 0) { - allExpanded[node.id] = true; - expandRecursive(node.children as BookmarkNode[]); - } - }); - }; - expandRecursive(bookmarksWithIds); - setExpanded(allExpanded); - }, [bookmarksWithIds]); - - const collapseAll = useCallback(() => { - const allCollapsed: Record = {}; - const collapseRecursive = (nodes: BookmarkNode[]) => { - nodes.forEach((node) => { - if (node.children && node.children.length > 0) { - allCollapsed[node.id] = false; - collapseRecursive(node.children as BookmarkNode[]); - } - }); - }; - collapseRecursive(bookmarksWithIds); - setExpanded(allCollapsed); - }, [bookmarksWithIds]); - const handleBookmarkClick = ( bookmark: PdfBookmarkObject, event: React.MouseEvent, @@ -499,31 +653,18 @@ export const BookmarkSidebar = ({ Bookmarks - {bookmarkSupport && bookmarksWithIds.length > 0 && ( - <> - {Object.values(expanded).some((val) => val === false) ? ( - - - - ) : ( - - - - )} - - )} + + + + + )} - {showEmptyState && ( -
+ {showEmptyState && !isAddingBookmark && ( + + No bookmarks in this document -
+ + + )} + + {isAddingBookmark && ( + + + + Add bookmark + + setNewBookmarkTitle(e.currentTarget.value)} + autoFocus + disabled={isSavingBookmark} + /> + + setNewBookmarkPage(typeof v === "number" ? v : 1) + } + disabled={isSavingBookmark} + /> + {addBookmarkError && ( + + {addBookmarkError} + + )} + + + + + + )} {showBookmarkList && ( -
- {renderBookmarks(filteredBookmarks)} -
+ <> + {!isAddingBookmark && ( + + )} +
+ {renderBookmarks(filteredBookmarks)} +
+ )} {showSearchEmpty && ( @@ -609,6 +851,41 @@ export const BookmarkSidebar = ({ )} + + {bookmarkSupport && documentCacheKey && ( + + + + + + Need to reorder or nest? Open the Bookmark Editor + + + + + )} ); }; diff --git a/frontend/editor/src/core/components/viewer/CommentsSidebar.tsx b/frontend/editor/src/core/components/viewer/CommentsSidebar.tsx index f74ad806c6..cc470b3ea6 100644 --- a/frontend/editor/src/core/components/viewer/CommentsSidebar.tsx +++ b/frontend/editor/src/core/components/viewer/CommentsSidebar.tsx @@ -311,11 +311,17 @@ export function CommentsSidebar({ clearHighlightCommentRequest, scrollActions, getZoomState, + toggleCommentsSidebar, } = useViewer() ?? {}; const scrollViewportRef = useRef(null); const { state, provides } = useAnnotation(documentId); const { handleToolSelectForced } = useToolWorkflow(); - const { activateAnnotationToolRef } = useAnnotationContext(); + const { + activateAnnotationToolRef, + activeAnnotationToolId, + setActiveAnnotationToolId, + } = useAnnotationContext(); + const isPlacingComment = activeAnnotationToolId === TEXT_COMMENT_TOOL_ID; const [draftContents, setDraftContents] = useState>( {}, ); @@ -639,12 +645,34 @@ export function CommentsSidebar({ ); const handleAddComment = useCallback(() => { + // Keep the sidebar open this time - the button morphs into a + // "Click on a page... cancel" hint so the user can see exactly + // what state the viewer is in. handleToolSelectForced(ANNOTATE_PANEL_ID); requestAnimationFrame(() => { activateAnnotationToolRef.current?.(TEXT_COMMENT_TOOL_ID); }); }, [handleToolSelectForced, activateAnnotationToolRef]); + const handleCancelPlacingComment = useCallback(() => { + // De-arm the textComment tool. The panel's activateAnnotationTool + // takes the AnnotationToolId "select" to reset to no-tool state. + activateAnnotationToolRef.current?.("select" as never); + setActiveAnnotationToolId(null); + }, [activateAnnotationToolRef, setActiveAnnotationToolId]); + + // ESC cancels placement mode while the sidebar is open. + useEffect(() => { + if (!visible || !isPlacingComment) return; + const onKeyDown = (e: KeyboardEvent) => { + if (e.key === "Escape") { + handleCancelPlacingComment(); + } + }; + window.addEventListener("keydown", onKeyDown); + return () => window.removeEventListener("keydown", onKeyDown); + }, [visible, isPlacingComment, handleCancelPlacingComment]); + if (!visible) return null; return ( @@ -716,6 +744,18 @@ export function CommentsSidebar({ )} + {toggleCommentsSidebar && ( + + + + )} @@ -733,434 +773,517 @@ export function CommentsSidebar({ "Place comments with the Comment, Insert Text, or Replace Text tools. They will appear here by page.", )} - + {isPlacingComment ? ( + + ) : ( + + )} ) : ( - pageNumbers.map((pageIndex) => { - const entries = byPage[pageIndex] ?? []; - const pageNum = pageIndex + 1; - return ( - - - {t("viewer.comments.pageLabel", "Page {{page}}", { - page: pageNum, - })} - - - {t("viewer.comments.nComments", "{{count}} comment(s)", { - count: entries.length, - })} - - - - {entries.map((entry) => { - const ann = entry.annotation?.object; - const id = ann?.id; - if (!id) return null; - const key = `${pageIndex}_${id}`; - const replyKey = `${pageIndex}_${id}_reply`; - const displayContent = getCommentDisplayContent(entry); - const draft = - draftContents[key] !== undefined - ? draftContents[key] - : displayContent; - const replyDraft = replyDrafts[replyKey] ?? ""; - const authorName = getAuthorName(ann, displayName); - /** Only treat as "comment posted" when annotation actually has content (user clicked Send), not on every keystroke. */ - const hasMainContent = - (displayContent ?? "").trim().length > 0; - const isEditingMain = editingMainKey === key; + <> + {isPlacingComment ? ( + + ) : ( + + )} + {pageNumbers.map((pageIndex) => { + const entries = byPage[pageIndex] ?? []; + const pageNum = pageIndex + 1; + return ( + + + {t("viewer.comments.pageLabel", "Page {{page}}", { + page: pageNum, + })} + + + {t("viewer.comments.nComments", "{{count}} comments", { + count: entries.length, + })} + + + + {entries.map((entry) => { + const ann = entry.annotation?.object; + const id = ann?.id; + if (!id) return null; + const key = `${pageIndex}_${id}`; + const replyKey = `${pageIndex}_${id}_reply`; + const displayContent = getCommentDisplayContent(entry); + const draft = + draftContents[key] !== undefined + ? draftContents[key] + : displayContent; + const replyDraft = replyDrafts[replyKey] ?? ""; + const authorName = getAuthorName(ann, displayName); + /** Only treat as "comment posted" when annotation actually has content (user clicked Send), not on every keystroke. */ + const hasMainContent = + (displayContent ?? "").trim().length > 0; + const isEditingMain = editingMainKey === key; - const mainTimestamp = formatCommentDate(ann); - const typeLabel = getAnnotationTypeLabel(ann, t); + const mainTimestamp = formatCommentDate(ann); + const typeLabel = getAnnotationTypeLabel(ann, t); - return ( - - - - - - {authorName} - - - {typeLabel} - {mainTimestamp ? ` · ${mainTimestamp}` : ""} - - - - - + + + + {authorName} + + + {typeLabel} + {mainTimestamp ? ` · ${mainTimestamp}` : ""} + + + + - - handleLocateAnnotation(pageIndex, ann) - } - > - - - - - - - - - - - - - - } - onClick={() => setEditingMainKey(key)} - > - {t("annotation.editText", "Edit")} - - - } - color="red" - onClick={() => - handleDeleteClick(pageIndex, id, ann) - } - > - {t("annotation.delete", "Delete")} - - - - - - - {!hasMainContent || isEditingMain ? ( - <> -