From 14ff8144bf7ea212fb7fbdde8e4ac445c768e260 Mon Sep 17 00:00:00 2001 From: EthanHealy01 Date: Wed, 12 Nov 2025 13:38:12 +0000 Subject: [PATCH] fix selection bug --- .../src/core/contexts/FilesModalContext.tsx | 33 +++++++-- frontend/src/core/hooks/useFileHandler.ts | 6 +- frontend/src/core/tools/Compare.tsx | 70 +++++-------------- 3 files changed, 48 insertions(+), 61 deletions(-) diff --git a/frontend/src/core/contexts/FilesModalContext.tsx b/frontend/src/core/contexts/FilesModalContext.tsx index 468609f7d..aa1d79d1c 100644 --- a/frontend/src/core/contexts/FilesModalContext.tsx +++ b/frontend/src/core/contexts/FilesModalContext.tsx @@ -1,7 +1,9 @@ import React, { createContext, useContext, useState, useCallback, useMemo } from 'react'; import { useFileHandler } from '@app/hooks/useFileHandler'; import { useFileActions } from '@app/contexts/FileContext'; +import { useFileContext } from '@app/contexts/file/fileHooks'; import { StirlingFileStub } from '@app/types/fileContext'; +import type { FileId } from '@app/types/file'; import { fileStorage } from '@app/services/fileStorage'; interface FilesModalContextType { @@ -19,6 +21,7 @@ const FilesModalContext = createContext(null); export const FilesModalProvider: React.FC<{ children: React.ReactNode }> = ({ children }) => { const { addFiles } = useFileHandler(); const { actions } = useFileActions(); + const fileCtx = useFileContext(); const [isFilesModalOpen, setIsFilesModalOpen] = useState(false); const [onModalClose, setOnModalClose] = useState<(() => void) | undefined>(); const [insertAfterPage, setInsertAfterPage] = useState(); @@ -37,16 +40,25 @@ export const FilesModalProvider: React.FC<{ children: React.ReactNode }> = ({ ch onModalClose?.(); }, [onModalClose]); - const handleFileUpload = useCallback((files: File[]) => { + const handleFileUpload = useCallback(async (files: File[]) => { if (customHandler) { // Use custom handler for special cases (like page insertion) customHandler(files, insertAfterPage); } else { - // Use normal file handling - addFiles(files); + // 1) Add via standard flow (auto-selects new files) + await addFiles(files); + // 2) Merge all requested file IDs (covers already-present files too) + const ids = files + .map((f) => fileCtx.findFileId(f) as FileId | undefined) + .filter((id): id is FileId => Boolean(id)); + if (ids.length > 0) { + const currentSelected = fileCtx.selectors.getSelectedStirlingFileStubs().map((s) => s.id); + const nextSelection = Array.from(new Set([...currentSelected, ...ids])); + actions.setSelectedFiles(nextSelection); + } } closeFilesModal(); - }, [addFiles, closeFilesModal, insertAfterPage, customHandler]); + }, [addFiles, closeFilesModal, insertAfterPage, customHandler, actions, fileCtx]); const handleRecentFileSelect = useCallback(async (stirlingFileStubs: StirlingFileStub[]) => { if (customHandler) { @@ -67,15 +79,22 @@ export const FilesModalProvider: React.FC<{ children: React.ReactNode }> = ({ ch console.error('Failed to load files for custom handler:', error); } } else { - // Normal case - use addStirlingFileStubs to preserve metadata + // Normal case - use addStirlingFileStubs to preserve metadata (auto-selects new) if (actions.addStirlingFileStubs) { - actions.addStirlingFileStubs(stirlingFileStubs, { selectFiles: true }); + await actions.addStirlingFileStubs(stirlingFileStubs, { selectFiles: true }); + // Merge all requested IDs into selection (covers files that already existed) + const requestedIds = stirlingFileStubs.map((s) => s.id); + if (requestedIds.length > 0) { + const currentSelected = fileCtx.selectors.getSelectedStirlingFileStubs().map((s) => s.id); + const nextSelection = Array.from(new Set([...currentSelected, ...requestedIds])); + actions.setSelectedFiles(nextSelection); + } } else { console.error('addStirlingFileStubs action not available'); } } closeFilesModal(); - }, [actions.addStirlingFileStubs, closeFilesModal, customHandler, insertAfterPage]); + }, [actions.addStirlingFileStubs, closeFilesModal, customHandler, insertAfterPage, actions, fileCtx]); const setModalCloseCallback = useCallback((callback: () => void) => { setOnModalClose(() => callback); diff --git a/frontend/src/core/hooks/useFileHandler.ts b/frontend/src/core/hooks/useFileHandler.ts index 93c7fe286..fc935b029 100644 --- a/frontend/src/core/hooks/useFileHandler.ts +++ b/frontend/src/core/hooks/useFileHandler.ts @@ -1,14 +1,16 @@ import { useCallback } from 'react'; import { useFileActions } from '@app/contexts/FileContext'; +import type { StirlingFile } from '@app/types/fileContext'; export const useFileHandler = () => { const { actions } = useFileActions(); - const addFiles = useCallback(async (files: File[], options: { insertAfterPageId?: string; selectFiles?: boolean } = {}) => { + const addFiles = useCallback(async (files: File[], options: { insertAfterPageId?: string; selectFiles?: boolean } = {}): Promise => { // Merge default options with passed options - passed options take precedence const mergedOptions = { selectFiles: true, ...options }; // Let FileContext handle deduplication with quickKey logic - await actions.addFiles(files, mergedOptions); + const result = await actions.addFiles(files, mergedOptions); + return result; }, [actions.addFiles]); return { diff --git a/frontend/src/core/tools/Compare.tsx b/frontend/src/core/tools/Compare.tsx index ad0ae81e3..a2a78a036 100644 --- a/frontend/src/core/tools/Compare.tsx +++ b/frontend/src/core/tools/Compare.tsx @@ -18,7 +18,7 @@ import { import CompareWorkbenchView from '@app/components/tools/compare/CompareWorkbenchView'; import { useToolWorkflow } from '@app/contexts/ToolWorkflowContext'; import { useNavigationActions } from '@app/contexts/NavigationContext'; -import { useFileContext } from '@app/contexts/file/fileHooks'; +import { useFileContext, useFileState } from '@app/contexts/file/fileHooks'; import type { FileId } from '@app/types/file'; import type { StirlingFile } from '@app/types/fileContext'; import DocumentThumbnail from '@app/components/shared/filePreview/DocumentThumbnail'; @@ -26,8 +26,6 @@ import type { CompareWorkbenchData } from '@app/types/compare'; import FitText from '@app/components/shared/FitText'; import { getDefaultWorkbench } from '@app/types/workbench'; import { useFilesModalContext } from '@app/contexts/FilesModalContext'; -import { createQuickKey } from '@app/types/fileContext'; -import { useFileHandler } from '@app/hooks/useFileHandler'; const CUSTOM_VIEW_ID = 'compareWorkbenchView'; const CUSTOM_WORKBENCH_ID = 'custom:compareWorkbenchView' as const; @@ -42,8 +40,8 @@ const Compare = (props: BaseToolProps) => { clearCustomWorkbenchViewData, } = useToolWorkflow(); const { selectors, actions: fileActions } = useFileContext(); + const { state: fileState } = useFileState(); const { openFilesModal } = useFilesModalContext(); - const { addFiles } = useFileHandler(); const base = useBaseTool( 'compare', @@ -98,7 +96,8 @@ const Compare = (props: BaseToolProps) => { // Auto-map from workbench selection: always reflect the first two selected files in order. // This also handles deselection by promoting the remaining selection to base and clearing comparison. useEffect(() => { - const selectedIds = base.selectedFiles.map(f => f.fileId as FileId); + // Use selected IDs directly from state so it works even if File objects aren't loaded yet + const selectedIds = (fileState.ui.selectedFileIds as FileId[]) ?? []; // Determine next base: keep current if still selected; otherwise use the first selected id const nextBase: FileId | null = params.baseFileId && selectedIds.includes(params.baseFileId) @@ -120,7 +119,7 @@ const Compare = (props: BaseToolProps) => { comparisonFileId: nextComp, })); } - }, [base.selectedFiles, base.params, params.baseFileId, params.comparisonFileId]); + }, [fileState.ui.selectedFileIds, base.params, params.baseFileId, params.comparisonFileId]); // Track workbench data and drive loading/result state transitions const lastProcessedAtRef = useRef(null); @@ -310,48 +309,7 @@ const Compare = (props: BaseToolProps) => { } }, [base.params, operation.result, params.baseFileId, params.comparisonFileId, runCompareWithIds]); - // Custom file handler that selects existing files instead of adding duplicates - const handleFileSelection = useCallback(async (files: File[]) => { - const allStubs = selectors.getStirlingFileStubs(); - const existingFilesByQuickKey = new Map(); - - // Build a map of quickKey -> fileId for all existing files - allStubs.forEach(stub => { - if (stub.quickKey) { - existingFilesByQuickKey.set(stub.quickKey, stub.id); - } - }); - - const filesToAdd: File[] = []; - const existingFileIds: FileId[] = []; - - // Check each file to see if it already exists - for (const file of files) { - const quickKey = createQuickKey(file); - const existingFileId = existingFilesByQuickKey.get(quickKey); - - if (existingFileId) { - // File exists, just select it - existingFileIds.push(existingFileId); - } else { - // File doesn't exist, add it - filesToAdd.push(file); - } - } - - // Add new files (they will be auto-selected by default) - if (filesToAdd.length > 0) { - await addFiles(filesToAdd, { selectFiles: true }); - } - - // If we found existing files, merge them with current selection - if (existingFileIds.length > 0) { - // After adding new files, get the updated selection and merge with existing file IDs - const updatedSelection = selectors.getSelectedStirlingFileStubs().map(s => s.id); - const allToSelect = [...new Set([...updatedSelection, ...existingFileIds])]; - fileActions.setSelectedFiles(allToSelect); - } - }, [addFiles, fileActions, selectors]); + // No custom handler; rely on global add flow which auto-selects added files const handleSwap = useCallback(() => { const baseId = params.baseFileId as FileId | null; @@ -389,7 +347,7 @@ const Compare = (props: BaseToolProps) => { justifyContent: 'space-between', cursor: shouldShowAddButton ? 'pointer' : 'default', }} - onClick={shouldShowAddButton ? () => openFilesModal({ customHandler: handleFileSelection }) : undefined} + onClick={shouldShowAddButton ? () => openFilesModal({}) : undefined} > {t( @@ -404,7 +362,7 @@ const Compare = (props: BaseToolProps) => { size="sm" onClick={(e) => { e.stopPropagation(); - openFilesModal({ customHandler: handleFileSelection }); + openFilesModal({}); }} style={{ flexShrink: 0 }} > @@ -462,11 +420,19 @@ const Compare = (props: BaseToolProps) => { ); }, - [params.baseFileId, params.comparisonFileId, selectors, t, openFilesModal, handleFileSelection] + [params.baseFileId, params.comparisonFileId, selectors, t, openFilesModal] ); + const baseStub = params.baseFileId ? selectors.getStirlingFileStub(params.baseFileId) : undefined; + const compStub = params.comparisonFileId ? selectors.getStirlingFileStub(params.comparisonFileId) : undefined; const canExecute = Boolean( - params.baseFileId && params.comparisonFileId && params.baseFileId !== params.comparisonFileId && !base.operation.isLoading && base.endpointEnabled !== false + params.baseFileId && + params.comparisonFileId && + params.baseFileId !== params.comparisonFileId && + baseStub && + compStub && + !base.operation.isLoading && + base.endpointEnabled !== false ); const hasBothSelected = Boolean(params.baseFileId && params.comparisonFileId);