feat: add named canvas presets including YouTube Shorts 9:16#805
feat: add named canvas presets including YouTube Shorts 9:16#805kseungyong wants to merge 2 commits into
Conversation
Adds optional projectType discriminator on TProjectMetadata so downstream features (preset pickers, export defaults, UI affordances) can branch on intent without inferring from canvas dimensions. - New const PROJECT_TYPES and type TProjectType - TProjectMetadata.projectType (optional for back-compat with stored projects) - ProjectManager.createNewProject accepts optional projectType, defaults to 'standard' - Unit tests for PROJECT_TYPES invariants
|
@kseungyong is attempting to deploy a commit to the OpenCut OSS Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis PR introduces a project-type discriminator ("standard" and "shorts") into project metadata and canvas presets. The system defines project types as a constant enum, enriches canvas presets with labels and aspect-ratio metadata, derives default presets for backward compatibility, and wires project-type acceptance through project creation. ChangesProject Type and Canvas Preset Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Introduces NAMED_CANVAS_PRESETS with id/label/aspectRatio/projectType metadata so the UI can show meaningful preset names (e.g. "YouTube Shorts (1080×1920)") and so downstream code can resolve a preset by intent rather than dimensions. - NAMED_CANVAS_PRESETS: landscape 16:9, YouTube Shorts 9:16, square, landscape 4:3 - SHORTS_CANVAS_PRESET convenience export for 9:16 vertical - DEFAULT_CANVAS_PRESETS preserved (derived from NAMED) so existing editor-store consumers keep working - Unit tests verify shape, ordering, and back-compat derivation
kseungyong
force-pushed
the
feat/shorts-canvas-preset
branch
from
49b10a9 to
4254dec
Compare
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/core/managers/project-manager.ts (1)
83-105:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInitialize canvas size from
projectTypeto keep metadata and project geometry consistent.Line 98 sets shorts intent, but Line 104 always initializes to the landscape default. A
"shorts"project should start with the 9:16 preset.Proposed fix
-import { DEFAULT_CANVAS_SIZE } from "`@/canvas/sizes`"; +import { DEFAULT_CANVAS_SIZE, SHORTS_CANVAS_PRESET } from "`@/canvas/sizes`"; @@ }): Promise<string> { + const initialCanvasSize = + projectType === "shorts" + ? { ...SHORTS_CANVAS_PRESET.size } + : { ...DEFAULT_CANVAS_SIZE }; + const mainScene = buildDefaultScene({ name: "Main scene", isMain: true }); const newProject: TProject = { @@ settings: { fps: DEFAULT_FPS, - canvasSize: DEFAULT_CANVAS_SIZE, + canvasSize: initialCanvasSize,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/core/managers/project-manager.ts` around lines 83 - 105, createNewProject currently always sets settings.canvasSize to the landscape DEFAULT_CANVAS_SIZE regardless of metadata.projectType; update createNewProject so when metadata.projectType === "shorts" (or when the projectType param equals "shorts") you initialize settings.canvasSize to the 9:16 shorts preset instead of DEFAULT_CANVAS_SIZE. Locate createNewProject (and the settings.canvasSize assignment) and branch on projectType/metadata.projectType to pick the appropriate preset (e.g., DEFAULT_CANVAS_SIZE_SHORTS or a 9:16 preset) so metadata.projectType and project geometry stay consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/canvas/sizes.ts`:
- Around line 51-53: SHORTS_CANVAS_PRESET currently uses a non-null assertion
after NAMED_CANVAS_PRESETS.find(...), remove the trailing "!" and add an
explicit runtime guard: assign the find result to a local (e.g., const preset =
NAMED_CANVAS_PRESETS.find(p => p.id === "shorts-1080x1920")), check if preset is
undefined and either throw a descriptive error or return a safe default
(matching TCanvasPreset shape), then export SHORTS_CANVAS_PRESET as that
validated value; this ensures missing configuration is detected at runtime with
a clear message instead of using the non-null assertion.
- Around line 59-63: DEFAULT_CANVAS_PRESETS and DEFAULT_CANVAS_SIZE currently
re-use the same mutable size objects from NAMED_CANVAS_PRESETS, so in-place
mutations will affect the source; update the exports to return clones instead of
references by mapping NAMED_CANVAS_PRESETS to new objects (e.g., shallow clone
each p.size when creating DEFAULT_CANVAS_PRESETS) and assign DEFAULT_CANVAS_SIZE
to a cloned copy of NAMED_CANVAS_PRESETS[0].size (preserving the TCanvasSize
shape) so consumers get independent objects.
---
Outside diff comments:
In `@apps/web/src/core/managers/project-manager.ts`:
- Around line 83-105: createNewProject currently always sets settings.canvasSize
to the landscape DEFAULT_CANVAS_SIZE regardless of metadata.projectType; update
createNewProject so when metadata.projectType === "shorts" (or when the
projectType param equals "shorts") you initialize settings.canvasSize to the
9:16 shorts preset instead of DEFAULT_CANVAS_SIZE. Locate createNewProject (and
the settings.canvasSize assignment) and branch on
projectType/metadata.projectType to pick the appropriate preset (e.g.,
DEFAULT_CANVAS_SIZE_SHORTS or a 9:16 preset) so metadata.projectType and project
geometry stay consistent.
🪄 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: fd0aab5c-7ce4-49b1-857e-9d42e883b362
📒 Files selected for processing (5)
apps/web/src/canvas/__tests__/sizes.test.tsapps/web/src/canvas/sizes.tsapps/web/src/core/managers/project-manager.tsapps/web/src/project/__tests__/project-type.test.tsapps/web/src/project/types.ts
| export const SHORTS_CANVAS_PRESET: TCanvasPreset = NAMED_CANVAS_PRESETS.find( | ||
| (p) => p.id === "shorts-1080x1920", | ||
| )!; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd apps/web && find . -name "sizes.ts" -type fRepository: OpenCut-app/OpenCut
Length of output: 85
🏁 Script executed:
cat -n apps/web/src/canvas/sizes.ts | sed -n '45,65p'Repository: OpenCut-app/OpenCut
Length of output: 714
Remove the non-null assertion and add an explicit runtime guard.
Line 53 uses the ! operator; this violates the guideline against non-null assertions in TypeScript files and can hide configuration errors until runtime.
Proposed fix
-export const SHORTS_CANVAS_PRESET: TCanvasPreset = NAMED_CANVAS_PRESETS.find(
- (p) => p.id === "shorts-1080x1920",
-)!;
+const shortsPreset = NAMED_CANVAS_PRESETS.find(
+ (p) => p.id === "shorts-1080x1920",
+);
+
+if (!shortsPreset) {
+ throw new Error('Missing required canvas preset: "shorts-1080x1920"');
+}
+
+export const SHORTS_CANVAS_PRESET: TCanvasPreset = shortsPreset;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const SHORTS_CANVAS_PRESET: TCanvasPreset = NAMED_CANVAS_PRESETS.find( | |
| (p) => p.id === "shorts-1080x1920", | |
| )!; | |
| const shortsPreset = NAMED_CANVAS_PRESETS.find( | |
| (p) => p.id === "shorts-1080x1920", | |
| ); | |
| if (!shortsPreset) { | |
| throw new Error('Missing required canvas preset: "shorts-1080x1920"'); | |
| } | |
| export const SHORTS_CANVAS_PRESET: TCanvasPreset = shortsPreset; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/canvas/sizes.ts` around lines 51 - 53, SHORTS_CANVAS_PRESET
currently uses a non-null assertion after NAMED_CANVAS_PRESETS.find(...), remove
the trailing "!" and add an explicit runtime guard: assign the find result to a
local (e.g., const preset = NAMED_CANVAS_PRESETS.find(p => p.id ===
"shorts-1080x1920")), check if preset is undefined and either throw a
descriptive error or return a safe default (matching TCanvasPreset shape), then
export SHORTS_CANVAS_PRESET as that validated value; this ensures missing
configuration is detected at runtime with a clear message instead of using the
non-null assertion.
| export const DEFAULT_CANVAS_PRESETS: TCanvasSize[] = NAMED_CANVAS_PRESETS.map( | ||
| (p) => p.size, | ||
| ); | ||
|
|
||
| export const DEFAULT_CANVAS_SIZE: TCanvasSize = NAMED_CANVAS_PRESETS[0].size; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /tmp && find . -type f -name "sizes.ts" | grep -E "(canvas|web)" | head -20Repository: OpenCut-app/OpenCut
Length of output: 45
🏁 Script executed:
cat -n apps/web/src/canvas/sizes.tsRepository: OpenCut-app/OpenCut
Length of output: 2179
🏁 Script executed:
rg "DEFAULT_CANVAS_PRESETS|DEFAULT_CANVAS_SIZE" apps/web/src --type ts -A 3 -B 1Repository: OpenCut-app/OpenCut
Length of output: 7240
🏁 Script executed:
rg "NAMED_CANVAS_PRESETS" apps/web/src --type ts -A 5 | head -50Repository: OpenCut-app/OpenCut
Length of output: 3259
🏁 Script executed:
rg "canvasPresets\s*=" apps/web/src --type ts -B 2 -A 2Repository: OpenCut-app/OpenCut
Length of output: 45
🏁 Script executed:
rg "DEFAULT_CANVAS_PRESETS\[" apps/web/src --type ts -B 2 -A 2Repository: OpenCut-app/OpenCut
Length of output: 45
🏁 Script executed:
rg "canvasSize\s*=" apps/web/src --type ts -B 2 -A 2 | head -50Repository: OpenCut-app/OpenCut
Length of output: 3179
🏁 Script executed:
cat -n apps/web/src/editor/editor-store.ts | head -50Repository: OpenCut-app/OpenCut
Length of output: 1057
🏁 Script executed:
rg "TCanvasSize" apps/web/src/project/types.ts -A 5Repository: OpenCut-app/OpenCut
Length of output: 407
Avoid sharing mutable size object references across exported constants.
Lines 59-63 currently re-export the same size objects from NAMED_CANVAS_PRESETS. Although NAMED_CANVAS_PRESETS is declared readonly, the constraint only prevents array reassignment—the nested size objects remain mutable. Any in-place mutation of properties in DEFAULT_CANVAS_PRESETS or DEFAULT_CANVAS_SIZE will mutate the source preset objects.
Proposed fix
export const DEFAULT_CANVAS_PRESETS: TCanvasSize[] = NAMED_CANVAS_PRESETS.map(
- (p) => p.size,
+ (p) => ({ ...p.size }),
);
-export const DEFAULT_CANVAS_SIZE: TCanvasSize = NAMED_CANVAS_PRESETS[0].size;
+export const DEFAULT_CANVAS_SIZE: TCanvasSize = {
+ ...NAMED_CANVAS_PRESETS[0].size,
+};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const DEFAULT_CANVAS_PRESETS: TCanvasSize[] = NAMED_CANVAS_PRESETS.map( | |
| (p) => p.size, | |
| ); | |
| export const DEFAULT_CANVAS_SIZE: TCanvasSize = NAMED_CANVAS_PRESETS[0].size; | |
| export const DEFAULT_CANVAS_PRESETS: TCanvasSize[] = NAMED_CANVAS_PRESETS.map( | |
| (p) => ({ ...p.size }), | |
| ); | |
| export const DEFAULT_CANVAS_SIZE: TCanvasSize = { | |
| ...NAMED_CANVAS_PRESETS[0].size, | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/canvas/sizes.ts` around lines 59 - 63, DEFAULT_CANVAS_PRESETS
and DEFAULT_CANVAS_SIZE currently re-use the same mutable size objects from
NAMED_CANVAS_PRESETS, so in-place mutations will affect the source; update the
exports to return clones instead of references by mapping NAMED_CANVAS_PRESETS
to new objects (e.g., shallow clone each p.size when creating
DEFAULT_CANVAS_PRESETS) and assign DEFAULT_CANVAS_SIZE to a cloned copy of
NAMED_CANVAS_PRESETS[0].size (preserving the TCanvasSize shape) so consumers get
independent objects.
|
Closing — continuing on a private fork. The branch |
1 participant
Summary
Currently
DEFAULT_CANVAS_PRESETSis a bareTCanvasSize[]— UIs have no name to render and no way to know an entry's intent (e.g. "this is the Shorts preset"). This PR adds a parallelNAMED_CANVAS_PRESETSwith id/label/aspectRatio/projectType metadata, including a dedicatedSHORTS_CANVAS_PRESETfor YouTube Shorts.Changes
canvas/sizes.ts— IntroduceNAMED_CANVAS_PRESETSandTCanvasPresettypecanvas/sizes.ts—DEFAULT_CANVAS_PRESETSis now derived fromNAMED_CANVAS_PRESETS.map(p => p.size)so existing consumers (editor-store.ts) are unaffectedcanvas/sizes.ts—SHORTS_CANVAS_PRESETexposed for direct referencecanvas/__tests__/sizes.test.ts— Coverage for shape, ordering, back-compatWhy Two Exports
DEFAULT_CANVAS_PRESETS(TCanvasSize[]) is preserved becauseeditor-store.ts:17consumes it directly. Migrating that call site is out of scope for this PR — UI changes to render labels are a separate concern.Depends On
#804 (the
projectTypefield PR) —TCanvasPreset.projectTypereferencesTProjectType. The base of this branch isfeat/project-type-field, so the commit from #804 is included in this PR. Once #804 merges to main, this PR can rebase onto main.Test Plan
bun test apps/web/src/canvas/__tests__/sizes.test.ts— 6 tests passbunx tsc --noEmit— no new errorseditor-store.ts:17(which readsDEFAULT_CANVAS_PRESETS) compiles unchangedSummary by CodeRabbit
New Features
Tests