feat(export): add frame count, ETA, elapsed time, and cancel confirma…#773
feat(export): add frame count, ETA, elapsed time, and cancel confirma…#773MB-Ndhlovu wants to merge 1 commit intoOpenCut-app:mainfrom
Conversation
…tion dialog Implements OpenCut-app#739 - Export progress now shows: - Frame count: 'Frame 120 / 1200' - Elapsed time: 'Elapsed 0:45' - Estimated time remaining: '~2:30 remaining' - Cancel confirmation dialog before cancelling mid-export Changes: - Extend ExportState interface with totalFrames, currentFrame, elapsedMs - ProjectManager.export() computes totalFrames from duration and fps at start - onProgress updates currentFrame and elapsedMs via Date.now() delta - ExportCancelDialog component wraps cancel action with Radix AlertDialog - formatDuration() utility for MM:SS time formatting - UI renders frame count, elapsed, and ETA when exporting
|
Someone is attempting to deploy a commit to the OpenCut OSS Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can validate your CodeRabbit configuration file in your editor.If your editor has YAML language server, you can enable auto-completion and validation by adding |
📝 WalkthroughWalkthroughThe pull request enhances the export feature with detailed progress tracking, displaying frame counts, elapsed time, and estimated time remaining in the UI, and introduces a confirmation dialog for canceling exports. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/components/editor/export-button.tsx`:
- Around line 54-58: The generic popover close handler (handlePopoverOpenChange)
is cancelling exports unconditionally via editor.project.cancelExport() and
editor.project.clearExportState(); change it to not cancel or clear when an
export is in progress—check the project's export status (e.g.,
editor.project.isExporting() or the export state object used elsewhere) and if
exporting, prevent closing the popover / bail out of the handler so the
confirmation flow can run; only call cancelExport() and clearExportState() when
no export is active.
In `@apps/web/src/core/managers/project-manager.ts`:
- Around line 214-217: activeProject.settings.fps (and options.fps) is a
FrameRate object, so before computing totalFrames convert exportFps to a numeric
value using the frameRateToFloat helper: import frameRateToFloat from
'@/lib/fps/utils', compute a numericFps = frameRateToFloat(exportFps) (or call
inline) and use numericFps when calculating totalFrames (the symbols to update
are exportFps and totalFrames in the block that references activeProject and
timeline.getTotalDuration()).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2061664b-7371-48f3-bca3-f68e93fdf8c8
📒 Files selected for processing (3)
apps/web/src/components/editor/export-button.tsxapps/web/src/core/managers/project-manager.tsapps/web/src/lib/export/index.ts
| const handlePopoverOpenChange = ({ open }: { open: boolean }) => { | ||
| if (!open) { | ||
| editor.project.cancelExport(); | ||
| editor.project.clearExportState(); | ||
| } |
There was a problem hiding this comment.
Don’t cancel exports from the generic popover close handler.
This bypasses the new confirmation flow: clicking outside the popover while exporting still cancels and clears state without asking.
🐛 Proposed fix: keep the popover open while exporting
const editor = useEditor();
const activeProject = useEditor((e) => e.project.getActiveOrNull());
+ const exportState = useEditor((e) => e.project.getExportState());
const hasProject = !!activeProject;
const handlePopoverOpenChange = ({ open }: { open: boolean }) => {
+ if (!open && exportState.isExporting) {
+ return;
+ }
if (!open) {
- editor.project.cancelExport();
editor.project.clearExportState();
}
setIsExportPopoverOpen(open);
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/editor/export-button.tsx` around lines 54 - 58, The
generic popover close handler (handlePopoverOpenChange) is cancelling exports
unconditionally via editor.project.cancelExport() and
editor.project.clearExportState(); change it to not cancel or clear when an
export is in progress—check the project's export status (e.g.,
editor.project.isExporting() or the export state object used elsewhere) and if
exporting, prevent closing the popover / bail out of the handler so the
confirmation flow can run; only call cancelExport() and clearExportState() when
no export is active.
| const activeProject = this.editor.project.getActive(); | ||
| const duration = this.editor.timeline.getTotalDuration(); | ||
| const exportFps = options.fps ?? activeProject.settings.fps; | ||
| const totalFrames = Math.round((duration / 1000) * exportFps); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect FrameRate-related helpers/usages. Expected: find a numeric conversion
# helper or the object fields needed to compute frames-per-second.
rg -n -C3 'FrameRate|fpsTo|toFps|framesPerSecond|numerator|denominator|num|den' --glob '!node_modules/**'
# Confirm the problematic arithmetic site.
rg -n -C3 '\(duration / 1000\) \* exportFps|totalFrames' apps/web/src/core/managers/project-manager.tsRepository: OpenCut-app/OpenCut
Length of output: 50379
🏁 Script executed:
cat -n apps/web/src/core/managers/project-manager.ts | sed -n '200,230p'Repository: OpenCut-app/OpenCut
Length of output: 1155
🏁 Script executed:
rg -n "interface ExportOptions|type ExportOptions|export.*ExportOptions" apps/web/src --max-count=5Repository: OpenCut-app/OpenCut
Length of output: 261
🏁 Script executed:
rg -n "settings.*fps|fps.*:" apps/web/src --max-count=10 -A 2Repository: OpenCut-app/OpenCut
Length of output: 27415
🏁 Script executed:
rg -n "export.*frameRateToFloat|function frameRateToFloat" apps/web/src --max-count=3 -B 1 -A 5Repository: OpenCut-app/OpenCut
Length of output: 543
Convert FrameRate to a numeric value before computing totalFrames.
Line 217 multiplies by exportFps, which is a FrameRate object, not a number. Use the standard frameRateToFloat() helper:
Fix
- const totalFrames = Math.round((duration / 1000) * exportFps);
+ const totalFrames = Math.round((duration / 1000) * frameRateToFloat(exportFps));Import frameRateToFloat from @/lib/fps/utils.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/core/managers/project-manager.ts` around lines 214 - 217,
activeProject.settings.fps (and options.fps) is a FrameRate object, so before
computing totalFrames convert exportFps to a numeric value using the
frameRateToFloat helper: import frameRateToFloat from '@/lib/fps/utils', compute
a numericFps = frameRateToFloat(exportFps) (or call inline) and use numericFps
when calculating totalFrames (the symbols to update are exportFps and
totalFrames in the block that references activeProject and
timeline.getTotalDuration()).
1 participant
…tion dialog
Implements #739 - Export progress now shows:
Changes:
We are not currently accepting PRs except for critical bugs.
If this is a bug fix:
If this is a feature:
This PR will be closed. Please open an issue to discuss first.
Summary by CodeRabbit