Skip to content

refactor(editor): extract MP4 export settings#565

Merged
meiiie merged 1 commit into
mainfrom
refactor/editor-mp4-export-settings
May 22, 2026
Merged

refactor(editor): extract MP4 export settings#565
meiiie merged 1 commit into
mainfrom
refactor/editor-mp4-export-settings

Conversation

@meiiie
Copy link
Copy Markdown
Collaborator

@meiiie meiiie commented May 22, 2026

Description

Extract MP4 export setting precedence from VideoEditor.tsx into a pure resolveMp4ExportSettings helper.

Motivation

The editor component should not inline the precedence rules for smoke export URL settings, one-off export menu settings, and persisted editor defaults. Naming this decision boundary makes the MP4 export orchestration easier to test and keeps VideoEditor.tsx moving toward smaller responsibilities.

Type of Change

  • Refactor
  • Tests

Related Issue(s)

Changes Made

  • Added src/components/video-editor/mp4ExportSettings.ts with a pure resolver for MP4 quality, encoding mode, and frame rate.
  • Added tests covering editor defaults, normal export settings, smoke export overrides, and partial smoke fallback behavior.
  • Replaced the inline MP4 settings selection block in VideoEditor.tsx with the resolver.

Scope Note

  • No UI changes.
  • No exporter implementation changes.
  • No route/backend/native helper/preload/IPC changes.
  • No Linux NVIDIA behavior changes.
  • This only names and tests an existing precedence rule.

Testing Guide

  1. Export MP4 with existing editor defaults and confirm quality/encoding/FPS behavior is unchanged.
  2. Export MP4 after changing the export menu quality, encoding mode, and FPS and confirm those one-off settings still apply.
  3. For smoke export URLs, confirm smokeQuality, smokeEncodingMode, and smokeFps still override menu/default settings.

Checklist

  • One small architectural slice only
  • Behavior-preserving extraction
  • Focused tests for moved precedence rules
  • No generated binaries or local probes included
  • Ready for review

Local Checks

  • npm test -- src/components/video-editor/mp4ExportSettings.test.ts
  • npm test -- src/components/video-editor/mp4ExportRouting.test.ts src/components/video-editor/useNvidiaCudaExportOptIn.test.ts
  • npm test -- src/lib/exporter/modernVideoExporter.nativeStaticLayout.test.ts electron/ipc/export/native-video.test.ts
  • npx tsc --noEmit --pretty false
  • npx biome check --formatter-enabled=false src/components/video-editor/VideoEditor.tsx src/components/video-editor/mp4ExportSettings.ts src/components/video-editor/mp4ExportSettings.test.ts
  • git diff --check

Runtime/Repro Evidence

Not run. This is a pure renderer-side settings precedence extraction with no native/preload/IPC contract change.

Summary by CodeRabbit

  • Refactor

    • Improved internal code organization for MP4 export settings resolution to enhance maintainability.
  • Tests

    • Added comprehensive test coverage for MP4 export settings resolution across multiple scenarios.

Note: This release contains internal code improvements with no user-facing changes.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 22, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: df132f4e-d51a-4f7b-a55f-e7443ec08440

📥 Commits

Reviewing files that changed from the base of the PR and between 1db58dc and 9f05f58.

📒 Files selected for processing (3)
  • src/components/video-editor/VideoEditor.tsx
  • src/components/video-editor/mp4ExportSettings.test.ts
  • src/components/video-editor/mp4ExportSettings.ts

📝 Walkthrough

Walkthrough

This PR extracts MP4 export settings resolution into a reusable helper function. A new resolveMp4ExportSettings() module computes quality, encodingMode, and selectedMp4FrameRate with conditional precedence based on smoke export state. The VideoEditor MP4 export logic now calls this helper instead of computing settings inline.

Changes

MP4 Export Settings Consolidation

Layer / File(s) Summary
MP4 Export Settings Resolver
src/components/video-editor/mp4ExportSettings.ts
New module exports ResolvedMp4ExportSettings type and resolveMp4ExportSettings() function. The resolver applies conditional logic based on smokeExportConfig.enabled to select quality, encodingMode, and selectedMp4FrameRate with fallback priority: smoke config fields when enabled, otherwise menu settings fields, otherwise explicit function parameters.
MP4 Export Settings Tests
src/components/video-editor/mp4ExportSettings.test.ts
Vitest test suite validates the resolver across four scenarios: default editor values when no overrides present, menu settings precedence for normal exports, smoke settings complete precedence when smoke export enabled, and fallback behavior when smoke settings are incomplete (notably missing selectedMp4FrameRate).
VideoEditor MP4 Export Refactoring
src/components/video-editor/VideoEditor.tsx
Refactors the MP4 export branch of handleExport to call resolveMp4ExportSettings() instead of computing quality, encodingMode, and selectedMp4FrameRate inline. Imports the resolver and passes current smoke, menu, and editor settings to derive the final export configuration.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • webadderallorg/Recordly#543: Both PRs coordinate around VideoEditor.tsx's smokeExportConfig: #543 refactors how it's parsed/provided, and the main PR then consumes that same config to resolve MP4 quality, encodingMode, and selectedMp4FrameRate via resolveMp4ExportSettings().

Suggested labels

Checked

Poem

🐰 A helper emerges from the MP4 fray,
With smoke and settings in noble display,
Fallbacks cascade in priority's way,
Tests verify each branching play,
The export code's simpler today! 🎬

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'refactor(editor): extract MP4 export settings' is concise, specific, and directly reflects the main change: extracting MP4 export settings logic into a separate module.
Description check ✅ Passed The PR description comprehensively covers all major sections from the template: description, motivation, type of change, related issues, changes made, testing guide, and checklist. The description goes beyond template requirements with useful scope notes and local checks.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/editor-mp4-export-settings

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@meiiie meiiie merged commit 6cc0712 into main May 22, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant