Add Archive and Download as GIF to watch page menu - #2496
Conversation
This comment has been minimized.
This comment has been minimized.
builder-io-integration
Bot
changed the title
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
How important do you think gifs are? Any clip longer than a few seconds would generate a massive gif that would be best for people to not use |
|
Gifs aren't critical. I played with the idea of a gif replacing the static thumbnail in the recipient share invite, but we went with static + play button. |
![builder-io-integration[bot]](https://avatars.githubusercontent.com/in/353721?s=60&v=4){kind=small}
There was a problem hiding this comment.
Builder reviewed your changes and found 3 potential issues 🟡
Review Details
Code Review Summary
PR #2496 adds Archive/Unarchive and Download as GIF to the Clips watch-page options menu. The menu reuses the existing action surface and editor-level access checks, and the player action now exposes archivedAt so the archive state can be represented correctly. The localized labels and disabled/pending states are also wired consistently. This is a standard-risk UI plus client-side media-processing change.
Key Findings
- 🟡 MEDIUM — Full-length GIF exports are unbounded and can exhaust browser/ffmpeg.wasm memory on ordinary long recordings.
- 🟡 MEDIUM — A failed GIF export can leave the fetched input and partial output in the shared ffmpeg filesystem, affecting later exports in the same tab.
- 🟡 MEDIUM — Unarchive on a recording that is both archived and trashed invokes
restore-recording, which clears both lifecycle flags and silently resurrects it from Trash.
The archive mutation wiring and permission boundary otherwise align with the existing action contract. Browser testing was attempted with a healthy Clips dev server, but all executor sessions lacked the required Chrome browser tools, so no visual test case produced evidence.
🧪 Browser testing: Could not verify — 16 planned cases escalated because browser automation tools were unavailable in executor sessions.
| 0, | ||
| recording.durationMs, |
There was a problem hiding this comment.
🟡 Bound full-length GIF exports before invoking ffmpeg
downloadGif passes the recording's entire durationMs to browser-side ffmpeg.wasm at 480px/12fps. Long recordings can exhaust tab memory or produce an impractically large GIF; add a bounded duration/size policy or require an explicit safe-range confirmation before starting the export.
Additional Info
Found by 2 of 4 code-review agents; browser planner also targeted this risk but could not execute due unavailable browser tools.
| t("recordingPage.buildingGif", { percent: 0 }), | ||
| ); | ||
| try { | ||
| const blob = await exportGif( |
There was a problem hiding this comment.
🟡 Clean up ffmpeg files when GIF encoding fails
The new GIF path calls exportGif, whose input/output files are deleted only after a successful exec and readFile. If encoding fails, the helper's finally removes only the progress listener, leaving the fetched source and any partial output in the singleton ffmpeg filesystem for the tab. Move per-export file deletion into a best-effort finally so failed exports release their media too.
Additional Info
Found by 1 of 4 code-review agents; confirmed by inspection of exportGif's failure path.
| const handleArchive = useCallback(() => { | ||
| if (archivePending) return; | ||
| setMenuOpen(false); | ||
| if (isArchived) restoreRecording.mutate({ id: recordingId }); |
There was a problem hiding this comment.
🟡 Do not unarchive trashed recordings through restore-recording
This selects restore-recording solely from isArchived. The player data exposes archivedAt but not trashedAt, while restore-recording clears both archivedAt and trashedAt; opening an archived-and-trashed clip by URL and choosing Unarchive therefore silently restores it from Trash as well. Gate lifecycle controls for trashed clips or use an archive-only restore action that preserves trashedAt.
Additional Info
Found by 1 of 4 code-review agents; browser planner independently confirmed the action clears both flags, though visual execution was unavailable.
3 participants
Summary
Adds "Archive" and "Download as GIF" options to the three-dot menu on the Clips watch/view page, bringing it in line with the options already available in the library.
Problem
The watch page's clip options menu only offered "Download recording" and "Delete". Archive was already supported from the library view but missing here, and there was no way to export a clip as a GIF at all.
Solution
RecordingOptionsMenuon the watch page to support archiving and to trigger a new GIF export flow using the existing ffmpeg-based export pipeline, reusingcanEdit/canDeletepermission logic for gating visibility.exportGifto accept configurable width/fps options so the watch page can request a larger, smoother download than the existing library thumbnail use case.Key Changes
app/routes/r.$recordingId.tsx: addedcanDownloadGif/canArchiveRecordingderived flags, adownloadGifcallback that callsexportGifwith progress toasts and triggers a file download, and passes newcanDownloadGif,gifPending,onDownloadGif,canArchive,isArchived, andonArchiveChangedprops intoRecordingOptionsMenu.app/lib/ffmpeg-export.ts: added aGifOptionsinterface (width,fps) toexportGif, defaulting to the previous 320px/15fps values, with the watch page using 480px/12fps for shareable full-length GIFs.recordingPage.buildingGif,recordingPage.gifExportFailed, anddeleteRecordingMenu.downloadAsGif,buildingGif,archive,unarchive,archived,restoredFromArchive,archiveFailed,unarchiveFailed) across locale catalogs (pt-BR, zh-CN, zh-TW, etc.).docs/plans/2026-07-29-clips-display-name-prompt.md) covering related future display-name prompt work.To clone this PR locally use the Github CLI with command
gh pr checkout 2496You can tag me at @BuilderIO for anything you want me to fix or change