Skip to content

Commit 26ceaee

Browse files
EthanHealy01EthanHealy01
committed
handle page editor view scoping
1 parent 2873e44 commit 26ceaee

File tree

3 files changed

+96
-89
lines changed

3 files changed

+96
-89
lines changed

‎docs/testing/viewer-scope-testing.md‎

Lines changed: 79 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -1,69 +1,69 @@
1-
# Viewer Scope Testing Guide
1+
# View Scope Testing Guide
22

3-
Tests the fix ensuring tools only process the **currently viewed file** when
4-
triggered from the viewer, not all selected files.
3+
## Fix 1 — Viewer bug (8 tools)
54

6-
## Setup (applies to all tests)
5+
8 tools called `useFileSelection()` directly instead of routing through
6+
`useBaseTool`. In the viewer, this meant they operated on **all selected files**
7+
instead of only the one being viewed. For example: 10 files loaded, viewing
8+
file 3, running Add Stamp — all 10 files got stamped.
79

8-
1. Load **3+ files** into the workbench
9-
2. Select all of them
10-
3. Open the viewer and navigate to a **specific file** (not the first one)
11-
4. Open the tool from the viewer sidebar
12-
5. Run the operation
13-
6. **Expected**: only the viewed file is processed; other files are unchanged
10+
**Root cause:** These tools had no viewer-scope awareness. `useFileSelection()`
11+
returns the raw workbench selection with no knowledge of which file is active in
12+
the viewer.
13+
14+
**Fix:** A new hook `useViewScopedFiles` was introduced:
15+
16+
```ts
17+
// Viewer → only the active file
18+
// Everywhere else → all loaded files
19+
const selectedFiles = useViewScopedFiles();
20+
```
21+
22+
The 8 tools were updated to call this instead of `useFileSelection()`.
23+
24+
**Tools fixed:** Add Stamp, Add Watermark, Add Password, Add Page Numbers,
25+
Add Attachments, Reorganize Pages, OCR, Convert
1426

1527
---
1628

17-
## Category 1 — Tools using `useBaseTool` (were already correct)
29+
## Fix 2 — Page selector / active files context (all tools)
1830

19-
These use `useBaseTool` which has always applied viewer scope. Smoke-test only
20-
to confirm no regression.
31+
`useBaseTool` returned `selectedFiles` (checked files only) in non-viewer
32+
contexts. In the page selector this is typically empty or stale — not the full
33+
set of loaded files that tools should operate on.
2134

22-
| Tool | Notes |
23-
|---|---|
24-
| Compress | Single-file tool |
25-
| Rotate | Single-file tool |
26-
| Split | Multi-page output |
27-
| Remove Pages | |
28-
| Extract Pages | |
29-
| Extract Images | |
30-
| Flatten | |
31-
| Repair | |
32-
| Sanitize | |
33-
| Remove Blanks | |
34-
| Remove Annotations | |
35-
| Remove Password | |
36-
| Add Certificate Sign | |
37-
| Change Metadata | |
38-
| Change Permissions | |
39-
| Crop | |
40-
| Adjust Contrast | |
41-
| Adjust Page Scale | |
42-
| Page Layout | |
43-
| Booklet Imposition | |
44-
| Single Large Page | |
45-
| Get PDF Info | |
46-
| Auto Rename | |
47-
| Overlay PDFs | |
48-
| Remove Image | |
49-
| Replace Color | |
50-
| Scanner Image Split | |
51-
| Timestamp PDF | |
52-
| Unlock PDF Forms | |
53-
| Validate Signature | |
54-
| Remove Certificate Sign | |
55-
| Redact | |
56-
| Edit Table of Contents | Uses `useBaseTool`; `useFileSelection` only for `clearSelections` |
57-
| Show JS | Uses `useBaseTool`; `useFileSelection` only for `clearSelections` |
35+
**Fix:** `useBaseTool` was updated to use `useViewScopedFiles`, which returns
36+
all loaded files in non-viewer contexts. This affected every tool via
37+
`useBaseTool`.
38+
39+
---
40+
41+
## Workarounds for Compare & Merge
42+
43+
Two tools intentionally need all loaded files regardless of view, so they use
44+
`ignoreViewerScope: true` in `useBaseTool`.
45+
46+
**Compare** — needs exactly 2 files for its Original/Edited slots. Scoping to
47+
one file would break the comparison entirely. `ignoreViewerScope: true` is set
48+
and `disableScopeHints: true` hides the "(this file)" button label hint. The
49+
slot auto-mapping logic was also improved alongside this fix.
50+
51+
**Merge** — needs 2+ files; merging a single file is meaningless. Rather than
52+
leaving the button silently disabled, Merge now:
53+
- Auto-redirects to the active files view on first open from the viewer
54+
- If the user navigates back to the viewer, shows a disabled button with a hint
55+
and a "Go to active files view" shortcut button
5856

