fix: add time and size estimation below export buttons (#224) - #2050
fix: add time and size estimation below export buttons (#224)#2050BMNTR wants to merge 3 commits into
Conversation
| <Show when={metadata.latest}> | ||
| {(metadata) => ( | ||
| <span class="text-[10px] text-gray-200 font-medium bg-black/50 px-2 py-0.5 rounded backdrop-blur-sm"> | ||
| Est. {metadata().size.toFixed(1)} MB • ~{Math.ceil(metadata().estimatedExportTime)}s |
There was a problem hiding this comment.
Format long estimates readably
The badge renders every estimatedExportTime value as raw seconds, so long recordings display values such as ~10800s instead of the more readable minutes-and-seconds format used by the main export view.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/routes/recordings-overlay.tsx
Line: 388
Comment:
**Format long estimates readably**
The badge renders every `estimatedExportTime` value as raw seconds, so long recordings display values such as `~10800s` instead of the more readable minutes-and-seconds format used by the main export view.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| : `~${Math.floor(sec / 60)}m ${sec % 60}s`; | ||
| return ( | ||
| <p class="text-xs text-gray-10 text-center mt-0.5"> | ||
| Est. size: {estimate.estimatedSizeMb.toFixed(1)} MB • Est. time: {timeStr} |
There was a problem hiding this comment.
Minor safety: toFixed() will throw if estimatedSizeMb is ever undefined/non-number (e.g. IPC hiccup). Guarding keeps the UI stable.
| Est. size: {estimate.estimatedSizeMb.toFixed(1)} MB • Est. time: {timeStr} | |
| Est. size: {Number.isFinite(estimate.estimatedSizeMb) ? estimate.estimatedSizeMb.toFixed(1) : "?"} MB • Est. time: {timeStr} |
| : `~${Math.floor(sec / 60)}m ${sec % 60}s`; | ||
| return ( | ||
| <span class="text-[10px] text-gray-200 font-medium bg-black/50 px-2 py-0.5 rounded backdrop-blur-sm"> | ||
| Est. {meta.size.toFixed(1)} MB • {timeStr} |
There was a problem hiding this comment.
Same here: toFixed() can throw if size comes back missing/invalid. Guard avoids a runtime crash.
| Est. {meta.size.toFixed(1)} MB • {timeStr} | |
| Est. {Number.isFinite(meta.size) ? meta.size.toFixed(1) : "?"} MB • {timeStr} |
| <Show when={metadata.latest}> | ||
| {(metadata) => { | ||
| const meta = metadata(); | ||
| const sec = Math.max(1, Math.ceil(meta.estimatedExportTime)); |
There was a problem hiding this comment.
Minor safety guard: if meta.estimatedExportTime ever comes back NaN/Infinity, this ends up rendering ~NaNm NaNs. Might be worth mirroring the Number.isFinite guard used in ExportPage.
| const sec = Math.max(1, Math.ceil(meta.estimatedExportTime)); | |
| const sec = Number.isFinite(meta.estimatedExportTime) | |
| ? Math.max(1, Math.ceil(meta.estimatedExportTime)) | |
| : 1; |
2c4e6b5 to
1496e80
Compare
Adds estimated export time and file size indicators directly under the export buttons to resolve #224.
Changes
ExportPage.tsx.recordings-overlay.tsx.Tested locally with short and long recording lengths.
/claim #224
Greptile Summary
This PR adds estimated export size and duration text to the editor export action and recording-thumbnail hover cards.
Confidence Score: 4/5
The PR appears safe to merge, with a non-blocking readability issue for long duration estimates in the recording overlay.
The estimate units and guarded data access are consistent with their producers, but the overlay’s seconds-only formatting becomes difficult to interpret for long recordings.
Files Needing Attention: apps/desktop/src/routes/recordings-overlay.tsx
Important Files Changed
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix: add time and size estimation below ..." | Re-trigger Greptile