Conversation
📝 WalkthroughWalkthroughThis PR introduces a comprehensive animation preset system for layer transitions. It adds type-safe preset definitions, new UI components for preset selection with preview capabilities, updates the layer rendering pipeline to apply transition presets during rendering, and extends the schema to support enter/exit transition data on layers. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as AnimationPresetSelect
participant Store as ProjectStore
participant Engine as LayerRendering
participant DOM as Preview Canvas
User->>UI: Hover over preset option
activate UI
UI->>UI: startPreview(presetId)
UI->>UI: Loop animation frame (1 second)
UI->>Engine: getLayerTransform(layer, previewTime, duration)
Engine->>Engine: applyTransitionPresets(layer, previewTime, duration)
Engine-->>UI: transform with preset applied
UI->>Engine: getLayerStyle(layer, previewTime, duration)
Engine->>Engine: applyTransitionPresets(layer, previewTime, duration)
Engine-->>UI: style with preset applied
UI->>DOM: Render preview box with transform/style
deactivate UI
User->>UI: Click preset to select
activate UI
UI->>UI: selectPreset(presetId)
UI->>UI: stopPreview()
UI->>Store: updateLayer(layer with enterTransition/exitTransition)
Store->>Store: updateLayer() saves transition presetId and duration
Store-->>UI: Layer updated
UI->>UI: Close popover
deactivate UI
User->>Engine: Render frame at time T
activate Engine
Engine->>Engine: getLayerTransform(layer, currentTime, projectDuration)
Engine->>Engine: applyTransitionPresets(layer, currentTime, projectDuration)
Engine->>Engine: Merge transition transform + animated transform + base transform
Engine-->>User: Final transform for frame
deactivate Engine
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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: 8
🧹 Nitpick comments (4)
src/lib/schemas/animation.ts (1)
233-236: Consider validating transition durations against visible layer window inLayerSchema.
LayerTransitionSchemaonly validatesduration > 0, but doesn't guard against transitions exceeding the layer's visible window. IfenterTransition.duration + exitTransition.duration > exitTime - enterTime, the two transitions will overlap, which may produce unexpected visual results. The existing.refine()block onLayerSchema(Line 286) is a natural place to add this cross-field validation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/schemas/animation.ts` around lines 233 - 236, Add a cross-field validation to the existing LayerSchema .refine() (the refine block already referenced on LayerSchema) that checks enterTransition.duration + exitTransition.duration <= (exitTime - enterTime) whenever enterTransition and exitTransition are present; if the sum exceeds the visible window (exitTime - enterTime) return a failing refinement with a clear message referencing enterTransition.duration and exitTransition.duration (or attach issues to those paths) so the validation prevents overlapping transitions and surfaces which transition durations are invalid relative to enterTime/exitTime.src/lib/components/editor/panels/properties/animation-preset-select.svelte (1)
72-87: RAF starts unnecessarily for theNoneoption.
startPreview('')is called when hovering over the "None" row.hoveredPresetresolves to the None entry (keyframes[]), so no animation occurs, yet the RAF loop runs at full speed. Guard the loop start against empty presets.♻️ Proposed fix
function startPreview(presetId: string) { hoveredPresetId = presetId; previewTime = 0; + if (!presetId) return; // Nothing to animate for "None" const startTime = performance.now(); const duration = 1000; function animate(currentTime: number) { const elapsed = currentTime - startTime; previewTime = (elapsed % duration) / duration; animationFrameId = requestAnimationFrame(animate); } animationFrameId = requestAnimationFrame(animate); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/components/editor/panels/properties/animation-preset-select.svelte` around lines 72 - 87, startPreview currently starts a requestAnimationFrame loop even for the "None" preset (empty id / no keyframes); change startPreview (and where it reads hoveredPresetId/previewTime/animationFrameId) to early-return before starting the RAF when the presetId is falsy or the resolved preset has no keyframes: still set hoveredPresetId and previewTime = 0, cancel any existing animationFrameId, but do not call requestAnimationFrame or create the animate loop for empty presets.src/lib/engine/presets.ts (2)
324-344: Consider pre-computing category-filtered arrays as constants.Since
animationPresetsis a module-level constant, these filters produce the same result on every call. Pre-computing them avoids repeated allocations and (in Svelte reactive contexts) new array references that may trigger unnecessary reactivity.♻️ Optional optimization
+const enterPresets = animationPresets.filter((p) => p.category === 'enter'); +const exitPresets = animationPresets.filter((p) => p.category === 'exit'); +const genericPresets = animationPresets.filter((p) => p.category === 'generic'); + export function getEnterPresets(): TypedAnimationPreset[] { - return animationPresets.filter((p) => p.category === 'enter'); + return enterPresets; } export function getExitPresets(): TypedAnimationPreset[] { - return animationPresets.filter((p) => p.category === 'exit'); + return exitPresets; } export function getGenericPresets(): TypedAnimationPreset[] { - return animationPresets.filter((p) => p.category === 'generic'); + return genericPresets; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/engine/presets.ts` around lines 324 - 344, The three functions getEnterPresets, getExitPresets, and getGenericPresets repeatedly filter the module-level animationPresets on every call, causing unnecessary allocations and new references; change this by computing const ENTER_PRESETS = animationPresets.filter(p => p.category === 'enter'), EXIT_PRESETS = animationPresets.filter(p => p.category === 'exit'), and GENERIC_PRESETS = animationPresets.filter(p => p.category === 'generic') once at module initialization and have getEnterPresets/getExitPresets/getGenericPresets simply return those precomputed constants to avoid repeated work and reactive churn.
39-58: Hardcoded style property names in the conditional type may drift fromLayerStyleduring future maintenance.The
valuetype manually enumerates all style properties by name (opacity,blur,brightness, etc.) rather than deriving them fromLayerStyle. While the current list is complete and correct, ifLayerStylegains or removes a numeric or string property in the future, this conditional won't auto-update and will silently resolve toneverfor new properties.Consider deriving numeric and string property subsets from
LayerStyleto keep them in sync automatically:♻️ Sketch of a derived approach
+// Derive numeric and string style properties from LayerStyle +type NumericStyleProperty = { + [K in keyof LayerStyle]: LayerStyle[K] extends number ? K : never; +}[keyof LayerStyle]; + +type StringStyleProperty = { + [K in keyof LayerStyle]: LayerStyle[K] extends string ? K : never; +}[keyof LayerStyle]; + export type PresetKeyframe<P extends BaseAnimatableProperty = BaseAnimatableProperty> = { time: number; property: P; value: P extends TransformProperty ? number - : P extends - | 'opacity' - | 'blur' - | 'brightness' - | 'contrast' - | 'saturate' - | 'dropShadowX' - | 'dropShadowY' - | 'dropShadowBlur' - ? number - : P extends 'dropShadowColor' - ? string - : never; + : P extends NumericStyleProperty + ? number + : P extends StringStyleProperty + ? string + : never; interpolation?: Interpolation; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/engine/presets.ts` around lines 39 - 58, The PresetKeyframe conditional for value currently hardcodes style property names; change it to derive numeric and string property subsets from LayerStyle so new/removed properties stay in sync: create helper type(s) like NumericStyleKeys = { [K in keyof LayerStyle]: LayerStyle[K] extends number ? K : never }[keyof LayerStyle] and StringStyleKeys similar, then replace the manual union in PresetKeyframe's value branch (the P extends 'opacity' | 'blur' | ... and P extends 'dropShadowColor' checks) with checks against those derived key sets while keeping the existing TransformProperty branch and referencing PresetKeyframe, BaseAnimatableProperty and TransformProperty to locate the code to modify.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/components/editor/panels/properties/animation-preset-select.svelte`:
- Line 24: animationFrameId is declared as a reactive $state but is only
reassigned every RAF tick and never read reactively; change the declaration to a
plain mutable local variable (let animationFrameId = null) and update any
assignments inside the RAF loop to assign that plain variable (references:
animationFrameId and the RAF loop that calls requestAnimationFrame) so you stop
registering unnecessary reactive tracking on each frame.
- Around line 104-108: The effect using $effect(() => { if (!open) {
stopPreview(); } }) lacks a cleanup, so when the component unmounts while open
the RAF loop never gets cancelled; update the $effect callback to return a
cleanup function that calls stopPreview (or directly cancels the animation
frame) so that any running preview is stopped both when the effect re-runs and
when the component is destroyed; locate the $effect block in
animation-preset-select.svelte and return a cleanup that invokes
stopPreview/cancelAnimationFrame accordingly.
- Around line 55-61: Replace the unsafe "as any" cast on kf.property in the
hoveredPreset.keyframes.map call with a proper concrete type: import the
keyframe/property type used by TypedAnimationPreset (e.g., the Keyframe type or
AnimatableProperty) and use that in the mapping (for example cast to
Keyframe['property'] or AnimatableProperty) or annotate the mapped object so
property has the correct type; update the map in the component where
hoveredPreset and keyframes are created to use the concrete type instead of any.
- Around line 1-8: The file
editor/panels/properties/animation-preset-select.svelte calls
createLayer('rectangle', ...) but never imports it, causing a build error; add
an import for createLayer from $lib/engine/layer-factory alongside the other
engine imports (group with getLayerTransform/getLayerStyle or place after them)
so createLayer is available where referenced in this Svelte component.
In `@src/lib/components/editor/panels/properties/groups/time-range-group.svelte`:
- Around line 71-99: The label "Duration (s)" in the InputsWrapper fields array
is orphaned when enterPresetId is falsy because ScrubInput is conditionally
rendered; fix by keeping label/input in sync: either render the ScrubInput
unconditionally (in the same location) and set it disabled when enterPresetId is
null/undefined (preserve id="enter-duration", value={enterDuration},
min/max/step and call updateTransition only when enabled), or move the
corresponding fields entry out of the static fields array and only include the {
for: 'enter-duration', labels: 'Duration (s)' } descriptor when enterPresetId is
truthy so the InputsWrapper’s fields match the rendered ScrubInput; update
references to enterPresetId, enterDuration, ScrubInput, InputsWrapper,
AnimationPresetSelect, and updateTransition accordingly.
In `@src/lib/engine/layer-rendering.ts`:
- Around line 140-141: Update the misleading comment above the calculation of
transitionProgress in layer-rendering.ts: replace "we interpolate from end
backwards" with a clear note that transitionProgress increases from 0 to
layer.exitTransition.duration and therefore exit presets play forward (animate
visible→hidden, e.g., opacity 1→0) so designers should author exit presets
accordingly; reference the transitionProgress variable and
layer.exitTransition.duration/timeUntilExit to locate the line.
- Around line 114-151: The shallow spreads into
transitionTransform/transitionStyle lose nested properties when enter and exit
presets overlap; update the merge logic inside the enter/exit branches (where
transitionTransform and transitionStyle are assigned after applyPresetAtTime) to
perform a deep-merge for known nested keys (at minimum position, rotation, scale
and any nested style objects) instead of simple { ...transitionTransform,
...transform } and { ...transitionStyle, ...style }; implement merging by
copying top-level keys and, for keys that are objects (e.g., position), merge
their sub-keys (x,y,z or axis fields) so enter and exit contributions are
combined, keeping existing values when the other side doesn't provide that
sub-key, and apply the same deep-merge approach for style when combining style
objects returned from applyPresetAtTime.
In `@src/lib/engine/presets.ts`:
- Around line 114-149: The preset position keys are being applied as absolute
values instead of offsets in applyPresetAtTime (layer-rendering.ts); update the
merge logic for position so preset and animated position components are added to
the layer's base transform rather than replacing it — e.g., compute finalX =
layer.transform.position.x + (transitionTransform.position?.x ?? 0) +
(animatedTransform.position?.x ?? 0) and likewise for finalY, and use those in
place of the current nullish-coalescing chain; adjust any places that read
transitionTransform.position.* or animatedTransform.position.* so they default
to 0 when missing, and ensure presets like
slide-in-left/slide-in-top/shake/float now act as offsets relative to the
layer's base position.
---
Nitpick comments:
In `@src/lib/components/editor/panels/properties/animation-preset-select.svelte`:
- Around line 72-87: startPreview currently starts a requestAnimationFrame loop
even for the "None" preset (empty id / no keyframes); change startPreview (and
where it reads hoveredPresetId/previewTime/animationFrameId) to early-return
before starting the RAF when the presetId is falsy or the resolved preset has no
keyframes: still set hoveredPresetId and previewTime = 0, cancel any existing
animationFrameId, but do not call requestAnimationFrame or create the animate
loop for empty presets.
In `@src/lib/engine/presets.ts`:
- Around line 324-344: The three functions getEnterPresets, getExitPresets, and
getGenericPresets repeatedly filter the module-level animationPresets on every
call, causing unnecessary allocations and new references; change this by
computing const ENTER_PRESETS = animationPresets.filter(p => p.category ===
'enter'), EXIT_PRESETS = animationPresets.filter(p => p.category === 'exit'),
and GENERIC_PRESETS = animationPresets.filter(p => p.category === 'generic')
once at module initialization and have
getEnterPresets/getExitPresets/getGenericPresets simply return those precomputed
constants to avoid repeated work and reactive churn.
- Around line 39-58: The PresetKeyframe conditional for value currently
hardcodes style property names; change it to derive numeric and string property
subsets from LayerStyle so new/removed properties stay in sync: create helper
type(s) like NumericStyleKeys = { [K in keyof LayerStyle]: LayerStyle[K] extends
number ? K : never }[keyof LayerStyle] and StringStyleKeys similar, then replace
the manual union in PresetKeyframe's value branch (the P extends 'opacity' |
'blur' | ... and P extends 'dropShadowColor' checks) with checks against those
derived key sets while keeping the existing TransformProperty branch and
referencing PresetKeyframe, BaseAnimatableProperty and TransformProperty to
locate the code to modify.
In `@src/lib/schemas/animation.ts`:
- Around line 233-236: Add a cross-field validation to the existing LayerSchema
.refine() (the refine block already referenced on LayerSchema) that checks
enterTransition.duration + exitTransition.duration <= (exitTime - enterTime)
whenever enterTransition and exitTransition are present; if the sum exceeds the
visible window (exitTime - enterTime) return a failing refinement with a clear
message referencing enterTransition.duration and exitTransition.duration (or
attach issues to those paths) so the validation prevents overlapping transitions
and surfaces which transition durations are invalid relative to
enterTime/exitTime.
| <script lang="ts"> | ||
| import * as Popover from '$lib/components/ui/popover'; | ||
| import { Button } from '$lib/components/ui/button'; | ||
| import { ChevronDown } from '@lucide/svelte'; | ||
| import type { TypedAnimationPreset } from '$lib/engine/presets'; | ||
| import { getLayerTransform, getLayerStyle } from '$lib/engine/layer-rendering'; | ||
| import { generateTransformCSS, generateFilterCSS } from '$lib/layers/base'; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find where createLayer is exported
rg "export.*createLayer" -t ts -t tsx -t svelte -A 2Repository: epavanello/devmotion
Length of output: 91
🏁 Script executed:
# Check if createLayer is used in the animation-preset-select.svelte file
cat -n src/lib/components/editor/panels/properties/animation-preset-select.svelte | head -100Repository: epavanello/devmotion
Length of output: 3812
🏁 Script executed:
# Also search for createLayer function definition without export keyword
rg "function createLayer|const createLayer" -t ts -t tsx -t svelte -B 2 -A 5Repository: epavanello/devmotion
Length of output: 91
🏁 Script executed:
# Search for createLayer export - using correct file types
rg "export.*createLayer" -t ts -t svelte -A 2Repository: epavanello/devmotion
Length of output: 94
🏁 Script executed:
# Also search for just the function definition
rg "function createLayer|const createLayer" -t ts -t svelte -B 2 -A 3Repository: epavanello/devmotion
Length of output: 94
🏁 Script executed:
# Check the rest of the animation-preset-select.svelte file to see all imports
cat -n src/lib/components/editor/panels/properties/animation-preset-select.svelteRepository: epavanello/devmotion
Length of output: 6243
🏁 Script executed:
# Search for createLayer without file type restrictions
rg "export.*createLayer"Repository: epavanello/devmotion
Length of output: 151
🏁 Script executed:
# Also search in typescript files to find where createLayer might be defined
fd -t f "\.ts$" -x grep -l "createLayer" {} \;Repository: epavanello/devmotion
Length of output: 123
Add missing createLayer import — build will fail without it.
createLayer('rectangle', ...) is called at line 31 but is not imported. Add the following import with the other $lib/engine imports:
import { createLayer } from '$lib/engine/layer-factory';
This import should be added after line 6 to maintain proper import order (engine imports grouped together).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/components/editor/panels/properties/animation-preset-select.svelte`
around lines 1 - 8, The file
editor/panels/properties/animation-preset-select.svelte calls
createLayer('rectangle', ...) but never imports it, causing a build error; add
an import for createLayer from $lib/engine/layer-factory alongside the other
engine imports (group with getLayerTransform/getLayerStyle or place after them)
so createLayer is available where referenced in this Svelte component.
| let open = $state(false); | ||
| let hoveredPresetId = $state<string | null>(null); | ||
| let previewTime = $state(0); | ||
| let animationFrameId = $state<number | null>(null); |
There was a problem hiding this comment.
animationFrameId should be a plain let, not $state.
animationFrameId is reassigned on every animation frame (~60 fps) inside the RAF loop but is never read in any $derived expression or template binding. Using $state here registers 60 unnecessary reactive-tracking operations per second with zero benefit.
⚡ Proposed fix
- let animationFrameId = $state<number | null>(null);
+ let animationFrameId: number | null = null;📝 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.
| let animationFrameId = $state<number | null>(null); | |
| let animationFrameId: number | null = null; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/components/editor/panels/properties/animation-preset-select.svelte`
at line 24, animationFrameId is declared as a reactive $state but is only
reassigned every RAF tick and never read reactively; change the declaration to a
plain mutable local variable (let animationFrameId = null) and update any
assignments inside the RAF loop to assign that plain variable (references:
animationFrameId and the RAF loop that calls requestAnimationFrame) so you stop
registering unnecessary reactive tracking on each frame.
| const keyframes = hoveredPreset.keyframes.map((kf) => ({ | ||
| id: crypto.randomUUID(), | ||
| time: kf.time, | ||
| property: kf.property as any, | ||
| value: kf.value, | ||
| interpolation: kf.interpolation | ||
| })); |
There was a problem hiding this comment.
as any on kf.property violates TypeScript strict-mode guideline.
The ESLint error flags this. The property field of TypedAnimationPreset keyframes likely has a concrete type (e.g., AnimatableProperty or a union string literal); cast to that type instead of any.
🛡️ Proposed fix (adjust type based on actual TypedAnimationPreset keyframe type)
- property: kf.property as any,
+ property: kf.property as import('$lib/schemas/animation').AnimatableProperty,As per coding guidelines: "Enable TypeScript strict mode and do not use any type - use unknown or proper types instead."
📝 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.
| const keyframes = hoveredPreset.keyframes.map((kf) => ({ | |
| id: crypto.randomUUID(), | |
| time: kf.time, | |
| property: kf.property as any, | |
| value: kf.value, | |
| interpolation: kf.interpolation | |
| })); | |
| const keyframes = hoveredPreset.keyframes.map((kf) => ({ | |
| id: crypto.randomUUID(), | |
| time: kf.time, | |
| property: kf.property as import('$lib/schemas/animation').AnimatableProperty, | |
| value: kf.value, | |
| interpolation: kf.interpolation | |
| })); |
🧰 Tools
🪛 ESLint
[error] 58-58: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/components/editor/panels/properties/animation-preset-select.svelte`
around lines 55 - 61, Replace the unsafe "as any" cast on kf.property in the
hoveredPreset.keyframes.map call with a proper concrete type: import the
keyframe/property type used by TypedAnimationPreset (e.g., the Keyframe type or
AnimatableProperty) and use that in the mapping (for example cast to
Keyframe['property'] or AnimatableProperty) or annotate the mapped object so
property has the correct type; update the map in the component where
hoveredPreset and keyframes are created to use the concrete type instead of any.
| $effect(() => { | ||
| if (!open) { | ||
| stopPreview(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
$effect missing cleanup return — RAF leaks when component unmounts while popover is open.
If you return a function from the effect, it will be called right before the effect is run again, or when the component is unmounted. Since no cleanup function is returned here, if the component is destroyed while open = true and a preview animation is running, cancelAnimationFrame is never called and the loop runs indefinitely.
🔒 Proposed fix
$effect(() => {
if (!open) {
stopPreview();
}
+ return () => stopPreview();
});📝 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.
| $effect(() => { | |
| if (!open) { | |
| stopPreview(); | |
| } | |
| }); | |
| $effect(() => { | |
| if (!open) { | |
| stopPreview(); | |
| } | |
| return () => stopPreview(); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/components/editor/panels/properties/animation-preset-select.svelte`
around lines 104 - 108, The effect using $effect(() => { if (!open) {
stopPreview(); } }) lacks a cleanup, so when the component unmounts while open
the RAF loop never gets cancelled; update the $effect callback to return a
cleanup function that calls stopPreview (or directly cancels the animation
frame) so that any running preview is stopped both when the effect re-runs and
when the component is destroyed; locate the $effect block in
animation-preset-select.svelte and return a cleanup that invokes
stopPreview/cancelAnimationFrame accordingly.
| <InputsWrapper | ||
| fields={[ | ||
| { for: 'enter-preset', labels: 'Animation' }, | ||
| { for: 'enter-duration', labels: 'Duration (s)' } | ||
| ]} | ||
| > | ||
| <AnimationPresetSelect | ||
| value={enterPresetId} | ||
| options={enterPresets} | ||
| placeholder="None" | ||
| onchange={(v) => { | ||
| enterPresetId = v; | ||
| updateTransition('enter', v, enterDuration); | ||
| }} | ||
| /> | ||
| {#if enterPresetId} | ||
| <ScrubInput | ||
| id="enter-duration" | ||
| value={enterDuration} | ||
| min={0.1} | ||
| max={5} | ||
| step={0.1} | ||
| onchange={(v) => { | ||
| enterDuration = v; | ||
| updateTransition('enter', enterPresetId, v); | ||
| }} | ||
| /> | ||
| {/if} | ||
| </InputsWrapper> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -e svelte "inputs-wrapper" -HRepository: epavanello/devmotion
Length of output: 118
🏁 Script executed:
cat -n src/lib/components/editor/panels/inputs-wrapper.svelteRepository: epavanello/devmotion
Length of output: 2427
Fix orphaned "Duration (s)" label when no animation preset is selected.
The fields array declares both Animation and Duration (s) labels, but the ScrubInput is conditionally rendered only when a preset exists. This causes the Duration (s) label (with for="enter-duration") to render without a corresponding input element, breaking the label-to-input association. Either render the ScrubInput unconditionally (disabled when no preset), or conditionally render both the field descriptor and child input together.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/components/editor/panels/properties/groups/time-range-group.svelte`
around lines 71 - 99, The label "Duration (s)" in the InputsWrapper fields array
is orphaned when enterPresetId is falsy because ScrubInput is conditionally
rendered; fix by keeping label/input in sync: either render the ScrubInput
unconditionally (in the same location) and set it disabled when enterPresetId is
null/undefined (preserve id="enter-duration", value={enterDuration},
min/max/step and call updateTransition only when enabled), or move the
corresponding fields entry out of the static fields array and only include the {
for: 'enter-duration', labels: 'Duration (s)' } descriptor when enterPresetId is
truthy so the InputsWrapper’s fields match the rendered ScrubInput; update
references to enterPresetId, enterDuration, ScrubInput, InputsWrapper,
AnimationPresetSelect, and updateTransition accordingly.
| let transitionTransform: Partial<Transform> = {}; | ||
| let transitionStyle: Partial<LayerStyle> = {}; | ||
|
|
||
| // Apply enter transition if within duration | ||
| if (layer.enterTransition && currentTime >= enterTime) { | ||
| const timeSinceEnter = currentTime - enterTime; | ||
| if (timeSinceEnter <= layer.enterTransition.duration) { | ||
| const preset = getPresetById(layer.enterTransition.presetId); | ||
| if (preset) { | ||
| const { transform, style } = applyPresetAtTime( | ||
| preset.keyframes, | ||
| timeSinceEnter, | ||
| layer.enterTransition.duration | ||
| ); | ||
| transitionTransform = { ...transitionTransform, ...transform }; | ||
| transitionStyle = { ...transitionStyle, ...style }; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Apply exit transition if within duration | ||
| if (layer.exitTransition && currentTime <= exitTime) { | ||
| const timeUntilExit = exitTime - currentTime; | ||
| if (timeUntilExit <= layer.exitTransition.duration) { | ||
| const preset = getPresetById(layer.exitTransition.presetId); | ||
| if (preset) { | ||
| // For exit transitions, we interpolate from end backwards | ||
| const transitionProgress = layer.exitTransition.duration - timeUntilExit; | ||
| const { transform, style } = applyPresetAtTime( | ||
| preset.keyframes, | ||
| transitionProgress, | ||
| layer.exitTransition.duration | ||
| ); | ||
| transitionTransform = { ...transitionTransform, ...transform }; | ||
| transitionStyle = { ...transitionStyle, ...style }; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Shallow spread loses sub-properties when enter and exit transitions overlap on the same top-level key.
Lines 128 and 147 use { ...transitionTransform, ...transform }. If both the enter and exit transitions are active simultaneously (possible when enterTransition.duration + exitTransition.duration > exitTime - enterTime) and both affect a nested key like position, the exit spread replaces the entire position object — discarding any axis contribution from the enter transition. For example, enter sets position.x and exit sets position.y; the result retains only exit's {x: 0, y: <exit>}.
Consider a deep-merge strategy for position, rotation, and scale sub-objects:
♻️ Proposed deep-merge fix for applyTransitionPresets
- transitionTransform = { ...transitionTransform, ...transform };
- transitionStyle = { ...transitionStyle, ...style };
+ transitionTransform = {
+ ...transitionTransform,
+ ...transform,
+ ...(transitionTransform.position || transform.position
+ ? { position: { ...transitionTransform.position, ...transform.position } }
+ : {}),
+ ...(transitionTransform.rotation || transform.rotation
+ ? { rotation: { ...transitionTransform.rotation, ...transform.rotation } }
+ : {}),
+ ...(transitionTransform.scale || transform.scale
+ ? { scale: { ...transitionTransform.scale, ...transform.scale } }
+ : {})
+ };
+ transitionStyle = { ...transitionStyle, ...style };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/engine/layer-rendering.ts` around lines 114 - 151, The shallow
spreads into transitionTransform/transitionStyle lose nested properties when
enter and exit presets overlap; update the merge logic inside the enter/exit
branches (where transitionTransform and transitionStyle are assigned after
applyPresetAtTime) to perform a deep-merge for known nested keys (at minimum
position, rotation, scale and any nested style objects) instead of simple {
...transitionTransform, ...transform } and { ...transitionStyle, ...style };
implement merging by copying top-level keys and, for keys that are objects
(e.g., position), merge their sub-keys (x,y,z or axis fields) so enter and exit
contributions are combined, keeping existing values when the other side doesn't
provide that sub-key, and apply the same deep-merge approach for style when
combining style objects returned from applyPresetAtTime.
| // For exit transitions, we interpolate from end backwards | ||
| const transitionProgress = layer.exitTransition.duration - timeUntilExit; |
There was a problem hiding this comment.
Misleading comment: exit preset plays forward, not "from end backwards".
transitionProgress increases from 0 to layer.exitTransition.duration as the layer approaches its exit — that's forward playback, not reverse. The comment "we interpolate from end backwards" is incorrect and could confuse future readers. Exit presets must be designed to animate from the visible to the hidden state (e.g., opacity 1 → 0) so that forward playback produces the correct exit effect.
📝 Proposed fix
- // For exit transitions, we interpolate from end backwards
- const transitionProgress = layer.exitTransition.duration - timeUntilExit;
+ // For exit transitions, play the preset forward as the layer approaches exit.
+ // Exit presets must be authored to animate from visible → hidden state.
+ const transitionProgress = layer.exitTransition.duration - timeUntilExit;📝 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.
| // For exit transitions, we interpolate from end backwards | |
| const transitionProgress = layer.exitTransition.duration - timeUntilExit; | |
| // For exit transitions, play the preset forward as the layer approaches exit. | |
| // Exit presets must be authored to animate from visible → hidden state. | |
| const transitionProgress = layer.exitTransition.duration - timeUntilExit; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/engine/layer-rendering.ts` around lines 140 - 141, Update the
misleading comment above the calculation of transitionProgress in
layer-rendering.ts: replace "we interpolate from end backwards" with a clear
note that transitionProgress increases from 0 to layer.exitTransition.duration
and therefore exit presets play forward (animate visible→hidden, e.g., opacity
1→0) so designers should author exit presets accordingly; reference the
transitionProgress variable and layer.exitTransition.duration/timeUntilExit to
locate the line.
| preset('slide-in-left', 'Slide In from Left', 'enter', [ | ||
| kf(0, 'position.x', -500, { family: 'continuous', strategy: 'ease-out' }), | ||
| kf(0, 'opacity', 0, { family: 'continuous', strategy: 'ease-out' }), | ||
| kf(1, 'position.x', 0, { family: 'continuous', strategy: 'linear' }), | ||
| kf(1, 'opacity', 1, { family: 'continuous', strategy: 'linear' }) | ||
| ]), | ||
| preset('slide-in-right', 'Slide In from Right', 'enter', [ | ||
| kf(0, 'position.x', 500, { family: 'continuous', strategy: 'ease-out' }), | ||
| kf(0, 'opacity', 0, { family: 'continuous', strategy: 'ease-out' }), | ||
| kf(1, 'position.x', 0, { family: 'continuous', strategy: 'linear' }), | ||
| kf(1, 'opacity', 1, { family: 'continuous', strategy: 'linear' }) | ||
| ]), | ||
| preset('slide-in-top', 'Slide In from Top', 'enter', [ | ||
| kf(0, 'position.y', -400, { family: 'continuous', strategy: 'ease-out' }), | ||
| kf(0, 'opacity', 0, { family: 'continuous', strategy: 'ease-out' }), | ||
| kf(1, 'position.y', 0, { family: 'continuous', strategy: 'linear' }), | ||
| kf(1, 'opacity', 1, { family: 'continuous', strategy: 'linear' }) | ||
| ]), | ||
| preset('slide-in-bottom', 'Slide In from Bottom', 'enter', [ | ||
| kf(0, 'position.y', 400, { family: 'continuous', strategy: 'ease-out' }), | ||
| kf(0, 'opacity', 0, { family: 'continuous', strategy: 'ease-out' }), | ||
| kf(1, 'position.y', 0, { family: 'continuous', strategy: 'linear' }), | ||
| kf(1, 'opacity', 1, { family: 'continuous', strategy: 'linear' }) | ||
| ]), | ||
| preset('slide-out-left', 'Slide Out to Left', 'exit', [ | ||
| kf(0, 'position.x', 0, { family: 'continuous', strategy: 'ease-in' }), | ||
| kf(0, 'opacity', 1, { family: 'continuous', strategy: 'ease-in' }), | ||
| kf(1, 'position.x', -500, { family: 'continuous', strategy: 'linear' }), | ||
| kf(1, 'opacity', 0, { family: 'continuous', strategy: 'linear' }) | ||
| ]), | ||
| preset('slide-out-right', 'Slide Out to Right', 'exit', [ | ||
| kf(0, 'position.x', 0, { family: 'continuous', strategy: 'ease-in' }), | ||
| kf(0, 'opacity', 1, { family: 'continuous', strategy: 'ease-in' }), | ||
| kf(1, 'position.x', 500, { family: 'continuous', strategy: 'linear' }), | ||
| kf(1, 'opacity', 0, { family: 'continuous', strategy: 'linear' }) | ||
| ]), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files src/lib/engine --type fRepository: epavanello/devmotion
Length of output: 2144
🏁 Script executed:
cat -n src/lib/engine/presets.ts | head -160Repository: epavanello/devmotion
Length of output: 7140
🏁 Script executed:
fd 'render\|interpolat' src/lib/engine --type fRepository: epavanello/devmotion
Length of output: 46
🏁 Script executed:
cat -n src/lib/engine/presets.ts | sed -n '200,260p'Repository: epavanello/devmotion
Length of output: 3966
🏁 Script executed:
git ls-files src/lib/engineRepository: epavanello/devmotion
Length of output: 222
🏁 Script executed:
rg -n 'getPresetById|applyPreset|keyframe' src/lib/engineRepository: epavanello/devmotion
Length of output: 5905
🏁 Script executed:
cat -n src/lib/engine/layer-rendering.ts | sed -n '115,220p'Repository: epavanello/devmotion
Length of output: 4486
🏁 Script executed:
rg -n -A 20 'function applyPresetAtTime' src/lib/engine/layer-rendering.tsRepository: epavanello/devmotion
Length of output: 778
🏁 Script executed:
cat -n src/lib/engine/layer-rendering.ts | sed -n '1,115p'Repository: epavanello/devmotion
Length of output: 5008
🏁 Script executed:
rg -n -A 10 'getTransitionTransformAndStyle\|transitionTransform\|transitionStyle' src/lib/engine/layer-rendering.ts | head -80Repository: epavanello/devmotion
Length of output: 46
🏁 Script executed:
cat -n src/lib/engine/layer-rendering.ts | sed -n '210,260p'Repository: epavanello/devmotion
Length of output: 2019
🏁 Script executed:
rg -n 'position\|offset\|relative' src/lib/engine/layer-rendering.ts | head -40Repository: epavanello/devmotion
Length of output: 46
🏁 Script executed:
fd 'test\|spec' src/lib/engine --type fRepository: epavanello/devmotion
Length of output: 46
🏁 Script executed:
rg -n 'layer.transform.position\|transform.position' src/lib/engine/layer-rendering.ts | head -20Repository: epavanello/devmotion
Length of output: 46
Preset position values override base layer positions instead of applying as offsets.
The applyPresetAtTime function in layer-rendering.ts (lines 213-221) assigns preset position values directly to the transform, then uses nullish coalescing to merge: transitionTransform.position?.x ?? animatedTransform.position?.x ?? layer.transform.position.x. This means preset values completely replace the layer's base position instead of offsetting from it.
For a layer with base position (300, 200):
slide-in-leftanimates to positionx=0instead ofx=300(end position wrong by 300)slide-in-topanimates to positiony=0instead ofy=200(end position wrong by 200)shakeandfloatpresets similarly assume base position is(0, 0)
Preset values should be added to (or blended relative to) the layer's base position, not replace it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/engine/presets.ts` around lines 114 - 149, The preset position keys
are being applied as absolute values instead of offsets in applyPresetAtTime
(layer-rendering.ts); update the merge logic for position so preset and animated
position components are added to the layer's base transform rather than
replacing it — e.g., compute finalX = layer.transform.position.x +
(transitionTransform.position?.x ?? 0) + (animatedTransform.position?.x ?? 0)
and likewise for finalY, and use those in place of the current
nullish-coalescing chain; adjust any places that read
transitionTransform.position.* or animatedTransform.position.* so they default
to 0 when missing, and ensure presets like
slide-in-left/slide-in-top/shake/float now act as offsets relative to the
layer's base position.
1 participant
Summary by CodeRabbit
Release Notes
New Features
UI/UX Improvements