mirror of
https://github.com/arsvendg/Stirling-PDF.git
synced 2026-07-16 19:33:11 +02:00
Add specific View Scope For Selected Files (#6050)
## Fix 1 — Viewer bug (8 tools) 8 tools called `useFileSelection()` directly instead of routing through `useBaseTool`. In the viewer, this meant they operated on **all selected files** instead of only the one being viewed. For example: 10 files loaded, viewing file 3, running Add Stamp — all 10 files got stamped. **Root cause:** These tools had no view-scope awareness. `useFileSelection()` returns the raw workbench selection with no knowledge of which file is active in the viewer. **Fix:** A new hook `useViewScopedFiles` was introduced: ```ts // Viewer → only the active file // Everywhere else → all loaded files const selectedFiles = useViewScopedFiles(); ``` The 8 tools were updated to call this instead of `useFileSelection()`. **Tools fixed:** Add Stamp, Add Watermark, Add Password, Add Page Numbers, Add Attachments, Reorganize Pages, OCR, Convert --- ## Fix 2 — Page selector / active files context (all tools) `useBaseTool` returned `selectedFiles` (checked files only) in non-viewer contexts. In the page selector this is typically empty or stale — not the full set of loaded files that tools should operate on. **Fix:** `useBaseTool` was updated to use `useViewScopedFiles`, which returns all loaded files in non-viewer contexts. This affected every tool via `useBaseTool`. --- ## Workarounds for Compare & Merge Two tools intentionally need all loaded files regardless of view, so they use `ignoreViewerScope: true` in `useBaseTool`. **Compare** — needs exactly 2 files for its Original/Edited slots. Scoping to one file would break the comparison entirely. `ignoreViewerScope: true` is set and `disableScopeHints: true` hides the "(this file)" button label hint. The slot auto-mapping logic was also improved alongside this fix. **Merge** — needs 2+ files; merging a single file is meaningless. Rather than leaving the button silently disabled, Merge now: - Auto-redirects to the active files view on first open from the viewer - If the user navigates back to the viewer, shows a disabled button with a hint and a "Go to active files view" shortcut button --- ## How to Test --- ## Fix 1 — 8 tools (viewer scoping) ### Test steps (same for each) 1. Load 3 PDFs into workbench 2. Open viewer, navigate to file 2 3. Open the tool, configure settings, run 4. ✅ Only file 2 is in the results 5. ✅ Button label shows **"[Action] (this file)"** 6. ✅ A note below the button reads **"Only applying to: [filename]"** | Tool | What to configure | |---|---| | **Add Stamp** | Enter any text stamp or upload an image stamp | | **Add Watermark** | Select text watermark, enter any text | | **Add Page Numbers** | Leave defaults | | **Add Password** | Enter any owner + user password | | **Add Attachments** | Attach any small file | | **Reorganize Pages** | Enter a page range e.g. `1,2` | | **OCR** | Leave default language | | **Convert** | Convert PDF → any format | --- ## Fix 2 — All tools (page selector context) ### Test steps 1. Load 3 PDFs into workbench 2. Open the page selector view 3. Open any tool from the sidebar, run it 4. ✅ All 3 files are processed (not zero or a stale subset) --- ## Compare (intentionally ignores view scope) **A — Auto-fill with exactly 2 files** 1. Load exactly 2 PDFs 2. Open Compare from either the viewer or active files view 3. ✅ Both slots are filled automatically (Original + Edited) 4. ✅ No scope hint appears on the button **B — Manual selection with 3+ files** 1. Load 3+ PDFs 2. Open Compare 3. ✅ The first 2 files fill the slots 4. ✅ A 3rd file does not add a 3rd slot (capped at 2) **C — File removed mid-session** 1. Load 2 PDFs, let Compare auto-fill both slots 2. Remove one file from the workbench 3. ✅ The corresponding slot clears; the other slot is unchanged **D — Viewer mode** 1. Load 2 PDFs, open viewer 2. Open Compare from the viewer sidebar 3. ✅ Both files are still available for slot selection (not scoped to current file) --- ## Merge (intentionally ignores view scope, disabled in viewer) **A — Auto-redirect on first open from viewer** 1. Load 2+ PDFs, open the viewer 2. Open Merge from the viewer sidebar 3. ✅ Immediately redirected to the active files view **B — Viewer mode disabled state (after navigating back)** 1. From the active files view, open Merge, then navigate back to the viewer 2. ✅ Execute button is **disabled** with tooltip "Switch to the file editor to select multiple files" 3. ✅ A note appears: *"Merge needs 2 or more files. Head to the file editor to select them."* 4. ✅ A **"Go to active files view"** button is shown; clicking it navigates back **C — Active files view works normally** 1. Load 3 PDFs, open Merge from the active files view 2. ✅ All 3 files appear in the merge list 3. ✅ Button shows **"Merge (3 files)"** 4. Run the merge 5. ✅ Output is a single PDF containing all 3 files --- ## Button label behaviour (all tools) | Context | Expected button text | |---|---| | Viewer, 1 file loaded | `[Action]` (no suffix) | | Viewer, 2+ files loaded | `[Action] (this file)` | | Active files view, 1 file loaded | `[Action]` (no suffix) | | Active files view, 2+ files loaded | `[Action] (N files)` | | Merge in viewer | disabled — no suffix | | Compare | never shows scope suffix (`disableScopeHints: true`) | --------- Co-authored-by: Reece Browne <[email protected]>
This commit is contained in:
co-authored by
Reece Browne
parent
3c48740c5e
commit
917edc43b3
@@ -52,8 +52,8 @@ const FileEditor = ({
|
||||
// Get navigation actions
|
||||
const { actions: navActions } = useNavigationActions();
|
||||
|
||||
// Get viewer context for setting active file index
|
||||
const { setActiveFileIndex } = useViewer();
|
||||
// Get viewer context for setting active file index and ID
|
||||
const { setActiveFileIndex, setActiveFileId } = useViewer();
|
||||
|
||||
// Get file selection context
|
||||
const { setSelectedFiles } = useFileSelection();
|
||||
@@ -351,12 +351,11 @@ const FileEditor = ({
|
||||
const handleViewFile = useCallback((fileId: FileId) => {
|
||||
const index = activeStirlingFileStubs.findIndex(r => r.id === fileId);
|
||||
if (index !== -1) {
|
||||
// Set the file as selected in context, sync the viewer index, and switch to viewer
|
||||
setSelectedFiles([fileId]);
|
||||
setActiveFileId(fileId as string);
|
||||
setActiveFileIndex(index);
|
||||
navActions.setWorkbench('viewer');
|
||||
}
|
||||
}, [activeStirlingFileStubs, setSelectedFiles, setActiveFileIndex, navActions.setWorkbench]);
|
||||
}, [activeStirlingFileStubs, setActiveFileId, setActiveFileIndex, navActions.setWorkbench]);
|
||||
|
||||
const handleLoadFromStorage = useCallback(async (selectedFiles: File[]) => {
|
||||
if (selectedFiles.length === 0) return;
|
||||
|
||||
@@ -26,11 +26,11 @@ import { formatFileSize } from '@app/utils/fileUtils';
|
||||
import ToolChain from '@app/components/shared/ToolChain';
|
||||
import HoverActionMenu, { HoverAction } from '@app/components/shared/HoverActionMenu';
|
||||
import { downloadFile } from '@app/services/downloadService';
|
||||
import FileEditorFileName from '@app/components/fileEditor/FileEditorFileName';
|
||||
import { PrivateContent } from '@app/components/shared/PrivateContent';
|
||||
import UploadToServerModal from '@app/components/shared/UploadToServerModal';
|
||||
import ShareFileModal from '@app/components/shared/ShareFileModal';
|
||||
import { useAppConfig } from '@app/contexts/AppConfigContext';
|
||||
import { truncateCenter } from '@app/utils/textUtils';
|
||||
|
||||
|
||||
|
||||
@@ -461,8 +461,8 @@ const FileEditorThumbnail = ({
|
||||
marginTop: '0.5rem',
|
||||
marginBottom: '0.5rem',
|
||||
}}>
|
||||
<Text size="lg" fw={700} className={styles.title} lineClamp={2} style={{ display: 'flex', alignItems: 'center', justifyContent: 'center', gap: '0.25rem' }}>
|
||||
<FileEditorFileName file={file} />
|
||||
<Text size="lg" fw={700} className={styles.title} title={file.name}>
|
||||
<PrivateContent>{truncateCenter(file.name, 40)}</PrivateContent>
|
||||
</Text>
|
||||
<Text
|
||||
size="sm"
|
||||
|
||||
@@ -6,16 +6,7 @@ import CloseIcon from '@mui/icons-material/Close';
|
||||
import FitText from '@app/components/shared/FitText';
|
||||
import { PrivateContent } from '@app/components/shared/PrivateContent';
|
||||
import { FileId } from '@app/types/file';
|
||||
|
||||
// Truncate text from the center: "very-long-filename.pdf" -> "very-lo...ame.pdf"
|
||||
function truncateCenter(text: string, maxLength: number = 25): string {
|
||||
if (text.length <= maxLength) return text;
|
||||
const ellipsis = '...';
|
||||
const charsToShow = maxLength - ellipsis.length;
|
||||
const frontChars = Math.ceil(charsToShow / 2);
|
||||
const backChars = Math.floor(charsToShow / 2);
|
||||
return text.substring(0, frontChars) + ellipsis + text.substring(text.length - backChars);
|
||||
}
|
||||
import { truncateCenter } from '@app/utils/textUtils';
|
||||
|
||||
interface FileDropdownMenuProps {
|
||||
displayName: string;
|
||||
|
||||
@@ -53,6 +53,7 @@ const OperationButton = ({
|
||||
: t('tool.endpointUnavailable', 'This tool is unavailable on your server.'),
|
||||
noFiles: t('tool.noFiles', 'Add a file to get started.'),
|
||||
invalidParams: t('tool.invalidParams', 'Fill in the required settings.'),
|
||||
viewerMode: t('tool.viewerMode', 'Switch to the file editor to select multiple files.'),
|
||||
};
|
||||
|
||||
const tooltipLabel = blockedByBackend
|
||||
|
||||
@@ -0,0 +1,66 @@
|
||||
import { Text } from '@mantine/core';
|
||||
import { useTranslation } from 'react-i18next';
|
||||
import OperationButton, { OperationButtonProps } from '@app/components/tools/shared/OperationButton';
|
||||
import { StirlingFile } from '@app/types/fileContext';
|
||||
import { useAllFiles } from '@app/contexts/FileContext';
|
||||
import { useViewer } from '@app/contexts/ViewerContext';
|
||||
import { useNavigationState } from '@app/contexts/NavigationContext';
|
||||
|
||||
export interface ScopedOperationButtonProps extends OperationButtonProps {
|
||||
selectedFiles: StirlingFile[];
|
||||
disableScopeHints?: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* Wraps OperationButton with scope-aware button text and a filename note.
|
||||
*
|
||||
* - Viewer mode (multiple files loaded): appends "(this file)" to button text and
|
||||
* shows a note naming the exact file that will be processed.
|
||||
* - File-editor mode with N>1 selected files: appends "(N files)" to button text.
|
||||
* - File-editor mode with 0 selected files: shows a hint to select files.
|
||||
* - All other cases: no change to button text or layout.
|
||||
*/
|
||||
export function ScopedOperationButton({ selectedFiles, disableScopeHints, ...props }: ScopedOperationButtonProps) {
|
||||
const { t } = useTranslation();
|
||||
const { workbench } = useNavigationState();
|
||||
const { activeFileIndex } = useViewer();
|
||||
const { files: allFiles } = useAllFiles();
|
||||
|
||||
const isViewerMode = workbench === 'viewer';
|
||||
const isFileEditorMode = workbench === 'fileEditor';
|
||||
const hasMultipleFilesLoaded = allFiles.length > 1;
|
||||
const baseText = props.submitText ?? t('submit', 'Submit');
|
||||
|
||||
const disabledForViewerMode = props.disabledReason === 'viewerMode';
|
||||
|
||||
let scopedText = baseText;
|
||||
if (!disableScopeHints && !disabledForViewerMode) {
|
||||
if (isViewerMode && hasMultipleFilesLoaded) {
|
||||
scopedText = `${baseText} (${t('tool.scopeThisFile', 'this file')})`;
|
||||
} else if (!isViewerMode && selectedFiles.length > 1) {
|
||||
scopedText = `${baseText} (${selectedFiles.length} ${t('tool.scopeFiles', 'files')})`;
|
||||
}
|
||||
}
|
||||
|
||||
const viewerFileName = !disableScopeHints && !disabledForViewerMode && isViewerMode && hasMultipleFilesLoaded
|
||||
? allFiles[activeFileIndex]?.name
|
||||
: null;
|
||||
|
||||
const showSelectFilesHint = !disableScopeHints && isFileEditorMode && allFiles.length > 0 && selectedFiles.length === 0;
|
||||
|
||||
return (
|
||||
<>
|
||||
<OperationButton {...props} submitText={scopedText} />
|
||||
{viewerFileName && (
|
||||
<Text size="xs" c="dimmed" ta="center" mx="md" mt={2}>
|
||||
{t('tool.singleFileScope', 'Only applying to: {{fileName}}', { fileName: viewerFileName })}
|
||||
</Text>
|
||||
)}
|
||||
{showSelectFilesHint && (
|
||||
<Text size="xs" c="dimmed" ta="center" mx="md" mt={2}>
|
||||
{t('tool.selectFilesHint', 'Select files in Active Files to run this tool')}
|
||||
</Text>
|
||||
)}
|
||||
</>
|
||||
);
|
||||
}
|
||||
@@ -1,7 +1,7 @@
|
||||
import React from 'react';
|
||||
import { Stack } from '@mantine/core';
|
||||
import { createToolSteps, ToolStepProvider } from '@app/components/tools/shared/ToolStep';
|
||||
import OperationButton from '@app/components/tools/shared/OperationButton';
|
||||
import { ScopedOperationButton } from '@app/components/tools/shared/ScopedOperationButton';
|
||||
import { ToolOperationHook } from '@app/hooks/tools/shared/useToolOperation';
|
||||
import { ToolWorkflowTitle, ToolWorkflowTitleProps } from '@app/components/tools/shared/ToolWorkflowTitle';
|
||||
import { StirlingFile } from '@app/types/fileContext';
|
||||
@@ -57,6 +57,8 @@ export interface ExecuteButtonConfig {
|
||||
disabled?: boolean;
|
||||
testId?: string;
|
||||
showCloudBadge?: boolean;
|
||||
/** Suppress the automatic "(this file)" / "(N files)" scope hints in the button text. */
|
||||
disableScopeHints?: boolean;
|
||||
}
|
||||
|
||||
export interface ReviewStepConfig<TParams = unknown> {
|
||||
@@ -77,6 +79,8 @@ export interface ToolFlowConfig<TParams = unknown> {
|
||||
// Optional preview content rendered between steps and the execute button
|
||||
preview?: React.ReactNode;
|
||||
executeButton?: ExecuteButtonConfig;
|
||||
/** Optional content rendered immediately below the execute button (e.g. contextual help). */
|
||||
belowExecuteButton?: React.ReactNode;
|
||||
review: ReviewStepConfig<TParams>;
|
||||
forceStepNumbers?: boolean;
|
||||
}
|
||||
@@ -128,17 +132,22 @@ export function createToolFlow<TParams = unknown>(config: ToolFlowConfig<TParams
|
||||
: eb.paramsValid === false ? 'invalidParams'
|
||||
: null;
|
||||
return (
|
||||
<OperationButton
|
||||
onClick={eb.onClick}
|
||||
isLoading={config.review.operation.isLoading}
|
||||
disabled={eb.disabled}
|
||||
disabledReason={effectiveDisabledReason}
|
||||
loadingText={eb.loadingText}
|
||||
submitText={eb.text}
|
||||
showCloudBadge={eb.showCloudBadge ?? config.review.operation.willUseCloud ?? false}
|
||||
data-testid={eb.testId}
|
||||
data-tour="run-button"
|
||||
/>
|
||||
<>
|
||||
<ScopedOperationButton
|
||||
selectedFiles={config.files.selectedFiles ?? []}
|
||||
disableScopeHints={eb.disableScopeHints}
|
||||
onClick={eb.onClick}
|
||||
isLoading={config.review.operation.isLoading}
|
||||
disabled={eb.disabled}
|
||||
disabledReason={effectiveDisabledReason}
|
||||
loadingText={eb.loadingText}
|
||||
submitText={eb.text}
|
||||
showCloudBadge={eb.showCloudBadge ?? config.review.operation.willUseCloud ?? false}
|
||||
data-testid={eb.testId}
|
||||
data-tour="run-button"
|
||||
/>
|
||||
{config.belowExecuteButton}
|
||||
</>
|
||||
);
|
||||
})()}
|
||||
|
||||
|
||||
@@ -176,8 +176,6 @@ const EmbedPdfViewerContent = ({
|
||||
// Similar to scroll preservation - track rotation across file reloads
|
||||
const pendingRotationRestoreRef = useRef<number | null>(null);
|
||||
const rotationRestoreAttemptsRef = useRef<number>(0);
|
||||
// Track the file ID we should be viewing after a save (to handle list reordering)
|
||||
const pendingFileIdRef = useRef<string | null>(null);
|
||||
|
||||
const formApplyInProgressRef = useRef(false);
|
||||
|
||||
@@ -188,11 +186,12 @@ const EmbedPdfViewerContent = ({
|
||||
const redactionTrackerRef = useRef<RedactionPendingTrackerAPI>(null);
|
||||
|
||||
// Get current file from FileContext
|
||||
const { selectors, state } = useFileState();
|
||||
const { selectors } = useFileState();
|
||||
const { actions } = useFileActions();
|
||||
const activeFiles = selectors.getFiles();
|
||||
const activeFilesRef = useRef(activeFiles);
|
||||
activeFilesRef.current = activeFiles;
|
||||
const activeFileIds = activeFiles.map(f => f.fileId);
|
||||
const selectedFileIds = state.ui.selectedFileIds;
|
||||
|
||||
// Navigation guard for unsaved changes
|
||||
const { setHasUnsavedChanges, registerUnsavedChangesChecker, unregisterUnsavedChangesChecker, registerNavigationWarningHandlers, unregisterNavigationWarningHandlers } = useNavigationGuard();
|
||||
@@ -277,41 +276,46 @@ const EmbedPdfViewerContent = ({
|
||||
const [internalActiveFileIndex, setInternalActiveFileIndex] = useState(0);
|
||||
const activeFileIndex = externalActiveFileIndex ?? internalActiveFileIndex;
|
||||
const setActiveFileIndex = externalSetActiveFileIndex ?? setInternalActiveFileIndex;
|
||||
const hasInitializedFromSelection = useRef(false);
|
||||
|
||||
// When viewer opens with a selected file, switch to that file
|
||||
// activeFileId (from ViewerContext) is the stable source of truth.
|
||||
// We derive activeFileIndex from it so reorders after tool operations don't lose the viewed file.
|
||||
const { activeFileId, setActiveFileId } = useViewer();
|
||||
|
||||
// Stable string key representing the current file list order.
|
||||
// Using a joined ID string avoids depending on the activeFiles array reference,
|
||||
// which is a new object every render and would cause an infinite effect loop.
|
||||
const fileIdsKey = activeFiles.map(f => f.fileId).join(',');
|
||||
|
||||
// When the file list actually changes, re-derive activeFileIndex from the stable activeFileId.
|
||||
useEffect(() => {
|
||||
if (!hasInitializedFromSelection.current && selectedFileIds.length > 0 && activeFiles.length > 0) {
|
||||
const selectedFileId = selectedFileIds[0];
|
||||
const index = activeFiles.findIndex(f => f.fileId === selectedFileId);
|
||||
if (index !== -1 && index !== activeFileIndex) {
|
||||
setActiveFileIndex(index);
|
||||
}
|
||||
hasInitializedFromSelection.current = true;
|
||||
if (!activeFileId || activeFiles.length === 0) return;
|
||||
const newIndex = activeFiles.findIndex(f => f.fileId === activeFileId);
|
||||
if (newIndex !== -1 && newIndex !== activeFileIndex) {
|
||||
setActiveFileIndex(newIndex);
|
||||
}
|
||||
}, [selectedFileIds, activeFiles, activeFileIndex]);
|
||||
}, [fileIdsKey, activeFileId]); // stable primitives — no infinite loop
|
||||
|
||||
// Reset active tab if it's out of bounds
|
||||
// When the user manually switches file tabs, keep activeFileId in sync.
|
||||
// Skips the initial mount to avoid overwriting an activeFileId set by handleViewFile.
|
||||
const activeFileIndexMountedRef = useRef(false);
|
||||
useEffect(() => {
|
||||
if (!activeFileIndexMountedRef.current) {
|
||||
activeFileIndexMountedRef.current = true;
|
||||
return;
|
||||
}
|
||||
const fileId = activeFilesRef.current[activeFileIndex]?.fileId;
|
||||
if (fileId && fileId !== activeFileId) {
|
||||
setActiveFileId(fileId);
|
||||
}
|
||||
}, [activeFileIndex]);
|
||||
|
||||
// Reset active tab if it's out of bounds (safety net)
|
||||
useEffect(() => {
|
||||
if (activeFileIndex >= activeFiles.length && activeFiles.length > 0) {
|
||||
setActiveFileIndex(0);
|
||||
}
|
||||
}, [activeFiles.length, activeFileIndex]);
|
||||
|
||||
// After saving a file, the list may reorder (sorted by version).
|
||||
// Track the saved file's ID and update activeFileIndex to follow it.
|
||||
useEffect(() => {
|
||||
if (pendingFileIdRef.current && activeFiles.length > 0) {
|
||||
const targetFileId = pendingFileIdRef.current;
|
||||
const newIndex = activeFiles.findIndex(f => f.fileId === targetFileId);
|
||||
if (newIndex !== -1 && newIndex !== activeFileIndex) {
|
||||
setActiveFileIndex(newIndex);
|
||||
}
|
||||
// Clear the pending file ID once we've found and switched to it
|
||||
pendingFileIdRef.current = null;
|
||||
}
|
||||
}, [activeFiles, activeFileIndex, setActiveFileIndex]);
|
||||
|
||||
// Determine which file to display
|
||||
const currentFile = React.useMemo(() => {
|
||||
if (previewFile) {
|
||||
@@ -627,11 +631,9 @@ const EmbedPdfViewerContent = ({
|
||||
// Store the rotation to restore after file replacement
|
||||
pendingRotationRestoreRef.current = currentRotation;
|
||||
rotationRestoreAttemptsRef.current = 0;
|
||||
// Store the new file ID so we can track it after the list reorders
|
||||
// Track the new file ID so the viewer follows it after the list reorders
|
||||
const newFileId = stubs[0]?.id;
|
||||
if (newFileId) {
|
||||
pendingFileIdRef.current = newFileId;
|
||||
}
|
||||
if (newFileId) setActiveFileId(newFileId);
|
||||
|
||||
// Step 4: Consume only the current file (replace in context)
|
||||
await actions.consumeFiles([currentFileId], stirlingFiles, stubs);
|
||||
@@ -682,11 +684,9 @@ const EmbedPdfViewerContent = ({
|
||||
pendingRotationRestoreRef.current = currentRotation;
|
||||
rotationRestoreAttemptsRef.current = 0;
|
||||
|
||||
// Store the new file ID for tracking
|
||||
// Track the new file ID so the viewer follows it after the list reorders
|
||||
const newFileId = stubs[0]?.id;
|
||||
if (newFileId) {
|
||||
pendingFileIdRef.current = newFileId;
|
||||
}
|
||||
if (newFileId) setActiveFileId(newFileId);
|
||||
|
||||
// Replace the current file in context
|
||||
await actions.consumeFiles([currentFileId], stirlingFiles, stubs);
|
||||
@@ -738,9 +738,7 @@ const EmbedPdfViewerContent = ({
|
||||
rotationRestoreAttemptsRef.current = 0;
|
||||
|
||||
const newFileId = stubs[0]?.id;
|
||||
if (newFileId) {
|
||||
pendingFileIdRef.current = newFileId;
|
||||
}
|
||||
if (newFileId) setActiveFileId(newFileId);
|
||||
|
||||
await actions.consumeFiles([currentFileId], stirlingFiles, stubs);
|
||||
} catch (error) {
|
||||
|
||||
Reference in New Issue
Block a user