5957
---
6058

61-
## Category 2 — Tools fixed in this PR (were broken, now fixed)
59+
## How to Test
6260

63-
These bypassed `useBaseTool` and have been updated to use `useViewerScopedFiles`.
61+
---
62+
63+
## Fix 1 — 8 tools (viewer scoping)
6464

6565
### Test steps (same for each)
66-
1. Load 3 PDFs into workbench, select all
66+
1. Load 3 PDFs into workbench
6767
2. Open viewer, navigate to file **#2**
6868
3. Open the tool, configure settings, run
6969
4. ✅ Only file #2 is in the results
@@ -83,24 +83,29 @@ These bypassed `useBaseTool` and have been updated to use `useViewerScopedFiles`
8383

8484
---
8585

86-
## Category 3 — Compare (intentionally ignores viewer scope)
86+
## Fix 2 — All tools (page selector context)
8787

88-
Compare always needs exactly 2 files and must never be scoped to a single viewer
89-
file. `ignoreViewerScope: true` is set explicitly.
88+
### Test steps
89+
1. Load 3 PDFs into workbench
90+
2. Open the page selector view, do **not** check any files
91+
3. Open any tool from the sidebar, run it
92+
4. ✅ All 3 files are processed (not zero or a stale subset)
9093

91-
### Tests
94+
---
95+
96+
## Compare (intentionally ignores view scope)
9297

9398
**A — Auto-fill with exactly 2 files**
9499
1. Load exactly 2 PDFs
95-
2. Open Compare from either the viewer or file editor
100+
2. Open Compare from either the viewer or active files view
96101
3. ✅ Both slots are filled automatically (Original + Edited)
97102
4. ✅ No scope hint appears on the button
98103

99104
**B — Manual selection with 3+ files**
100-
1. Load 3+ PDFs, select any 2
105+
1. Load 3+ PDFs
101106
2. Open Compare
102-
3. ✅ The 2 selected files fill the slots
103-
4. ✅ Selecting a 3rd file does not add a 3rd slot (capped at 2)
107+
3. ✅ The first 2 files fill the slots
108+
4. ✅ A 3rd file does not add a 3rd slot (capped at 2)
104109

105110
**C — File removed mid-session**
106111
1. Load 2 PDFs, let Compare auto-fill both slots
@@ -114,21 +119,21 @@ file. `ignoreViewerScope: true` is set explicitly.
114119

115120
---
116121

117-
## Category 4 — Merge (intentionally ignores viewer scope, disabled in viewer)
118-
119-
Merge requires 2+ files and is disabled in viewer mode with a redirect hint.
122+
## Merge (intentionally ignores view scope, disabled in viewer)
120123

121-
### Tests
122-
123-
**A — Viewer mode disabled state**
124-
1. Load 2+ PDFs, open viewer
124+
**A — Auto-redirect on first open from viewer**
125+
1. Load 2+ PDFs, open the viewer
125126
2. Open Merge from the viewer sidebar
126-
3. ✅ Execute button is **disabled** with tooltip "Switch to the file editor to select multiple files"
127-
4. ✅ A note appears below the button: *"Merge needs 2 or more files. Head to the file editor to select them."*
128-
5. ✅ A **"Go to file editor"** button is shown; clicking it navigates to the file editor
127+
3. ✅ Immediately redirected to the active files view
128+
129+
**B — Viewer mode disabled state (after navigating back)**
130+
1. From the active files view, open Merge, then navigate back to the viewer
131+
2. ✅ Execute button is **disabled** with tooltip "Switch to the file editor to select multiple files"
132+
3. ✅ A note appears: *"Merge needs 2 or more files. Head to the file editor to select them."*
133+
4. ✅ A **"Go to active files view"** button is shown; clicking it navigates back
129134

