From 16491c00ce3429c359090857016a4e721386d372 Mon Sep 17 00:00:00 2001 From: Heiko Joerg Schick Date: Sun, 17 May 2026 09:00:22 +0200 Subject: [PATCH] Add IMPROVEMENTS.md with code audit findings across bugs, code quality, performance, and maintainability --- IMPROVEMENTS.md | 77 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 IMPROVEMENTS.md diff --git a/IMPROVEMENTS.md b/IMPROVEMENTS.md new file mode 100644 index 00000000..5df87a22 --- /dev/null +++ b/IMPROVEMENTS.md @@ -0,0 +1,77 @@ +# Improvements & Bugfixes + +--- + +## Critical Bugs + +| # | Issue | Location | +|---|-------|----------| +| 1 | **Alignment broken** — `shape.width` / `shape.height` are read without calling `shape.load()`. Office.js only returns loaded properties, so values are always `undefined` or `0`, causing incorrect positioning. Compare `MatchSizes.tsx:67` which correctly calls `sourceShape.load(...)` before reading dimensions. | `src/taskpane/components/AlignmentButtons.tsx:127-130, 176-179` | +| 2 | **Performance disaster** — `findTitleShape()` in `InsertTitles.tsx` does up to 3 sequential `await context.sync()` calls per shape per slide (load shape → sync, load textFrame → sync, load textRange → sync). A 50-slide, 10-shape presentation = up to 1,500 IPC round-trips. Should batch all loads before syncing. | `src/taskpane/components/InsertTitles.tsx:108, 116-128` | +| 3 | **Dead / broken code** — `commands.ts` imports Outlook-only API (`Office.context.mailbox`) which is `undefined` in PowerPoint. Also binds a function to `Office.actions.associate` that is never referenced in `manifest.xml`. The entire file is boilerplate from an Outlook add-in template. | `src/commands/commands.ts:25, 35` | + +--- + +## Major Improvements + +| # | Issue | Location | +|---|-------|----------| +| 4 | **Massive code duplication** — `ConfidentialButtons.tsx` and `DraftButtons.tsx` share ~200 identical lines: same `ProcessingResult` interface (duplicated), same `handleError()` pattern, same `processAdd*()` / `processRemove*()` loop structure, same CSS grid button styles. Also partially duplicated in `ProgressBarButtons.tsx`. Extract into a shared "BatchSlideOperation" component or hook. | `src/taskpane/components/ConfidentialButtons.tsx`, `DraftButtons.tsx`, `ProgressBarButtons.tsx` | +| 5 | **No business-logic layer** — All PowerPoint API calls are embedded directly in React components. No separation between UI and data access. Each component independently calls `PowerPoint.run()`, loads shapes, and validates selections. Should have a service / API layer. | All components | +| 6 | **`StatusContext` tightly coupled** — Defined in `App.tsx` instead of its own file. Every component that needs status imports from `App.tsx`, creating coupling and circular import risk. Move to a dedicated context file. | `src/taskpane/components/App.tsx` + all consuming components | +| 7 | **Inconsistent error handling** — `ActionButton.tsx` wraps operations in `try/catch/finally` with proper `isProcessing` state management. Other components use raw `