Fix delete bug and save bug

This commit is contained in:
Reece 2025-11-12 16:43:53 +00:00
parent f1c4b5f2c7
commit e29ef36bb9
5 changed files with 235 additions and 109 deletions

View File

@ -67,7 +67,7 @@ export class DeletePagesCommand extends DOMCommand {
private pagesToDelete: number[],
private getCurrentDocument: () => PDFDocument | null,
private setDocument: (doc: PDFDocument) => void,
private setSelectedPages: (pages: number[]) => void,
private setSelectedPageIds: (pageIds: string[]) => void,
private getSplitPositions: () => Set<number>,
private setSplitPositions: (positions: Set<number>) => void,
private getSelectedPages: () => number[],
@ -99,6 +99,13 @@ export class DeletePagesCommand extends DOMCommand {
this.hasExecuted = true;
}
const selectedPageNumbersBefore = this.getSelectedPages();
const selectedIdSet = new Set(
selectedPageNumbersBefore
.map((pageNum) => currentDoc.pages.find((p) => p.pageNumber === pageNum)?.id)
.filter((id): id is string => Boolean(id))
);
// Filter out deleted pages by ID (stable across undo/redo)
const remainingPages = currentDoc.pages.filter(page =>
!this.pageIdsToDelete.includes(page.id)
@ -106,7 +113,7 @@ export class DeletePagesCommand extends DOMCommand {
if (remainingPages.length === 0) {
// If all pages would be deleted, clear selection/splits and close PDF
this.setSelectedPages([]);
this.setSelectedPageIds([]);
this.setSplitPositions(new Set());
this.onAllPagesDeleted?.();
return;
@ -135,7 +142,12 @@ export class DeletePagesCommand extends DOMCommand {
// Apply changes
this.setDocument(updatedDocument);
this.setSelectedPages([]);
const remainingSelectedPageIds = remainingPages
.filter((page) => selectedIdSet.has(page.id))
.map((page) => page.id);
this.setSelectedPageIds(remainingSelectedPageIds);
this.setSplitPositions(newPositions);
}
@ -145,7 +157,12 @@ export class DeletePagesCommand extends DOMCommand {
// Simply restore the complete original document state
this.setDocument(this.originalDocument);
this.setSplitPositions(this.originalSplitPositions);
this.setSelectedPages(this.originalSelectedPages);
const restoredIds = this.originalSelectedPages
.map((pageNum) =>
this.originalDocument!.pages.find((page) => page.pageNumber === pageNum)?.id || ""
)
.filter((id) => id !== "");
this.setSelectedPageIds(restoredIds);
}
get description(): string {

View File

@ -24,6 +24,7 @@ export const useEditedDocumentState = ({
const editedDocumentRef = useRef<PDFDocument | null>(null);
const pagePositionCacheRef = useRef<Map<string, number>>(new Map());
const pageNeighborCacheRef = useRef<Map<string, string | null>>(new Map());
const lastSyncedSignatureRef = useRef<string | null>(null);
// Clone the initial document once so we can safely mutate working state
useEffect(() => {
@ -71,111 +72,157 @@ export const useEditedDocumentState = ({
return mergedPdfDocument.pages.map((page) => page.id).join(",");
}, [mergedPdfDocument]);
useEffect(() => {
if (!mergedPdfDocument) {
lastSyncedSignatureRef.current = null;
}
}, [mergedPdfDocument]);
// Keep editedDocument in sync with out-of-band insert/remove events (e.g. uploads finishing)
useEffect(() => {
const currentEditedDocument = editedDocumentRef.current;
if (!mergedPdfDocument || !currentEditedDocument) return;
const sourcePages = mergedPdfDocument.pages;
const sourceIds = new Set(sourcePages.map((p) => p.id));
const signatureChanged =
mergedDocSignature !== lastSyncedSignatureRef.current;
const metadataChanged =
currentEditedDocument.id !== mergedPdfDocument.id ||
currentEditedDocument.file !== mergedPdfDocument.file ||
currentEditedDocument.name !== mergedPdfDocument.name;
const prevIds = new Set(currentEditedDocument.pages.map((p) => p.id));
const newPages: PDFPage[] = [];
for (const page of sourcePages) {
if (!prevIds.has(page.id)) {
newPages.push(page);
}
}
const hasAdditions = newPages.length > 0;
const isEphemeralPage = (page: PDFPage) => {
// Blank pages and placeholders are editor-local pages that don't exist in the source document.
return Boolean(page.isBlankPage || page.isPlaceholder);
};
let hasRemovals = false;
for (const page of currentEditedDocument.pages) {
if (!sourceIds.has(page.id) && !isEphemeralPage(page)) {
hasRemovals = true;
break;
}
}
if (!hasAdditions && !hasRemovals) return;
if (!signatureChanged && !metadataChanged) return;
setEditedDocument((prev) => {
if (!prev) return prev;
let pages = [...prev.pages];
const placeholderPositions = new Map<FileId, number>();
pages.forEach((page, index) => {
if (page.isPlaceholder && page.originalFileId) {
placeholderPositions.set(page.originalFileId, index);
let pages = prev.pages;
if (signatureChanged) {
const sourcePages = mergedPdfDocument.pages;
const sourceIds = new Set(sourcePages.map((p) => p.id));
const prevIds = new Set(prev.pages.map((p) => p.id));
const newPages: PDFPage[] = [];
for (const page of sourcePages) {
if (!prevIds.has(page.id)) {
newPages.push(page);
}
}
});
const nextInsertIndexByFile = new Map(placeholderPositions);
const hasAdditions = newPages.length > 0;
const isEphemeralPage = (page: PDFPage) =>
Boolean(page.isBlankPage || page.isPlaceholder);
if (hasRemovals) {
pages = pages.filter((page) => sourceIds.has(page.id) || isEphemeralPage(page));
}
let hasRemovals = false;
for (const page of prev.pages) {
if (!sourceIds.has(page.id) && !isEphemeralPage(page)) {
hasRemovals = true;
break;
}
}
if (hasAdditions) {
const mergedIndexMap = new Map<string, number>();
sourcePages.forEach((page, index) => mergedIndexMap.set(page.id, index));
if (hasAdditions || hasRemovals) {
pages = [...prev.pages];
const additions = newPages
.map((page) => ({
page,
cachedIndex: pagePositionCacheRef.current.get(page.id),
mergedIndex: mergedIndexMap.get(page.id) ?? sourcePages.length,
neighborId: pageNeighborCacheRef.current.get(page.id),
}))
.sort((a, b) => {
const aIndex = a.cachedIndex ?? a.mergedIndex;
const bIndex = b.cachedIndex ?? b.mergedIndex;
if (aIndex !== bIndex) return aIndex - bIndex;
return a.mergedIndex - b.mergedIndex;
const placeholderPositions = new Map<FileId, number>();
pages.forEach((page, index) => {
if (page.isPlaceholder && page.originalFileId) {
placeholderPositions.set(page.originalFileId, index);
}
});
additions.forEach(({ page, neighborId, cachedIndex, mergedIndex }) => {
if (pages.some((existing) => existing.id === page.id)) {
return;
const nextInsertIndexByFile = new Map(placeholderPositions);
if (hasRemovals) {
pages = pages.filter(
(page) => sourceIds.has(page.id) || isEphemeralPage(page)
);
}
let insertIndex: number;
const originalFileId = page.originalFileId;
const placeholderIndex =
originalFileId !== undefined
? nextInsertIndexByFile.get(originalFileId)
: undefined;
if (hasAdditions) {
const mergedIndexMap = new Map<string, number>();
sourcePages.forEach((page, index) =>
mergedIndexMap.set(page.id, index)
);
if (originalFileId && placeholderIndex !== undefined) {
insertIndex = Math.min(placeholderIndex, pages.length);
nextInsertIndexByFile.set(originalFileId, insertIndex + 1);
} else if (neighborId === null) {
insertIndex = 0;
} else if (neighborId) {
const neighborIndex = pages.findIndex((p) => p.id === neighborId);
if (neighborIndex !== -1) {
insertIndex = neighborIndex + 1;
} else {
const fallbackIndex = cachedIndex ?? mergedIndex ?? pages.length;
insertIndex = Math.min(fallbackIndex, pages.length);
}
} else {
const fallbackIndex = cachedIndex ?? mergedIndex ?? pages.length;
insertIndex = Math.min(fallbackIndex, pages.length);
const additions = newPages
.map((page) => ({
page,
cachedIndex: pagePositionCacheRef.current.get(page.id),
mergedIndex: mergedIndexMap.get(page.id) ?? sourcePages.length,
neighborId: pageNeighborCacheRef.current.get(page.id),
}))
.sort((a, b) => {
const aIndex = a.cachedIndex ?? a.mergedIndex;
const bIndex = b.cachedIndex ?? b.mergedIndex;
if (aIndex !== bIndex) return aIndex - bIndex;
return a.mergedIndex - b.mergedIndex;
});
additions.forEach(({ page, neighborId, cachedIndex, mergedIndex }) => {
if (pages.some((existing) => existing.id === page.id)) {
return;
}
let insertIndex: number;
const originalFileId = page.originalFileId;
const placeholderIndex =
originalFileId !== undefined
? nextInsertIndexByFile.get(originalFileId)
: undefined;
if (originalFileId && placeholderIndex !== undefined) {
insertIndex = Math.min(placeholderIndex, pages.length);
nextInsertIndexByFile.set(originalFileId, insertIndex + 1);
} else if (neighborId === null) {
insertIndex = 0;
} else if (neighborId) {
const neighborIndex = pages.findIndex((p) => p.id === neighborId);
if (neighborIndex !== -1) {
insertIndex = neighborIndex + 1;
} else {
const fallbackIndex = cachedIndex ?? mergedIndex ?? pages.length;
insertIndex = Math.min(fallbackIndex, pages.length);
}
} else {
const fallbackIndex = cachedIndex ?? mergedIndex ?? pages.length;
insertIndex = Math.min(fallbackIndex, pages.length);
}
const clonedPage = { ...page };
pages.splice(insertIndex, 0, clonedPage);
});
}
const clonedPage = { ...page };
pages.splice(insertIndex, 0, clonedPage);
});
pages = pages.map((page, index) => ({
...page,
pageNumber: index + 1,
}));
}
}
pages = pages.map((page, index) => ({ ...page, pageNumber: index + 1 }));
return { ...prev, pages };
const shouldReplaceBase = metadataChanged || signatureChanged;
const baseDocument = shouldReplaceBase
? {
...mergedPdfDocument,
destroy: prev.destroy,
}
: prev;
if (baseDocument === prev && pages === prev.pages) {
return prev;
}
return {
...baseDocument,
pages,
totalPages: pages.length,
};
});
if (signatureChanged) {
lastSyncedSignatureRef.current = mergedDocSignature;
}
}, [mergedPdfDocument, fileOrderKey, mergedDocSignature]);
const displayDocument = editedDocument || initialDocument;

View File

@ -96,10 +96,7 @@ export const usePageEditorCommands = ({
pagesToDelete,
getEditedDocument,
setEditedDocument,
(pageNumbers: number[]) => {
const updatedIds = getPageIdsFromNumbers(pageNumbers);
setSelectedPageIds(updatedIds);
},
setSelectedPageIds,
() => splitPositions,
setSplitPositions,
() => getPageNumbersFromIds(selectedPageIds),
@ -113,7 +110,6 @@ export const usePageEditorCommands = ({
closePdf,
executeCommandWithTracking,
getEditedDocument,
getPageIdsFromNumbers,
getPageNumbersFromIds,
selectedPageIds,
setEditedDocument,
@ -162,10 +158,7 @@ export const usePageEditorCommands = ({
selectedPageNumbers,
getEditedDocument,
setEditedDocument,
(pageNumbers: number[]) => {
const pageIds = getPageIdsFromNumbers(pageNumbers);
setSelectedPageIds(pageIds);
},
setSelectedPageIds,
() => splitPositions,
setSplitPositions,
() => selectedPageNumbers,
@ -177,7 +170,6 @@ export const usePageEditorCommands = ({
displayDocument,
executeCommandWithTracking,
getEditedDocument,
getPageIdsFromNumbers,
getPageNumbersFromIds,
selectedPageIds,
setEditedDocument,
@ -194,10 +186,7 @@ export const usePageEditorCommands = ({
[pageNumber],
getEditedDocument,
setEditedDocument,
(pageNumbers: number[]) => {
const pageIds = getPageIdsFromNumbers(pageNumbers);
setSelectedPageIds(pageIds);
},
setSelectedPageIds,
() => splitPositions,
setSplitPositions,
() => getPageNumbersFromIds(selectedPageIds),
@ -209,7 +198,6 @@ export const usePageEditorCommands = ({
closePdf,
getEditedDocument,
executeCommandWithTracking,
getPageIdsFromNumbers,
getPageNumbersFromIds,
selectedPageIds,
setEditedDocument,

View File

@ -37,7 +37,18 @@ export function usePageDocument(): PageDocumentHook {
});
}, [allFileIdsKey, activeFilesSignature, selectors]);
const primaryFileId = activeFileIds[0] ?? null;
const selectedActiveFileIds = useMemo(() => {
if (activeFileIds.length === 0) {
return [];
}
const selectedSet = new Set(state.ui.selectedFileIds);
if (selectedSet.size === 0) {
return [];
}
return activeFileIds.filter((id) => selectedSet.has(id));
}, [activeFileIds, selectedFileIdsKey]);
const primaryFileId = selectedActiveFileIds[0] ?? activeFileIds[0] ?? null;
// UI state
const globalProcessing = state.ui.isProcessing;
@ -63,10 +74,14 @@ export function usePageDocument(): PageDocumentHook {
return null;
}
const namingFileIds = selectedActiveFileIds.length > 0 ? selectedActiveFileIds : activeFileIds;
const name =
activeFileIds.length === 1
? (primaryStirlingFileStub.name ?? 'document.pdf')
: activeFileIds
namingFileIds.length <= 1
? (namingFileIds[0]
? selectors.getStirlingFileStub(namingFileIds[0])?.name ?? 'document.pdf'
: 'document.pdf')
: namingFileIds
.map(id => (selectors.getStirlingFileStub(id)?.name ?? 'file').replace(/\.pdf$/i, ''))
.join(' + ');
@ -230,7 +245,7 @@ export function usePageDocument(): PageDocumentHook {
};
return mergedDoc;
}, [activeFileIds, primaryFileId, primaryStirlingFileStub, processedFilePages, processedFileTotalPages, selectors, activeFilesSignature, selectedFileIdsKey, state.ui.selectedFileIds, allFileIds, currentPagesSignature, currentPages]);
}, [activeFileIds, selectedActiveFileIds, primaryFileId, primaryStirlingFileStub, processedFilePages, processedFileTotalPages, selectors, activeFilesSignature, selectedFileIdsKey, state.ui.selectedFileIds, allFileIds, currentPagesSignature, currentPages]);
// Large document detection for smart loading
const isVeryLargeDocument = useMemo(() => {

View File

@ -26,6 +26,36 @@ interface UsePageEditorExportParams {
setSplitPositions: Dispatch<SetStateAction<Set<number>>>;
}
const removePlaceholderPages = (document: PDFDocument): PDFDocument => {
const filteredPages = document.pages.filter((page) => !page.isPlaceholder);
if (filteredPages.length === document.pages.length) {
return document;
}
const normalizedPages = filteredPages.map((page, index) => ({
...page,
pageNumber: index + 1,
}));
return {
...document,
pages: normalizedPages,
totalPages: normalizedPages.length,
};
};
const normalizeProcessedDocuments = (
processed: PDFDocument | PDFDocument[]
): PDFDocument | PDFDocument[] => {
if (Array.isArray(processed)) {
const normalized = processed
.map(removePlaceholderPages)
.filter((doc) => doc.pages.length > 0);
return normalized;
}
return removePlaceholderPages(processed);
};
export const usePageEditorExport = ({
displayDocument,
selectedPageIds,
@ -84,9 +114,16 @@ export const usePageEditorExport = ({
splitPositions
);
const documentWithDOMState = Array.isArray(processedDocuments)
? processedDocuments[0]
: processedDocuments;
const normalizedDocuments = normalizeProcessedDocuments(processedDocuments);
const documentWithDOMState = Array.isArray(normalizedDocuments)
? normalizedDocuments[0]
: normalizedDocuments;
if (!documentWithDOMState || documentWithDOMState.pages.length === 0) {
console.warn("Export skipped: no concrete pages available after filtering placeholders.");
setExportLoading(false);
return;
}
const validSelectedPageIds = selectedPageIds.filter((pageId) =>
documentWithDOMState.pages.some((page) => page.id === pageId)
@ -137,10 +174,21 @@ export const usePageEditorExport = ({
splitPositions
);
const normalizedDocuments = normalizeProcessedDocuments(processedDocuments);
if (
(Array.isArray(normalizedDocuments) && normalizedDocuments.length === 0) ||
(!Array.isArray(normalizedDocuments) && normalizedDocuments.pages.length === 0)
) {
console.warn("Export skipped: no concrete pages available after filtering placeholders.");
setExportLoading(false);
return;
}
const sourceFiles = getSourceFiles();
const exportFilename = getExportFilename();
const files = await exportProcessedDocumentsToFiles(
processedDocuments,
normalizedDocuments,
sourceFiles,
exportFilename
);
@ -190,10 +238,21 @@ export const usePageEditorExport = ({
splitPositions
);
const normalizedDocuments = normalizeProcessedDocuments(processedDocuments);
if (
(Array.isArray(normalizedDocuments) && normalizedDocuments.length === 0) ||
(!Array.isArray(normalizedDocuments) && normalizedDocuments.pages.length === 0)
) {
console.warn("Apply changes skipped: no concrete pages available after filtering placeholders.");
setExportLoading(false);
return;
}
const sourceFiles = getSourceFiles();
const exportFilename = getExportFilename();
const files = await exportProcessedDocumentsToFiles(
processedDocuments,
normalizedDocuments,
sourceFiles,
exportFilename
);