130-
**B — File editor mode works normally**
131-
1. Load 3 PDFs, select all, open Merge from the file editor
135+
**C — Active files view works normally**
136+
1. Load 3 PDFs, open Merge from the active files view
132137
2. ✅ All 3 files appear in the merge list
133138
3. ✅ Button shows **"Merge (3 files)"**
134139
4. Run the merge
@@ -142,7 +147,7 @@ Merge requires 2+ files and is disabled in viewer mode with a redirect hint.
142147
|---|---|
143148
| Viewer, 1 file loaded | `[Action]` (no suffix) |
144149
| Viewer, 2+ files loaded | `[Action] (this file)` |
145-
| File editor, 1 file selected | `[Action]` (no suffix) |
146-
| File editor, 2+ files selected | `[Action] (N files)` |
150+
| Active files view, 1 file loaded | `[Action]` (no suffix) |
151+
| Active files view, 2+ files loaded | `[Action] (N files)` |
147152
| Merge in viewer | disabled — no suffix |
148153
| Compare | never shows scope suffix (`disableScopeHints: true`) |

‎frontend/src/core/hooks/tools/shared/useBaseTool.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { useEffect, useCallback, useRef } from 'react';
2-
import { useFileSelection } from '@app/contexts/FileContext';
2+
import { useAllFiles } from '@app/contexts/FileContext';
33
import { useEndpointEnabled } from '@app/hooks/useEndpointConfig';
44
import { useViewScopedFiles } from '@app/hooks/tools/shared/useViewScopedFiles';
55
import { BaseToolProps } from '@app/types/tool';
@@ -53,14 +53,14 @@ export function useBaseTool<TParams, TParamsHook extends BaseParametersHook<TPar
5353
const ignoreViewerScope = options?.ignoreViewerScope ?? false;
5454
const { onPreviewFile, onComplete, onError } = props;
5555

56-
const { selectedFiles: rawSelectedFiles } = useFileSelection();
56+
const { files: allFiles } = useAllFiles();
5757
const viewerScopedFiles = useViewScopedFiles();
5858

5959
// In the viewer the tool always operates on the file currently shown, so that
60-
// "what you see is what gets processed". In the file editor the user has explicitly
61-
// chosen files via the selection UI, so we respect that full selection.
62-
// Tools that opt out via ignoreViewerScope always use the raw selection.
63-
const effectiveFiles = ignoreViewerScope ? rawSelectedFiles : viewerScopedFiles;
60+
// "what you see is what gets processed". Outside the viewer (pageEditor, fileEditor,
61+
// custom) tools operate on all loaded files. Tools with ignoreViewerScope also
62+
// receive all loaded files, bypassing the single-file viewer scoping.
63+
const effectiveFiles = ignoreViewerScope ? allFiles : viewerScopedFiles;
6464

6565
const previousFileCount = useRef(effectiveFiles.length);
6666

‎frontend/src/core/hooks/tools/shared/useViewScopedFiles.ts‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,29 @@
11
import { useMemo } from 'react';
2-
import { useFileSelection, useAllFiles } from '@app/contexts/FileContext';
2+
import { useAllFiles } from '@app/contexts/FileContext';
33
import { useViewer } from '@app/contexts/ViewerContext';
44
import { useNavigationState } from '@app/contexts/NavigationContext';
55
import { StirlingFile } from '@app/types/fileContext';
66

77
/**
8-
* Returns the effective file selection for tool operations.
8+
* Returns the effective file set for tool operations.
99
*
10-
* In viewer mode, scopes to the single file currently shown so that
11-
* "what you see is what gets processed". In file-editor mode the full
12-
* selection is returned unchanged.
10+
* - Viewer: scopes to the single file currently shown ("what you see is what gets processed").
11+
* - All other views (pageEditor, fileEditor, custom): returns all loaded files.
12+
*
13+
* Individual file selection is intentionally ignored outside the viewer — in views
14+
* like the page selector, selection tracks pages not files, and tools should
15+
* operate on the full active file set.
1316
*/
1417
export function useViewScopedFiles(): StirlingFile[] {
15-
const { selectedFiles } = useFileSelection();
1618
const { activeFileIndex } = useViewer();
1719
const { files: allFiles } = useAllFiles();
1820
const { workbench } = useNavigationState();
1921

2022
return useMemo(() => {
2123
if (workbench === 'viewer') {
2224
const viewerFile = allFiles[activeFileIndex];
23-
return viewerFile ? [viewerFile] : selectedFiles;
25+
return viewerFile ? [viewerFile] : allFiles;
2426
}
25-
return selectedFiles;
26-
}, [workbench, allFiles, activeFileIndex, selectedFiles]);
27+
return allFiles;
28+
}, [workbench, allFiles, activeFileIndex]);
2729
}

0 commit comments

Comments
 (0)