Skip to content

feat(pdf): add PDF editing tools to the donut - #428

Merged
KnifeLemon merged 7 commits into
KnifeLemon:mainfrom
Perricheno:feature/pdf-edit-tools
Oct 24, 2026
Merged

KnifeLemon merged 7 commits into
KnifeLemon:mainfrom
Perricheno:feature/pdf-edit-tools

Conversation

@Perricheno

@Perricheno Perricheno commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

What does this change?

Adds one Edit group to the back of the PDF donut. It fans out to six actions: Crop, Rotate left, Rotate right, Pages, Extract, and Add PDF.

Crop changes PDF crop boxes, rotations use the page /Rotate value, and the page organiser copies PDF pages directly, so text and vector content are not rasterised. The organiser can reorder, delete, rotate, add, and extract pages. Existing libraries receive the new group through schema migration 33 without replacing customised donut layouts.

How was it tested?

  • dotnet format --verify-no-changes (Core, Engines, and App projects)
  • dotnet test (the complete UI suite includes unrelated repository-root/platform tests)
  • Targeted validation: 458 Core tests, 60 localisation tests, 4 low-level PDF tests, and 6 UI/integration tests
    • PDF crop/rotation/page-copy coverage includes rotated pages and compression
    • UI/integration coverage includes add/reorder/rotate/delete/save, donut availability and migration
  • Tried it in the app: rendered the real PDF donut and page editor headlessly, including the page thumbnails and selected-page preview

Screenshots (UI changes)

Edit stays inside the PDF donut. Selecting it fans out the six small actions, so direct operations remain one click away and only Pages / Add open the organiser.

PDF donut with Edit expanded into Crop, Left, Right, Pages, Extract and Add

The organiser now renders real thumbnails for every page and a large preview of the selected page while reordering, rotating, deleting, extracting, or combining PDFs.

PDF page editor with page thumbnails, a selected-page preview and all editing actions

Both screenshots are produced by headless UI tests at the controls' real sizes.

