feat(frontend): add script image workflow and no-capture toggle UI - #4
Builder106 wants to merge 3 commits into
Conversation
ArunNGun
left a comment
There was a problem hiding this comment.
Code Review
✅ What's good
Image serialization approach (parseStoredScript / serializeScriptText) is clever — embedding base64 image data as a tagged footer in the script text avoids breaking the existing storage format. The atob/btoa with JSON is clean and has a try/catch fallback.
insertAtCursor is well-written — correctly handles selection ranges and updates stats/preview after insert.
Rust side (lib.rs) is clean and minimal. set_content_protected for the no-capture toggle is the right Tauri API for this. Error handling is consistent.
CSS is tidy. The image-only fullscreen centering with flexbox is a nice touch for image-only scripts.
⚠️ Things to flag
1. buildScript receives scriptInput.value — worth a comment
scriptInput.value only contains human-readable text (images are in state.currentScriptImages), so the [[__images__:...]] footer never appears here. This is correct but subtle — a short comment explaining the separation would help future readers.
2. Image IDs — potential collision under rapid paste
const imageId = `img_${Date.now()}_${Math.random().toString(36).slice(2, 8)}`Only 6 chars of randomness. Low risk, but pasting multiple images in quick succession could theoretically collide. Consider crypto.randomUUID() — available in modern browsers and Tauri.
3. No image size limit on paste/drop
Large images get stored as full base64 data URLs in localStorage. A single high-res screenshot could be 2–5MB+ encoded, and localStorage is typically capped at 5–10MB total. A silent failure here could corrupt stored scripts. Worth adding a file size check with a user-visible warning.
4. Unnecessary setHideFromCapture call on startup when value is false
const initialHideCapture = localStorage.getItem(HIDE_CAPTURE_KEY) === '1'
applyHideFromCapture(initialHideCapture)When initialHideCapture is false (the default), this still invokes API.setHideFromCapture(false) on every startup. Minor, but could be guarded with if (initialHideCapture).
5. package-lock.json added without visible package.json changes
The lock file is 232 lines but no package.json diff is shown. Was the lock file missing before? Worth clarifying — if @tauri-apps/cli was already a dep, the lock should have existed.
6. .gitignore un-ignore for src-tauri/src/
The !src-tauri/src/ lines look like they're fixing an accidental exclusion of Rust source files. Fine, but a brief PR note on why would help.
Summary
| Blocking issues | None |
| Recommended | crypto.randomUUID(), image size limit before storing |
| Nice to have | Comment on serialization format, startup guard for hide-capture |
Overall solid work — the image workflow is well thought out end-to-end. Happy to approve once the size limit concern is addressed or acknowledged.
Code reviewFound 3 issues:
The PR's 52-line openTeleprompt/src-tauri/src/lib.rs Lines 1 to 52 in 5cfd928
The PR's version resets openTeleprompt/src-tauri/tauri.conf.json Lines 1 to 28 in 5cfd928
openTeleprompt/frontend/renderer/app.js Lines 296 to 305 in 5cfd928 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
Blocker: PR targets a frontend tree that's no longer active on
|
|
Closing as superseded by the v3.0.0 rewrite. The overlay/no-capture pieces here duplicate what's already in |
Summary
Adds script image support and no-capture UI controls in the renderer.
Changes
Note
Intended as PR 3 of 3 in split sequence.