From e142af2863052010f1e309bcfb6450ab3ecc775d Mon Sep 17 00:00:00 2001 From: James Brunton Date: Thu, 28 Aug 2025 10:56:07 +0100 Subject: [PATCH] V2 Make FileId type opaque and use consistently throughout project (#4307) # Description of Changes The `FileId` type in V2 currently is just defined to be a string. This makes it really easy to accidentally pass strings into things accepting file IDs (such as file names). This PR makes the `FileId` type [an opaque type](https://www.geeksforgeeks.org/typescript/opaque-types-in-typescript/), so it is compatible with things accepting strings (arguably not ideal for this...) but strings are not compatible with it without explicit conversion. The PR also includes changes to use `FileId` consistently throughout the project (everywhere I could find uses of `fileId: string`), so that we have the maximum benefit from the type safety. > [!note] > I've marked quite a few things as `FIX ME` where we're passing names in as IDs. If that is intended behaviour, I'm happy to remove the fix me and insert a cast instead, but they probably need comments explaining why we're using a file name as an ID. --- .../src/components/fileEditor/FileEditor.tsx | 69 ++++++----- .../fileEditor/FileEditorThumbnail.tsx | 23 ++-- .../history/FileOperationHistory.tsx | 9 +- .../components/pageEditor/FileThumbnail.tsx | 17 +-- .../src/components/pageEditor/PageEditor.tsx | 70 ++++++----- .../pageEditor/commands/pageCommands.ts | 103 +++++++-------- .../pageEditor/hooks/usePageDocument.ts | 47 +++---- frontend/src/components/shared/FileGrid.tsx | 9 +- .../src/components/shared/FilePickerModal.tsx | 59 ++++----- .../tools/convert/ConvertSettings.tsx | 51 ++++---- frontend/src/components/viewer/Viewer.tsx | 5 +- frontend/src/contexts/FileContext.tsx | 61 +++++---- frontend/src/contexts/FileManagerContext.tsx | 29 ++--- frontend/src/contexts/FilesModalContext.tsx | 5 +- frontend/src/contexts/IndexedDBContext.tsx | 24 ++-- frontend/src/contexts/file/FileReducer.ts | 68 +++++----- frontend/src/contexts/file/fileActions.ts | 117 +++++++++--------- frontend/src/contexts/file/fileHooks.ts | 39 +++--- frontend/src/contexts/file/fileSelectors.ts | 42 +++---- frontend/src/contexts/file/lifecycle.ts | 29 ++--- .../hooks/tools/shared/useToolOperation.ts | 3 +- frontend/src/hooks/useFileHandler.ts | 9 +- frontend/src/hooks/useFileManager.ts | 35 +++--- frontend/src/hooks/useThumbnailGeneration.ts | 51 ++++---- frontend/src/hooks/useToolManagement.tsx | 7 +- .../src/services/fileProcessingService.ts | 27 ++-- frontend/src/services/fileStorage.ts | 23 ++-- .../services/thumbnailGenerationService.ts | 67 +++++----- frontend/src/types/file.ts | 12 +- frontend/src/types/fileContext.ts | 55 ++++---- frontend/src/types/pageEditor.ts | 4 +- frontend/src/utils/toolOperationTracker.ts | 5 +- 32 files changed, 600 insertions(+), 574 deletions(-) diff --git a/frontend/src/components/fileEditor/FileEditor.tsx b/frontend/src/components/fileEditor/FileEditor.tsx index eee1eb1a7..d531db8a3 100644 --- a/frontend/src/components/fileEditor/FileEditor.tsx +++ b/frontend/src/components/fileEditor/FileEditor.tsx @@ -16,6 +16,7 @@ import styles from './FileEditor.module.css'; import FileEditorThumbnail from './FileEditorThumbnail'; import FilePickerModal from '../shared/FilePickerModal'; import SkeletonLoader from '../shared/SkeletonLoader'; +import { FileId } from '../../types/file'; interface FileEditorProps { @@ -46,17 +47,17 @@ const FileEditor = ({ // Use optimized FileContext hooks const { state, selectors } = useFileState(); const { addFiles, removeFiles, reorderFiles } = useFileManagement(); - + // Extract needed values from state (memoized to prevent infinite loops) const activeFiles = useMemo(() => selectors.getFiles(), [selectors.getFilesSignature()]); const activeFileRecords = useMemo(() => selectors.getFileRecords(), [selectors.getFilesSignature()]); const selectedFileIds = state.ui.selectedFileIds; const isProcessing = state.ui.isProcessing; - + // Get the real context actions const { actions } = useFileActions(); const { actions: navActions } = useNavigationActions(); - + // Get file selection context const { setSelectedFiles } = useFileSelection(); @@ -86,9 +87,9 @@ const FileEditor = ({ }); // Get selected file IDs from context (defensive programming) const contextSelectedIds = Array.isArray(selectedFileIds) ? selectedFileIds : []; - + // Create refs for frequently changing values to stabilize callbacks - const contextSelectedIdsRef = useRef([]); + const contextSelectedIdsRef = useRef([]); contextSelectedIdsRef.current = contextSelectedIds; // Use activeFileRecords directly - no conversion needed @@ -98,7 +99,7 @@ const FileEditor = ({ const recordToFileItem = useCallback((record: any) => { const file = selectors.getFile(record.id); if (!file) return null; - + return { id: record.id, name: file.name, @@ -166,7 +167,7 @@ const FileEditor = ({ id: operationId, type: 'convert', timestamp: Date.now(), - fileIds: extractionResult.extractedFiles.map(f => f.name), + fileIds: extractionResult.extractedFiles.map(f => f.name) as FileId[] /* FIX ME: This doesn't seem right */, status: 'pending', metadata: { originalFileName: file.name, @@ -179,7 +180,7 @@ const FileEditor = ({ } } }; - + if (extractionResult.errors.length > 0) { errors.push(...extractionResult.errors); } @@ -219,7 +220,7 @@ const FileEditor = ({ id: operationId, type: 'upload', timestamp: Date.now(), - fileIds: [file.name], + fileIds: [file.name as FileId /* This doesn't seem right */], status: 'pending', metadata: { originalFileName: file.name, @@ -239,7 +240,7 @@ const FileEditor = ({ const errorMessage = err instanceof Error ? err.message : 'Failed to process files'; setError(errorMessage); console.error('File processing error:', err); - + // Reset extraction progress on error setZipExtractionProgress({ isExtracting: false, @@ -263,21 +264,21 @@ const FileEditor = ({ // Remove all files from context but keep in storage const allFileIds = activeFileRecords.map(record => record.id); removeFiles(allFileIds, false); // false = keep in storage - + // Clear selections setSelectedFiles([]); }, [activeFileRecords, removeFiles, setSelectedFiles]); - const toggleFile = useCallback((fileId: string) => { + const toggleFile = useCallback((fileId: FileId) => { const currentSelectedIds = contextSelectedIdsRef.current; - + const targetRecord = activeFileRecords.find(r => r.id === fileId); if (!targetRecord) return; const contextFileId = fileId; // No need to create a new ID const isSelected = currentSelectedIds.includes(contextFileId); - let newSelection: string[]; + let newSelection: FileId[]; if (isSelected) { // Remove file from selection @@ -286,7 +287,7 @@ const FileEditor = ({ // Add file to selection // In tool mode, typically allow multiple files unless specified otherwise const maxAllowed = toolMode ? 10 : Infinity; // Default max for tools - + if (maxAllowed === 1) { newSelection = [contextFileId]; } else { @@ -314,30 +315,30 @@ const FileEditor = ({ }, [setSelectedFiles]); // File reordering handler for drag and drop - const handleReorderFiles = useCallback((sourceFileId: string, targetFileId: string, selectedFileIds: string[]) => { + const handleReorderFiles = useCallback((sourceFileId: FileId, targetFileId: FileId, selectedFileIds: FileId[]) => { const currentIds = activeFileRecords.map(r => r.id); - + // Find indices const sourceIndex = currentIds.findIndex(id => id === sourceFileId); const targetIndex = currentIds.findIndex(id => id === targetFileId); - + if (sourceIndex === -1 || targetIndex === -1) { console.warn('Could not find source or target file for reordering'); return; } // Handle multi-file selection reordering - const filesToMove = selectedFileIds.length > 1 + const filesToMove = selectedFileIds.length > 1 ? selectedFileIds.filter(id => currentIds.includes(id)) : [sourceFileId]; // Create new order const newOrder = [...currentIds]; - + // Remove files to move from their current positions (in reverse order to maintain indices) const sourceIndices = filesToMove.map(id => newOrder.findIndex(nId => nId === id)) .sort((a, b) => b - a); // Sort descending - + sourceIndices.forEach(index => { newOrder.splice(index, 1); }); @@ -372,7 +373,7 @@ const FileEditor = ({ // File operations using context - const handleDeleteFile = useCallback((fileId: string) => { + const handleDeleteFile = useCallback((fileId: FileId) => { const record = activeFileRecords.find(r => r.id === fileId); const file = record ? selectors.getFile(record.id) : null; @@ -385,7 +386,7 @@ const FileEditor = ({ id: operationId, type: 'remove', timestamp: Date.now(), - fileIds: [fileName], + fileIds: [fileName as FileId /* FIX ME: This doesn't seem right */], status: 'pending', metadata: { originalFileName: fileName, @@ -396,7 +397,7 @@ const FileEditor = ({ } } }; - + // Remove file from context but keep in storage (close, don't delete) removeFiles([contextFileId], false); @@ -406,7 +407,7 @@ const FileEditor = ({ } }, [activeFileRecords, selectors, removeFiles, setSelectedFiles, selectedFileIds]); - const handleViewFile = useCallback((fileId: string) => { + const handleViewFile = useCallback((fileId: FileId) => { const record = activeFileRecords.find(r => r.id === fileId); if (record) { // Set the file as selected in context and switch to viewer for preview @@ -415,7 +416,7 @@ const FileEditor = ({ } }, [activeFileRecords, setSelectedFiles, navActions.setMode]); - const handleMergeFromHere = useCallback((fileId: string) => { + const handleMergeFromHere = useCallback((fileId: FileId) => { const startIndex = activeFileRecords.findIndex(r => r.id === fileId); if (startIndex === -1) return; @@ -426,14 +427,14 @@ const FileEditor = ({ } }, [activeFileRecords, selectors, onMergeFiles]); - const handleSplitFile = useCallback((fileId: string) => { + const handleSplitFile = useCallback((fileId: FileId) => { const file = selectors.getFile(fileId); if (file && onOpenPageEditor) { onOpenPageEditor(file); } }, [selectors, onOpenPageEditor]); - const handleLoadFromStorage = useCallback(async (selectedFiles: any[]) => { + const handleLoadFromStorage = useCallback(async (selectedFiles: File[]) => { if (selectedFiles.length === 0) return; try { @@ -513,11 +514,11 @@ const FileEditor = ({ ) : ( -
{ const fileItem = recordToFileItem(record); if (!fileItem) return null; - + return ( void; - onDeleteFile: (fileId: string) => void; - onViewFile: (fileId: string) => void; + onToggleFile: (fileId: FileId) => void; + onDeleteFile: (fileId: FileId) => void; + onViewFile: (fileId: FileId) => void; onSetStatus: (status: string) => void; - onReorderFiles?: (sourceFileId: string, targetFileId: string, selectedFileIds: string[]) => void; - onDownloadFile?: (fileId: string) => void; + onReorderFiles?: (sourceFileId: FileId, targetFileId: FileId, selectedFileIds: FileId[]) => void; + onDownloadFile?: (fileId: FileId) => void; toolMode?: boolean; isSupported?: boolean; } @@ -161,8 +162,8 @@ const FileEditorThumbnail = ({ onDrop: ({ source }) => { const sourceData = source.data; if (sourceData.type === 'file' && onReorderFiles) { - const sourceFileId = sourceData.fileId as string; - const selectedFileIds = sourceData.selectedFiles as string[]; + const sourceFileId = sourceData.fileId as FileId; + const selectedFileIds = sourceData.selectedFiles as FileId[]; onReorderFiles(sourceFileId, file.id, selectedFileIds); } } @@ -332,7 +333,7 @@ const FileEditorThumbnail = ({ )} {/* Title + meta line */} -
= ({ maxHeight = 400 }) => { // These were stub functions in the old context - replace with empty stubs - const getFileHistory = (fileId: string) => ({ operations: [], createdAt: Date.now(), lastModified: Date.now() }); - const getAppliedOperations = (fileId: string) => []; + const getFileHistory = (fileId: FileId) => ({ operations: [], createdAt: Date.now(), lastModified: Date.now() }); + const getAppliedOperations = (fileId: FileId) => []; const history = getFileHistory(fileId); const allOperations = showOnlyApplied ? getAppliedOperations(fileId) : history?.operations || []; diff --git a/frontend/src/components/pageEditor/FileThumbnail.tsx b/frontend/src/components/pageEditor/FileThumbnail.tsx index a0a7d1795..1eda1f6c8 100644 --- a/frontend/src/components/pageEditor/FileThumbnail.tsx +++ b/frontend/src/components/pageEditor/FileThumbnail.tsx @@ -11,9 +11,10 @@ import { draggable, dropTargetForElements } from '@atlaskit/pragmatic-drag-and-d import styles from './PageEditor.module.css'; import { useFileContext } from '../../contexts/FileContext'; +import { FileId } from '../../types/file'; interface FileItem { - id: string; + id: FileId; name: string; pageCount: number; thumbnail: string | null; @@ -27,12 +28,12 @@ interface FileThumbnailProps { totalFiles: number; selectedFiles: string[]; selectionMode: boolean; - onToggleFile: (fileId: string) => void; - onDeleteFile: (fileId: string) => void; - onViewFile: (fileId: string) => void; + onToggleFile: (fileId: FileId) => void; + onDeleteFile: (fileId: FileId) => void; + onViewFile: (fileId: FileId) => void; onSetStatus: (status: string) => void; - onReorderFiles?: (sourceFileId: string, targetFileId: string, selectedFileIds: string[]) => void; - onDownloadFile?: (fileId: string) => void; + onReorderFiles?: (sourceFileId: FileId, targetFileId: FileId, selectedFileIds: FileId[]) => void; + onDownloadFile?: (fileId: FileId) => void; toolMode?: boolean; isSupported?: boolean; } @@ -161,8 +162,8 @@ const FileThumbnail = ({ onDrop: ({ source }) => { const sourceData = source.data; if (sourceData.type === 'file' && onReorderFiles) { - const sourceFileId = sourceData.fileId as string; - const selectedFileIds = sourceData.selectedFiles as string[]; + const sourceFileId = sourceData.fileId as FileId; + const selectedFileIds = sourceData.selectedFiles as FileId[]; onReorderFiles(sourceFileId, file.id, selectedFileIds); } } diff --git a/frontend/src/components/pageEditor/PageEditor.tsx b/frontend/src/components/pageEditor/PageEditor.tsx index 69818d28d..ec007f327 100644 --- a/frontend/src/components/pageEditor/PageEditor.tsx +++ b/frontend/src/components/pageEditor/PageEditor.tsx @@ -17,6 +17,7 @@ import PageThumbnail from './PageThumbnail'; import DragDropGrid from './DragDropGrid'; import SkeletonLoader from '../shared/SkeletonLoader'; import NavigationWarningModal from '../shared/NavigationWarningModal'; +import { FileId } from "../../types/file"; import { DOMCommand, @@ -83,7 +84,7 @@ const PageEditor = ({ // Grid container ref for positioning split indicators const gridContainerRef = useRef(null); - + // State to trigger re-renders when container size changes const [containerDimensions, setContainerDimensions] = useState({ width: 0, height: 0 }); @@ -128,7 +129,7 @@ const PageEditor = ({ // Interface functions for parent component const displayDocument = editedDocument || mergedPdfDocument; - + // Utility functions to convert between page IDs and page numbers const getPageNumbersFromIds = useCallback((pageIds: string[]): number[] => { if (!displayDocument) return []; @@ -137,7 +138,7 @@ const PageEditor = ({ return page?.pageNumber || 0; }).filter(num => num > 0); }, [displayDocument]); - + const getPageIdsFromNumbers = useCallback((pageNumbers: number[]): string[] => { if (!displayDocument) return []; return pageNumbers.map(num => { @@ -145,10 +146,10 @@ const PageEditor = ({ return page?.id || ''; }).filter(id => id !== ''); }, [displayDocument]); - + // Convert selectedPageIds to numbers for components that still need numbers - const selectedPageNumbers = useMemo(() => - getPageNumbersFromIds(selectedPageIds), + const selectedPageNumbers = useMemo(() => + getPageNumbersFromIds(selectedPageIds), [selectedPageIds, getPageNumbersFromIds] ); @@ -173,7 +174,8 @@ const PageEditor = ({ const createRotateCommand = useCallback((pageIds: string[], rotation: number) => ({ execute: () => { const bulkRotateCommand = new BulkRotateCommand(pageIds, rotation); - undoManagerRef.current.executeCommand(bulkRotateCommand); + + undoManagerRef.current.executeCommand(bulkRotateCommand); } }), []); @@ -182,7 +184,8 @@ const PageEditor = ({ if (!displayDocument) return; const pagesToDelete = pageIds.map(pageId => { - const page = displayDocument.pages.find(p => p.id === pageId); + + const page = displayDocument.pages.find(p => p.id === pageId); return page?.pageNumber || 0; }).filter(num => num > 0); @@ -213,7 +216,7 @@ const PageEditor = ({ ); undoManagerRef.current.executeCommand(splitCommand); } - }), [splitPositions]); +}), [splitPositions]); // Command executor for PageThumbnail const executeCommand = useCallback((command: any) => { @@ -234,7 +237,7 @@ const PageEditor = ({ const handleRotate = useCallback((direction: 'left' | 'right') => { if (!displayDocument || selectedPageIds.length === 0) return; const rotation = direction === 'left' ? -90 : 90; - + handleRotatePages(selectedPageIds, rotation); }, [displayDocument, selectedPageIds, handleRotatePages]); @@ -296,14 +299,14 @@ const PageEditor = ({ // Smart toggle logic: follow the majority, default to adding splits if equal const existingSplitsCount = selectedPositions.filter(pos => splitPositions.has(pos)).length; const noSplitsCount = selectedPositions.length - existingSplitsCount; - + // Remove splits only if majority already have splits // If equal (50/50), default to adding splits const shouldRemoveSplits = existingSplitsCount > noSplitsCount; - + const newSplitPositions = new Set(splitPositions); - + if (shouldRemoveSplits) { // Remove splits from all selected positions selectedPositions.forEach(pos => newSplitPositions.delete(pos)); @@ -316,8 +319,8 @@ const PageEditor = ({ const smartSplitCommand = { execute: () => setSplitPositions(newSplitPositions), undo: () => setSplitPositions(splitPositions), - description: shouldRemoveSplits - ? `Remove ${selectedPositions.length} split(s)` + description: shouldRemoveSplits + ? `Remove ${selectedPositions.length} split(s)` : `Add ${selectedPositions.length - existingSplitsCount} split(s)` }; @@ -343,13 +346,13 @@ const PageEditor = ({ // Smart toggle logic: follow the majority, default to adding splits if equal const existingSplitsCount = selectedPositions.filter(pos => splitPositions.has(pos)).length; const noSplitsCount = selectedPositions.length - existingSplitsCount; - + // Remove splits only if majority already have splits // If equal (50/50), default to adding splits const shouldRemoveSplits = existingSplitsCount > noSplitsCount; - + const newSplitPositions = new Set(splitPositions); - + if (shouldRemoveSplits) { // Remove splits from all selected positions selectedPositions.forEach(pos => newSplitPositions.delete(pos)); @@ -362,8 +365,8 @@ const PageEditor = ({ const smartSplitCommand = { execute: () => setSplitPositions(newSplitPositions), undo: () => setSplitPositions(splitPositions), - description: shouldRemoveSplits - ? `Remove ${selectedPositions.length} split(s)` + description: shouldRemoveSplits + ? `Remove ${selectedPositions.length} split(s)` : `Add ${selectedPositions.length - existingSplitsCount} split(s)` }; @@ -404,7 +407,7 @@ const PageEditor = ({ try { const targetPage = displayDocument.pages.find(p => p.pageNumber === insertAfterPage); if (!targetPage) return; - + await actions.addFiles(files, { insertAfterPageId: targetPage.id }); } catch (error) { console.error('Failed to insert files:', error); @@ -443,8 +446,8 @@ const PageEditor = ({ }, [displayDocument, getPageNumbersFromIds]); // Helper function to collect source files for multi-file export - const getSourceFiles = useCallback((): Map | null => { - const sourceFiles = new Map(); + const getSourceFiles = useCallback((): Map | null => { + const sourceFiles = new Map(); // Always include original files activeFileIds.forEach(fileId => { @@ -457,7 +460,7 @@ const PageEditor = ({ // Use multi-file export if we have multiple original files const hasInsertedFiles = false; const hasMultipleOriginalFiles = activeFileIds.length > 1; - + if (!hasInsertedFiles && !hasMultipleOriginalFiles) { return null; // Use single-file export method } @@ -499,7 +502,7 @@ const PageEditor = ({ // Step 2: Use the already selected page IDs // Filter to only include IDs that exist in the document with DOM state - const validSelectedPageIds = selectedPageIds.filter(pageId => + const validSelectedPageIds = selectedPageIds.filter(pageId => documentWithDOMState.pages.some(p => p.id === pageId) ); @@ -551,11 +554,11 @@ const PageEditor = ({ const sourceFiles = getSourceFiles(); const baseExportFilename = getExportFilename(); const baseName = baseExportFilename.replace(/\.pdf$/i, ''); - + for (let i = 0; i < processedDocuments.length; i++) { const doc = processedDocuments[i]; const partFilename = `${baseName}_part_${i + 1}.pdf`; - + const result = sourceFiles ? await pdfExportService.exportPDFMultiFile(doc, sourceFiles, [], { filename: partFilename }) : await pdfExportService.exportPDF(doc, [], { filename: partFilename }); @@ -622,6 +625,7 @@ const PageEditor = ({ const closePdf = useCallback(() => { actions.clearAllFiles(); + undoManagerRef.current.clear(); setSelectedPageIds([]); setSelectionMode(false); @@ -632,7 +636,7 @@ const PageEditor = ({ if (!displayDocument) return; // For now, trigger the actual export directly - // In the original, this would show a preview modal first + // In the original, this would show a preview modal first if (selectedOnly) { onExportSelected(); } else { @@ -723,23 +727,23 @@ const PageEditor = ({ const ITEM_WIDTH = parseFloat(GRID_CONSTANTS.ITEM_WIDTH) * remToPx; const ITEM_HEIGHT = parseFloat(GRID_CONSTANTS.ITEM_HEIGHT) * remToPx; const ITEM_GAP = parseFloat(GRID_CONSTANTS.ITEM_GAP) * remToPx; - + return Array.from(splitPositions).map((position) => { - + // Calculate items per row using DragDropGrid's logic const availableWidth = containerWidth - ITEM_GAP; // Account for first gap const itemWithGap = ITEM_WIDTH + ITEM_GAP; const itemsPerRow = Math.max(1, Math.floor(availableWidth / itemWithGap)); - + // Calculate position within the grid (same as DragDropGrid) const row = Math.floor(position / itemsPerRow); const col = position % itemsPerRow; - + // Position split line between pages (after the current page) // Calculate grid centering offset (same as DragDropGrid) const gridWidth = itemsPerRow * ITEM_WIDTH + (itemsPerRow - 1) * ITEM_GAP; const gridOffset = Math.max(0, (containerWidth - gridWidth) / 2); - + const leftPosition = gridOffset + col * itemWithGap + ITEM_WIDTH + (ITEM_GAP / 2); const topPosition = row * ITEM_HEIGHT + (ITEM_HEIGHT * 0.05); // Center vertically (5% offset since page is 90% height) diff --git a/frontend/src/components/pageEditor/commands/pageCommands.ts b/frontend/src/components/pageEditor/commands/pageCommands.ts index 4c033696e..96b93aa63 100644 --- a/frontend/src/components/pageEditor/commands/pageCommands.ts +++ b/frontend/src/components/pageEditor/commands/pageCommands.ts @@ -1,3 +1,4 @@ +import { FileId } from '../../../types/file'; import { PDFDocument, PDFPage } from '../../../types/pageEditor'; // V1-style DOM-first command system (replaces the old React state commands) @@ -83,18 +84,18 @@ export class DeletePagesCommand extends DOMCommand { }; this.originalSplitPositions = new Set(this.getSplitPositions()); this.originalSelectedPages = [...this.getSelectedPages()]; - + // Convert page numbers to page IDs for stable identification this.pageIdsToDelete = this.pagesToDelete.map(pageNum => { const page = currentDoc.pages.find(p => p.pageNumber === pageNum); return page?.id || ''; }).filter(id => id); - + this.hasExecuted = true; } // Filter out deleted pages by ID (stable across undo/redo) - const remainingPages = currentDoc.pages.filter(page => + const remainingPages = currentDoc.pages.filter(page => !this.pageIdsToDelete.includes(page.id) ); @@ -172,20 +173,20 @@ export class ReorderPagesCommand extends DOMCommand { const selectedPageObjects = this.selectedPages .map(pageNum => currentDoc.pages.find(p => p.pageNumber === pageNum)) .filter(page => page !== undefined) as PDFPage[]; - + const remainingPages = newPages.filter(page => !this.selectedPages!.includes(page.pageNumber)); remainingPages.splice(this.targetIndex, 0, ...selectedPageObjects); - + remainingPages.forEach((page, index) => { page.pageNumber = index + 1; }); - + newPages.splice(0, newPages.length, ...remainingPages); } else { // Single page reorder const [movedPage] = newPages.splice(sourceIndex, 1); newPages.splice(this.targetIndex, 0, movedPage); - + newPages.forEach((page, index) => { page.pageNumber = index + 1; }); @@ -237,13 +238,13 @@ export class SplitCommand extends DOMCommand { // Toggle the split position const currentPositions = this.getSplitPositions(); const newPositions = new Set(currentPositions); - + if (newPositions.has(this.position)) { newPositions.delete(this.position); } else { newPositions.add(this.position); } - + this.setSplitPositions(newPositions); } @@ -282,7 +283,7 @@ export class BulkRotateCommand extends DOMCommand { const currentRotation = rotateMatch ? parseInt(rotateMatch[1]) : 0; this.originalRotations.set(pageId, currentRotation); } - + // Apply rotation using transform to trigger CSS animation const currentTransform = img.style.transform || ''; const rotateMatch = currentTransform.match(/rotate\(([^)]+)\)/); @@ -354,7 +355,7 @@ export class BulkSplitCommand extends DOMCommand { export class SplitAllCommand extends DOMCommand { private originalSplitPositions: Set = new Set(); private allPossibleSplits: Set = new Set(); - + constructor( private totalPages: number, private getSplitPositions: () => Set, @@ -366,15 +367,15 @@ export class SplitAllCommand extends DOMCommand { this.allPossibleSplits.add(i); } } - + execute(): void { // Store original state for undo this.originalSplitPositions = new Set(this.getSplitPositions()); - + // Check if all splits are already active const currentSplits = this.getSplitPositions(); const hasAllSplits = Array.from(this.allPossibleSplits).every(pos => currentSplits.has(pos)); - + if (hasAllSplits) { // Remove all splits this.setSplitPositions(new Set()); @@ -383,12 +384,12 @@ export class SplitAllCommand extends DOMCommand { this.setSplitPositions(this.allPossibleSplits); } } - + undo(): void { // Restore original split positions this.setSplitPositions(this.originalSplitPositions); } - + get description(): string { const currentSplits = this.getSplitPositions(); const hasAllSplits = Array.from(this.allPossibleSplits).every(pos => currentSplits.has(pos)); @@ -453,7 +454,7 @@ export class PageBreakCommand extends DOMCommand { }; this.setDocument(updatedDocument); - + // No need to maintain selection - page IDs remain stable, so selection persists automatically } @@ -529,7 +530,7 @@ export class BulkPageBreakCommand extends DOMCommand { }; this.setDocument(updatedDocument); - + // Maintain existing selection by mapping original selected pages to their new positions const updatedSelection: number[] = []; this.originalSelectedPages.forEach(originalPageNum => { @@ -558,9 +559,9 @@ export class BulkPageBreakCommand extends DOMCommand { export class InsertFilesCommand extends DOMCommand { private insertedPages: PDFPage[] = []; private originalDocument: PDFDocument | null = null; - private fileDataMap = new Map(); // Store file data for thumbnail generation + private fileDataMap = new Map(); // Store file data for thumbnail generation private originalProcessedFile: any = null; // Store original ProcessedFile for undo - private insertedFileMap = new Map(); // Store inserted files for export + private insertedFileMap = new Map(); // Store inserted files for export constructor( private files: File[], @@ -569,7 +570,7 @@ export class InsertFilesCommand extends DOMCommand { private setDocument: (doc: PDFDocument) => void, private setSelectedPages: (pages: number[]) => void, private getSelectedPages: () => number[], - private updateFileContext?: (updatedDocument: PDFDocument, insertedFiles?: Map) => void + private updateFileContext?: (updatedDocument: PDFDocument, insertedFiles?: Map) => void ) { super(); } @@ -587,19 +588,19 @@ export class InsertFilesCommand extends DOMCommand { try { // Process each file to extract pages and wait for all to complete const allNewPages: PDFPage[] = []; - + // Process all files and wait for their completion const baseTimestamp = Date.now(); const extractionPromises = this.files.map(async (file, index) => { - const fileId = `inserted-${file.name}-${baseTimestamp + index}`; + const fileId = `inserted-${file.name}-${baseTimestamp + index}` as FileId; // Store inserted file for export this.insertedFileMap.set(fileId, file); // Use base timestamp + index to ensure unique but predictable file IDs return await this.extractPagesFromFile(file, baseTimestamp + index); }); - + const extractedPageArrays = await Promise.all(extractionPromises); - + // Flatten all extracted pages for (const pages of extractedPageArrays) { allNewPages.push(...pages); @@ -658,7 +659,7 @@ export class InsertFilesCommand extends DOMCommand { // Maintain existing selection by mapping original selected pages to their new positions const originalSelection = this.getSelectedPages(); const updatedSelection: number[] = []; - + originalSelection.forEach(originalPageNum => { if (originalPageNum <= this.insertAfterPageNumber) { // Pages before insertion point keep same number @@ -668,7 +669,7 @@ export class InsertFilesCommand extends DOMCommand { updatedSelection.push(originalPageNum + allNewPages.length); } }); - + this.setSelectedPages(updatedSelection); } catch (error) { @@ -683,35 +684,35 @@ export class InsertFilesCommand extends DOMCommand { private async generateThumbnailsForInsertedPages(updatedDocument: PDFDocument): Promise { try { const { thumbnailGenerationService } = await import('../../../services/thumbnailGenerationService'); - + // Group pages by file ID to generate thumbnails efficiently - const pagesByFileId = new Map(); - + const pagesByFileId = new Map(); + for (const page of this.insertedPages) { - const fileId = page.id.substring(0, page.id.lastIndexOf('-page-')); + const fileId = page.id.substring(0, page.id.lastIndexOf('-page-')) as FileId /* FIX ME: This looks wrong - like we've thrown away info too early and need to recreate it */; if (!pagesByFileId.has(fileId)) { pagesByFileId.set(fileId, []); } pagesByFileId.get(fileId)!.push(page); } - + // Generate thumbnails for each file for (const [fileId, pages] of pagesByFileId) { const arrayBuffer = this.fileDataMap.get(fileId); - + console.log('Generating thumbnails for file:', fileId); console.log('Pages:', pages.length); console.log('ArrayBuffer size:', arrayBuffer?.byteLength || 'undefined'); - + if (arrayBuffer && arrayBuffer.byteLength > 0) { // Extract page numbers for all pages from this file const pageNumbers = pages.map(page => { const pageNumMatch = page.id.match(/-page-(\d+)$/); return pageNumMatch ? parseInt(pageNumMatch[1]) : 1; }); - + console.log('Generating thumbnails for page numbers:', pageNumbers); - + // Generate thumbnails for all pages from this file at once const results = await thumbnailGenerationService.generateThumbnails( fileId, @@ -719,14 +720,14 @@ export class InsertFilesCommand extends DOMCommand { pageNumbers, { scale: 0.2, quality: 0.8 } ); - + console.log('Thumbnail generation results:', results.length, 'thumbnails generated'); - + // Update pages with generated thumbnails for (let i = 0; i < results.length && i < pages.length; i++) { const result = results[i]; const page = pages[i]; - + if (result.success) { const pageIndex = updatedDocument.pages.findIndex(p => p.id === page.id); if (pageIndex >= 0) { @@ -735,7 +736,7 @@ export class InsertFilesCommand extends DOMCommand { } } } - + // Trigger re-render by updating the document this.setDocument({ ...updatedDocument }); } else { @@ -754,7 +755,7 @@ export class InsertFilesCommand extends DOMCommand { try { const arrayBuffer = event.target?.result as ArrayBuffer; console.log('File reader onload - arrayBuffer size:', arrayBuffer?.byteLength || 'undefined'); - + if (!arrayBuffer) { reject(new Error('Failed to read file')); return; @@ -762,24 +763,24 @@ export class InsertFilesCommand extends DOMCommand { // Clone the ArrayBuffer before passing to PDF.js (it might consume it) const clonedArrayBuffer = arrayBuffer.slice(0); - + // Use PDF.js via the worker manager to extract pages const { pdfWorkerManager } = await import('../../../services/pdfWorkerManager'); const pdf = await pdfWorkerManager.createDocument(clonedArrayBuffer); - + const pageCount = pdf.numPages; const pages: PDFPage[] = []; - const fileId = `inserted-${file.name}-${baseTimestamp}`; - + const fileId = `inserted-${file.name}-${baseTimestamp}` as FileId; + console.log('Original ArrayBuffer size:', arrayBuffer.byteLength); console.log('Storing ArrayBuffer for fileId:', fileId, 'size:', arrayBuffer.byteLength); - + // Store the original ArrayBuffer for thumbnail generation this.fileDataMap.set(fileId, arrayBuffer); - + console.log('After storing - fileDataMap size:', this.fileDataMap.size); console.log('Stored value size:', this.fileDataMap.get(fileId)?.byteLength || 'undefined'); - + for (let i = 1; i <= pageCount; i++) { const pageId = `${fileId}-page-${i}`; pages.push({ @@ -793,10 +794,10 @@ export class InsertFilesCommand extends DOMCommand { isBlankPage: false }); } - + // Clean up PDF document pdfWorkerManager.destroyDocument(pdf); - + resolve(pages); } catch (error) { reject(error); @@ -876,4 +877,4 @@ export class UndoManager { this.redoStack = []; this.onStateChange?.(); } -} \ No newline at end of file +} diff --git a/frontend/src/components/pageEditor/hooks/usePageDocument.ts b/frontend/src/components/pageEditor/hooks/usePageDocument.ts index 5a9d13f9f..b620c87b8 100644 --- a/frontend/src/components/pageEditor/hooks/usePageDocument.ts +++ b/frontend/src/components/pageEditor/hooks/usePageDocument.ts @@ -1,6 +1,7 @@ import { useMemo } from 'react'; import { useFileState } from '../../../contexts/FileContext'; import { PDFDocument, PDFPage } from '../../../types/pageEditor'; +import { FileId } from '../../../types/file'; export interface PageDocumentHook { document: PDFDocument | null; @@ -14,17 +15,17 @@ export interface PageDocumentHook { */ export function usePageDocument(): PageDocumentHook { const { state, selectors } = useFileState(); - + // Prefer IDs + selectors to avoid array identity churn const activeFileIds = state.files.ids; const primaryFileId = activeFileIds[0] ?? null; - + // Stable signature for effects (prevents loops) const filesSignature = selectors.getFilesSignature(); - + // UI state const globalProcessing = state.ui.isProcessing; - + // Get primary file record outside useMemo to track processedFile changes const primaryFileRecord = primaryFileId ? selectors.getFileRecord(primaryFileId) : null; const processedFilePages = primaryFileRecord?.processedFile?.pages; @@ -35,7 +36,7 @@ export function usePageDocument(): PageDocumentHook { if (activeFileIds.length === 0) return null; const primaryFile = primaryFileId ? selectors.getFile(primaryFileId) : null; - + // If we have file IDs but no file record, something is wrong - return null to show loading if (!primaryFileRecord) { console.log('🎬 PageEditor: No primary file record found, showing loading'); @@ -50,9 +51,9 @@ export function usePageDocument(): PageDocumentHook { .join(' + '); // Build page insertion map from files with insertion positions - const insertionMap = new Map(); // insertAfterPageId -> fileIds - const originalFileIds: string[] = []; - + const insertionMap = new Map(); // insertAfterPageId -> fileIds + const originalFileIds: FileId[] = []; + activeFileIds.forEach(fileId => { const record = selectors.getFileRecord(fileId); if (record?.insertAfterPageId !== undefined) { @@ -64,21 +65,21 @@ export function usePageDocument(): PageDocumentHook { originalFileIds.push(fileId); } }); - + // Build pages by interleaving original pages with insertions let pages: PDFPage[] = []; let totalPageCount = 0; - + // Helper function to create pages from a file - const createPagesFromFile = (fileId: string, startPageNumber: number): PDFPage[] => { + const createPagesFromFile = (fileId: FileId, startPageNumber: number): PDFPage[] => { const fileRecord = selectors.getFileRecord(fileId); if (!fileRecord) { return []; } - + const processedFile = fileRecord.processedFile; let filePages: PDFPage[] = []; - + if (processedFile?.pages && processedFile.pages.length > 0) { // Use fully processed pages with thumbnails filePages = processedFile.pages.map((page, pageIndex) => ({ @@ -104,7 +105,7 @@ export function usePageDocument(): PageDocumentHook { splitAfter: false, })); } - + return filePages; }; @@ -114,35 +115,35 @@ export function usePageDocument(): PageDocumentHook { const filePages = createPagesFromFile(fileId, 1); // Temporary numbering originalFilePages.push(...filePages); }); - - // Start with all original pages numbered sequentially + + // Start with all original pages numbered sequentially pages = originalFilePages.map((page, index) => ({ ...page, pageNumber: index + 1 })); - + // Process each insertion by finding the page ID and inserting after it for (const [insertAfterPageId, fileIds] of insertionMap.entries()) { const targetPageIndex = pages.findIndex(p => p.id === insertAfterPageId); - + if (targetPageIndex === -1) continue; - + // Collect all pages to insert const allNewPages: PDFPage[] = []; fileIds.forEach(fileId => { const insertedPages = createPagesFromFile(fileId, 1); allNewPages.push(...insertedPages); }); - + // Insert all new pages after the target page pages.splice(targetPageIndex + 1, 0, ...allNewPages); - + // Renumber all pages after insertion pages.forEach((page, index) => { page.pageNumber = index + 1; }); } - + totalPageCount = pages.length; if (pages.length === 0) { @@ -173,4 +174,4 @@ export function usePageDocument(): PageDocumentHook { isVeryLargeDocument, isLoading }; -} \ No newline at end of file +} diff --git a/frontend/src/components/shared/FileGrid.tsx b/frontend/src/components/shared/FileGrid.tsx index 169e19f88..1a43196d6 100644 --- a/frontend/src/components/shared/FileGrid.tsx +++ b/frontend/src/components/shared/FileGrid.tsx @@ -5,6 +5,7 @@ import SearchIcon from "@mui/icons-material/Search"; import SortIcon from "@mui/icons-material/Sort"; import FileCard from "./FileCard"; import { FileRecord } from "../../types/fileContext"; +import { FileId } from "../../types/file"; interface FileGridProps { files: Array<{ file: File; record?: FileRecord }>; @@ -12,8 +13,8 @@ interface FileGridProps { onDoubleClick?: (item: { file: File; record?: FileRecord }) => void; onView?: (item: { file: File; record?: FileRecord }) => void; onEdit?: (item: { file: File; record?: FileRecord }) => void; - onSelect?: (fileId: string) => void; - selectedFiles?: string[]; + onSelect?: (fileId: FileId) => void; + selectedFiles?: FileId[]; showSearch?: boolean; showSort?: boolean; maxDisplay?: number; // If set, shows only this many files with "Show All" option @@ -119,11 +120,11 @@ const FileGrid = ({ direction="row" wrap="wrap" gap="md" - h="30rem" + h="30rem" style={{ overflowY: "auto", width: "100%" }} > {displayFiles.map((item, idx) => { - const fileId = item.record?.id || item.file.name; + const fileId = item.record?.id || item.file.name as FileId /* FIX ME: This doesn't seem right */; const originalIdx = files.findIndex(f => (f.record?.id || f.file.name) === fileId); const supported = isFileSupported ? isFileSupported(item.file.name) : true; return ( diff --git a/frontend/src/components/shared/FilePickerModal.tsx b/frontend/src/components/shared/FilePickerModal.tsx index 8aa054f25..fde20f014 100644 --- a/frontend/src/components/shared/FilePickerModal.tsx +++ b/frontend/src/components/shared/FilePickerModal.tsx @@ -1,12 +1,12 @@ import React, { useState, useEffect } from 'react'; -import { - Modal, - Text, - Button, - Group, - Stack, - Checkbox, - ScrollArea, +import { + Modal, + Text, + Button, + Group, + Stack, + Checkbox, + ScrollArea, Box, Image, Badge, @@ -15,6 +15,7 @@ import { } from '@mantine/core'; import PictureAsPdfIcon from '@mui/icons-material/PictureAsPdf'; import { useTranslation } from 'react-i18next'; +import { FileId } from '../../types/file'; interface FilePickerModalProps { opened: boolean; @@ -30,7 +31,7 @@ const FilePickerModal = ({ onSelectFiles, }: FilePickerModalProps) => { const { t } = useTranslation(); - const [selectedFileIds, setSelectedFileIds] = useState([]); + const [selectedFileIds, setSelectedFileIds] = useState([]); // Reset selection when modal opens useEffect(() => { @@ -39,9 +40,9 @@ const FilePickerModal = ({ } }, [opened]); - const toggleFileSelection = (fileId: string) => { + const toggleFileSelection = (fileId: FileId) => { setSelectedFileIds(prev => { - return prev.includes(fileId) + return prev.includes(fileId) ? prev.filter(id => id !== fileId) : [...prev, fileId]; }); @@ -56,10 +57,10 @@ const FilePickerModal = ({ }; const handleConfirm = async () => { - const selectedFiles = storedFiles.filter(f => + const selectedFiles = storedFiles.filter(f => selectedFileIds.includes(f.id) ); - + // Convert stored files to File objects const convertedFiles = await Promise.all( selectedFiles.map(async (fileItem) => { @@ -68,12 +69,12 @@ const FilePickerModal = ({ if (fileItem instanceof File) { return fileItem; } - + // If it has a file property, use that if (fileItem.file && fileItem.file instanceof File) { return fileItem.file; } - + // If it's from IndexedDB storage, reconstruct the File if (fileItem.arrayBuffer && typeof fileItem.arrayBuffer === 'function') { const arrayBuffer = await fileItem.arrayBuffer(); @@ -83,8 +84,8 @@ const FilePickerModal = ({ lastModified: fileItem.lastModified || Date.now() }); } - - // If it has data property, reconstruct the File + + // If it has data property, reconstruct the File if (fileItem.data) { const blob = new Blob([fileItem.data], { type: fileItem.type || 'application/pdf' }); return new File([blob], fileItem.name, { @@ -92,7 +93,7 @@ const FilePickerModal = ({ lastModified: fileItem.lastModified || Date.now() }); } - + console.warn('Could not convert file item:', fileItem); return null; } catch (error) { @@ -101,10 +102,10 @@ const FilePickerModal = ({ } }) ); - + // Filter out any null values and return valid Files const validFiles = convertedFiles.filter((f): f is File => f !== null); - + onSelectFiles(validFiles); onClose(); }; @@ -156,18 +157,18 @@ const FilePickerModal = ({ {storedFiles.map((file) => { const fileId = file.id; const isSelected = selectedFileIds.includes(fileId); - + return ( toggleFileSelection(fileId)} onClick={(e) => e.stopPropagation()} /> - + {/* Thumbnail */} {t("close", "Cancel")} -