Checklist

  • New UI strings are in all src/Filee.App/Assets/i18n/*.json files
  • No new third-party code or binaries were added

@Perricheno Perricheno changed the title Add PDF editing tools to the donut feat(pdf): add PDF editing tools to the donut Oct 22, 2026
@Perricheno

Copy link
Copy Markdown
Contributor Author

@KnifeLemon Small admin note: this is a feature PR. I opened it before noticing that the repository label workflow runs only once on opened and expects a Conventional Commits title, and fork authors cannot add labels manually. Could you please add the feature label when you get a chance? Thanks!

@KnifeLemon KnifeLemon left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Perricheno, and I've added the feature label. This is careful work: the page tools keep text and vectors as PDF objects, the editor renders its previews off the UI thread, and the migration leaves customised donuts alone. CI is green. A few things before it can go in:

  1. Bookmarks, form fields and metadata. Rotate left/right go through SplitOrExtract, and Crop through PdfPageTools.CropAll; both copy the pages into a new PdfDocument. For a whole-document rotate or crop that drops the outline, AcroForm fields, page labels and document info that CompressAsync is careful to keep. When every page is kept, please open the file with PdfDocumentOpenMode.Modify and change /Rotate and the crop box in place.
  2. Split shown as "Extract". preset.split_pdf.short makes the existing Split PDF (one file per page) read "Extract" on the Edit ring. Please keep it as "Split".
  3. Large PDFs. PdfPageTools.Preview reads the whole file and all page sizes again for every thumbnail, so a 500-page PDF is read 500 times. Please open the document once per editor (or cache the bytes and sizes). The editor's constructor also counts the pages on the UI thread before the window shows.
  4. Tools in groups. EnsureConsistency and the group editor now allow every tool in a group, the image Crop tool included. If that's intended, docs/ARCHITECTURE.md should say so; its "Crop overlay" section also needs the PDF crop and the new page editor.
  5. docs/ENGINES.md: the PDFsharp row should mention rotating, cropping and page organising.
  6. ko.json: pdf_editor.hint ends in 합니다체 ("추출할 수 있습니다"); the app uses 해요체 ("추출할 수 있어요").

Settings schema 33 is already taken by earlier PRs. We'll renumber the migration when we merge, so nothing to do there.

@Perricheno

Copy link
Copy Markdown
Contributor Author

Addressed all six points in b0f9c92.

  1. Whole-document rotate and crop now copy the original PDF and reopen that copy with PdfDocumentOpenMode.Modify. Only /Rotate or the crop boxes are changed. This path is used by the page editor, the normal rotate presets, and rotate + compression; page rebuilding remains only for an actual split, extract, deletion, reorder, or multi-file edit. The new tests create and verify a real outline, an AcroForm field, a page-label number tree, and document title/author after both rotate and crop.
  2. preset.split_pdf.short now says “Split” (with the corresponding translation in every shipped locale); the editor’s separate one-page action remains “Extract”.
  3. Each editor source now has one PdfPreviewSource snapshot: its bytes and all page sizes are cached once and reused for thumbnails and the large preview. Initial PDFs are opened asynchronously after the window appears, so the constructor no longer parses or counts pages on the UI thread. A cache test renders two pages after the backing file has been removed, proving that neither render rereads it.
  4. docs/ARCHITECTURE.md now documents conversion/tool members in groups, PDF crop, the page editor, the modify-vs-rebuild rule, and preview caching.
  5. The PDFsharp row in docs/ENGINES.md now includes page organising, lossless rotation, and crop-box editing.
  6. The Korean hint now uses 해요체: 추출할 수 있어요.

Local focused checks:

  • PDF page tools and structure preservation: 9/9
  • PDF page tools + compression: 24 passed, 1 environment-dependent Ghostscript test skipped
  • editor and group UI tests: 11/11
  • localisation/resource tests: 65/65
  • Release app build: successful

The broader engine run reached 1,432 passed and 50 skipped; its five local failures are unrelated installed-tool/font/codec differences (LibreOffice, Pandoc priority, Korean CAD font, and FFmpeg libspeex), while all affected PDF tests pass.

@KnifeLemon KnifeLemon left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Perricheno, all six points are in, and done well: whole-document rotate and crop now change /Rotate and the crop box in a copy opened in modify mode, the tests check a real outline, form field, page labels and document info, Split reads "Split" again in every language, the editor opens its PDFs once after the window is up and reuses one snapshot per file, the docs cover groups, PDF crop and the page editor, and the Korean hint is in 해요체. On b0f9c92 merged with today's main: PdfPageToolsTests 9/9, the editor, toolbar and localization tests pass apart from Nepali (see the end), the Core settings tests 94/94, and dotnet format is clean.

The new modify-in-place path brings two problems, and both are easy to hit with real files:

  1. PDFs with permission restrictions. Many PDFs (bank statements, invoices, e-books) are encrypted with only an owner password: they open without a password but don't allow changes. ModifyCopy opens the copy with PdfDocumentOpenMode.Modify, which PDFsharp refuses for those, and PdfSharpConverter.Open turns that into "The PDF is password-protected…". I tried a two-page PDF with only an owner password: the Rotate right preset (with and without compression) and PDF Crop both fail now, while the page-copy path in the previous commit handled it. PdfSharpConverter.OpenForChanges already has the answer for compression: when Modify fails because of the owner password, copy the pages into a new document. Using the same fallback in ModifyCopy keeps these files working (their outline and forms are lost, as before). The page editor's whole-document rotate goes through the same helper.
  2. Read-only source files. File.Copy keeps the read-only attribute, so for a read-only PDF (from a USB stick or CD, or a folder that marks files read-only) saving the copy fails with "Access to the path … is denied". I got that for both PDF Crop and the rotate preset. The no-rotation shortcut (degrees == 0 → File.Copy) would also hand the user a read-only output. Please clear FileAttributes.ReadOnly on the copy, or copy the bytes through a stream instead of File.Copy.

A test for each (an owner-password-only PDF and a read-only source), next to the structure tests, would keep them covered.

About #437: both PRs now carry a cached PDF preview class that does the same job, PdfPreviewSource here and PdfPreviewDocument in #437 (that one also renders the whole thumbnail strip in one PDFium load). Since both PRs are yours, could you keep one of them in whichever PR lands first and have the other use it when it's rebased? The two branches also conflict in 8 files where both add a tool (docs/ARCHITECTURE.md, docs/ENGINES.md, PresetAvailability, RadialController, WindowService, BuiltInData, AppSettings, UserDataStore), so the second one needs a rebase anyway.

Nepali (ne.json) was merged into main after your last push, so Every_language_has_exactly_the_english_keys(ne) now fails on the 19 new keys once main is merged in. That's main moving under you: we'll add the Nepali strings ourselves when merging, and renumber the settings schema too, so nothing to do there.

@Perricheno

Copy link
Copy Markdown
Contributor Author

Thanks for catching both cases. Fixed in 85d529f:

  • ModifyCopy now uses the same OpenForChanges helper as compression. Ordinary whole-document edits still preserve the catalogue, outlines, forms, labels and information; owner-password-only PDFs fall back to importing their pages, with the existing limitation that outlines/forms are not retained. PDFs that require an opening password still fail with the usual password message.
  • Generated copies clear FileAttributes.ReadOnly. This covers crop, editor rotation, the no-rotation shortcut, and both compression output choices. The original bytes and original read-only attribute remain unchanged.
  • Added nine regression cases covering crop and whole-document editor rotation with owner restrictions, a read-only source, and both together; rotate presets with/without compression; and read-only no-rotation output with/without compression. The existing structure-preservation tests remain green.

Validation: PDF page tools and compression tests: 33 passed, 1 skipped because Ghostscript is not installed. Formatting checks for the changed engine and test files are clean.

Agreed about the shared preview code. PdfPreviewDocument from #437 is the better base because it also renders the thumbnail strip in one PDFium load. Once the first PR lands, I will rebase the other, remove its duplicate preview class and adapt its callers to that one implementation while resolving the shared tool-registration conflicts. This push keeps the two PRs independently mergeable. Thanks for handling Nepali and the schema numbering on merge.

@KnifeLemon KnifeLemon left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Perricheno, both fixes are right: ModifyCopy goes through OpenForChanges (an owner-password-only PDF falls back to copying its pages, one that needs a password to open still gets the usual message), and CopyWritable clears the read-only attribute on every generated copy (crop, editor rotation, the no-rotation shortcut, both compression outcomes), with nine regression tests. On 85d529f: PdfPageToolsTests and the compression tests 34/34 (Ghostscript included), the editor, crop, group, donut and localization tests 168/168, Core 455/458, and dotnet format is clean.

The next step is the rebase you offered: #437 goes in first (with #429 just before it), then please rebase this onto main, drop PdfPreviewSource and use PdfPreviewDocument, and register the PDF tools next to the sheet studio. Things main changed under this branch that the rebase will meet:

  • PdfSharpConverter: main now has a WithoutMetadata shortcut for "Remove metadata" that also uses OpenForChanges. Keep both of your catch clauses there, and add Rotation: PdfRotation.None to that shortcut's pattern so a preset that rotates and removes metadata isn't sent down the metadata-only path.
  • The PDF donut's back face now has "Remove metadata" (schema 37); with "Edit" first it has 11 slices, which fits.
  • If #439 (Cut a clip) is merged before your rebase, tools are told apart by Preset.Tool there, and IsCropTool counts a tool without one as the crop. Keep the tool-pdf-* check ahead of the crop check, in PresetAvailability and where the window is opened.

Don't worry about Nepali, Thai or the schema number: we'll add the 19 strings to ne.json and th.json and renumber when merging. I'll approve after the rebase.

To get this into the next release, we'll push the remaining changes to your branch ourselves today (maintainer edits are on). Have a look at the commits when they land, and tell us if anything looks off.

KnifeLemon and others added 2 commits October 24, 2026 10:10
The PDF Edit group's settings migration moves after main's schema 43 to 46
(USM takes 44 and Cut a clip 45, both going into main first). The new tools
sit after the spreadsheet print studio in the library, and "Edit" is first on
the PDF donut's back face next to main's "Remove metadata". Tool dispatch
and availability keep the print studio. Nepali and Thai strings for the
PDF page tools.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Cut a clip (KnifeLemon#439) is schema 45 and USM 44 on main; the PDF Edit group stays
at 46. WindowService.ShowTool opens the page editor for "Reorder / delete
pages" and "Add pages" before the crop (their presets have no Tool of their
own, which reads as a crop; "Crop PDF" is one), and PresetAvailability
judges the PDF tools before the image crop. Turkish strings for the PDF
page tools. The palette tests now expect "Edit" among the built-in groups
and leave the PDF tools out where a single kind is the point.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@KnifeLemon
KnifeLemon merged commit e492ec2 into KnifeLemon:main Oct 24, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants