78 lines
8.5 KiB
Markdown
78 lines
8.5 KiB
Markdown
# 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 `<Button>` elements with ad-hoc error handling. `ProgressBarButtons.tsx:162, 239` accesses `error.message` without type-check (crashes if a non-Error is thrown). `ConfidentialButtons.tsx:206-218` silently swallows font loading errors, producing invisible or wrongly colored markings. | Multiple files |
|
|
| 8 | **No testing infrastructure** — Zero test files, no test runner (Jest / Vitest), no testing libraries in `devDependencies`, no mock utilities for the Office.js API. Regression risk grows with every feature. | Entire project |
|
|
|
|
---
|
|
|
|
## Medium Issues
|
|
|
|
| # | Issue | Location |
|
|
|---|-------|----------|
|
|
| 9 | **Magic numbers everywhere** — Slide dimensions (`960` / `540`) duplicated in 4+ files. Alignment offsets (`81.1`, `480`, `879.75`, `136.75`, `321`, `505.25`) are unexplained. Extract to a shared constants file. | `ConfidentialButtons.tsx:35-37`, `GridGuidelineManager.tsx:33-34`, `ProgressBarButtons.tsx:23-24`, `InsertTitles.tsx:17`, `AlignmentButtons.tsx:30-38` |
|
|
| 10 | **No confirmation for destructive operations** — Removing all confidential markings or draft watermarks from every slide / master has no "Are you sure?" prompt. | `ConfidentialButtons.tsx`, `DraftButtons.tsx` |
|
|
| 11 | **`gridOpacity` unbounded** — `useNumericInputHandler` has no upper-bound validation. Values > 100 produce negative transparency in `line.fill.transparency = (100 - gridOpacity) / 100`, which is invalid. Should clamp to `[0, 1]`. | `src/taskpane/components/GridGuidelineManager.tsx:280, 301` |
|
|
| 12 | **No progress feedback for batch operations** — `InsertTitles` processes all slides, `ConfidentialButtons` processes every slide — only a spinner is shown, with no progress bar or slide counter. Users have no indication of how long the operation will take. | `InsertTitles.tsx`, `ConfidentialButtons.tsx`, `DraftButtons.tsx` |
|
|
| 13 | **Misleading status type** — `RoundImage.tsx` sets `setStatusType("warning")` after a *successful* mask creation. The mask was created, but the user needs to perform a manual step. Use `"info"` or a distinct instructional state instead. | `src/taskpane/components/RoundImage.tsx:124` |
|
|
| 14 | **Manifest uses template GUID** — Add-in ID `be5c6f61-4bbf-4bd2-a8da-5031abf096a5` is from the Yeoman Office template. Should be regenerated for production to avoid collisions. | `manifest.xml:3` |
|
|
| 15 | **No Content Security Policy** — Neither `taskpane.html` nor `commands.html` defines a `<meta>` CSP tag. | `src/taskpane/taskpane.html`, `src/commands/commands.html` |
|
|
| 16 | **Placeholder support URL** — `SupportUrl` in `manifest.xml:11` points to `https://www.contoso.com/help`, a Microsoft template placeholder. | `manifest.xml:11` |
|
|
|
|
---
|
|
|
|
## Quick Wins (Low Effort, High Value)
|
|
|
|
| # | Issue | Location |
|
|
|---|-------|----------|
|
|
| 17 | **Fix `any` types** — Replace `error: any` with `error: unknown` in `catch` blocks to enforce proper type narrowing (3 locations). | `ConfidentialButtons.tsx:96`, `DraftButtons.tsx:104`, `AlignmentButtons.tsx:92` |
|
|
| 18 | **Remove unused imports** — `ArrowMaximizeRegular` / `ArrowMinimizeRegular` imported but unused in `MatchSizes.tsx`. | `src/taskpane/components/MatchSizes.tsx:9` |
|
|
| 19 | **Expose width-only / height-only match** — `SizeMatchType.Width` and `SizeMatchType.Height` enums exist and the logic supports them, but only "Match Sizes" (both dimensions) has a UI button. | `src/taskpane/components/MatchSizes.tsx` |
|
|
| 20 | **Fix `package.json` repo URL** — Points to `Office-Addin-TaskPane-React` (Microsoft template repo), not the actual project repository. | `package.json:7` |
|
|
| 21 | **Bump `tsconfig.json` target** — `es5` is unnecessary since the add-in explicitly requires modern Office. Move to `ES2020+` to drop `core-js` polyfills and reduce bundle size. | `tsconfig.json:16` |
|
|
| 22 | **Remove dead config** — `ts-node` section exists in `tsconfig.json` but `ts-node` is not a dependency. Also, `ie 11` in `browserslist` but the HTML file explicitly rejects IE / Edge Legacy. | `tsconfig.json:31-36`, `package.json:73-75` |
|
|
| 23 | **Remove empty spacer `<div>` elements** — `App.tsx` uses `<div style={{ marginTop: "8px" }}></div>` as spacers. Use CSS `gap` on the parent flex container instead. | `src/taskpane/components/App.tsx:153-161` |
|
|
| 24 | **Inconsistent TypeScript compilation** — `.ts` files use `babel-loader` (no type checking) while `.tsx` files use `ts-loader` (with type checking). This can cause files to pass build but contain type errors. | `webpack.config.js:37-48` |
|
|
| 25 | **Fragile HMR pattern** — `index.tsx` casts `module` to a custom `HotModule` interface. Should use `import.meta.webpackHot` or `@types/webpack-env`. The `require("./components/App")` on line 32 circumvents ES module system. | `src/taskpane/index.tsx:30-32` |
|
|
| 26 | **Hardcoded production domain** — `urlProd` in `webpack.config.js:9` is hardcoded. If the deployment domain changes, this and `manifest.xml` could diverge. | `webpack.config.js:9` |
|
|
| 27 | **Incomplete dark mode** — Dark mode check runs once on mount with no listener for system theme changes or Office theme events. | `src/taskpane/components/App.tsx:109-113` |
|
|
| 28 | **`StandardiseSizes.tsx` loads but never uses line weight** — `RoundImage.tsx:82-83` loads `imageShape.lineFormat.weight` but never uses the value, causing an unnecessary sync. | `src/taskpane/components/RoundImage.tsx:82-83` |
|
|
|
|
---
|
|
|
|
## Security
|
|
|
|
| # | Issue | Location |
|
|
|---|-------|----------|
|
|
| 29 | **Overly permissive CORS** — `Access-Control-Allow-Origin: *` on dev server. While dev-only, restrict for best practice. | `webpack.config.js:101` |
|
|
| 30 | **Template `GetStarted.LearnMoreUrl`** — Points to `https://go.microsoft.com/fwlink/?LinkId=276812`, a generic Microsoft link. | `manifest.xml:71` |
|
|
|
|
---
|
|
|
|
## Recommended Execution Order
|
|
|
|
1. **Fix alignment bug** (#1) — functional breakage
|
|
2. **Refactor `InsertTitles` sync pattern** (#2) — unusably slow on real presentations
|
|
3. **Extract shared logic** (#4, #5, #6) — clean the foundation before adding features
|
|
4. **Fix easy bugs** (#11, #13, #17, #18, #22, #28)
|
|
5. **Add infrastructure** (#8, #15, #19, #20, #21)
|
|
6. **Add UX improvements** (#10, #12, #27)
|