Skip to content

wip #28

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
epavanello wants to merge 1 commit into
base: main
Choose a base branch
from
Open

wip #28

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/lib/components/ai/model-selector.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<script lang="ts">
import { ChevronDown, Lock } from '@lucide/svelte';
import { ChevronDown } from '@lucide/svelte';
import { AI_MODELS, getModel } from '$lib/ai/models';
import * as DropdownMenu from '$lib/components/ui/dropdown-menu/index.js';
import { getUser } from '$lib/functions/auth.remote';
Expand Down
4 changes: 2 additions & 2 deletions src/lib/components/editor/canvas/layers-renderer.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@
}
// Otherwise compute using shared rendering functions
return {
transform: getLayerTransform(layer, currentTime),
style: getLayerStyle(layer, currentTime),
transform: getLayerTransform(layer, currentTime, duration),
style: getLayerStyle(layer, currentTime, duration),
customProps: getLayerProps(layer, currentTime)
};
}
Expand Down
16 changes: 4 additions & 12 deletions src/lib/components/editor/panels/properties-panel.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,15 @@
import type { Layer } from '$lib/schemas/animation';
import { getDefaultInterpolationForProperty } from '$lib/utils/interpolation-utils';
import { defaultLayerStyle, defaultTransform } from '$lib/schemas/base';
import { Clock, Move, Palette, Layers, Sparkles, Boxes } from '@lucide/svelte';
import { Clock, Move, Palette, Layers, Boxes } from '@lucide/svelte';

// Property group components
import TimeRangeGroup from './properties/groups/time-range-group.svelte';
import TransformGroup from './properties/groups/transform-group.svelte';
import StyleGroup from './properties/groups/style-group.svelte';
import AnimationPresetsGroup from './properties/groups/animation-presets-group.svelte';
import KeyframesGroup from './properties/groups/keyframes-group.svelte';
import LayerPropertiesGroup from './properties/groups/layer-properties-group.svelte';
import AnimationPresetsGroup from './properties/groups/animation-presets-group.svelte';

const editorState = $derived(getEditorState());
const projectStore = $derived(editorState.project);
Expand Down Expand Up @@ -334,11 +334,11 @@

<Separator />

<!-- Time Range -->
<!-- Time Range & Transitions -->
<PropertiesGroup icon={Clock}>
{#snippet label()}
<div class="flex w-full items-center justify-between">
<Label class="font-semibold">Time Range</Label>
<Label class="font-semibold">Time & Transitions</Label>
{#if selectedLayer.contentDuration !== undefined}
{@const contentDuration = selectedLayer.contentDuration}
{@const contentOffset = selectedLayer.contentOffset ?? 0}
Expand All @@ -350,14 +350,6 @@
</div>
{/snippet}
<TimeRangeGroup layer={selectedLayer} />
</PropertiesGroup>
{/if}

{#if !isProjectSettings}
<Separator />

<!-- Animation Presets -->
<PropertiesGroup label="Animation Presets" icon={Sparkles}>
<AnimationPresetsGroup layer={selectedLayer} />
</PropertiesGroup>

Expand Down
157 changes: 157 additions & 0 deletions src/lib/components/editor/panels/properties/animation-preset-select.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
<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';

Comment on lines +1 to +8
Copy link

@coderabbitai coderabbitai bot Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, find where createLayer is exported
rg "export.*createLayer" -t ts -t tsx -t svelte -A 2

Repository: 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 -100

Repository: 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 5

Repository: epavanello/devmotion

Length of output: 91

🏁 Script executed:

# Search for createLayer export - using correct file types
rg "export.*createLayer" -t ts -t svelte -A 2

Repository: 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 3

Repository: 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.svelte

Repository: 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.

const {
value,
options,
placeholder = 'Select animation',
onchange
}: {
value: string;
options: TypedAnimationPreset[];
placeholder?: string;
onchange: (value: string) => void;
} = $props();

let open = $state(false);
let hoveredPresetId = $state<string | null>(null);
let previewTime = $state(0);
let animationFrameId = $state<number | null>(null);
Copy link

@coderabbitai coderabbitai bot Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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.

Suggested change
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 selectedPreset = $derived(options.find((opt) => opt.id === value));
const hoveredPreset = $derived(options.find((opt) => opt.id === hoveredPresetId));

// Create a dummy layer for preview
const previewLayer = $derived(
createLayer('rectangle', {
name: 'Preview',
transform: {
position: { x: 0, y: 0, z: 0 },
rotation: { x: 0, y: 0, z: 0 },
scale: { x: 1, y: 1, z: 1 },
anchor: 'center'
},
style: { opacity: 1 },
props: {
width: 60,
height: 60,
fill: '#3b82f6',
stroke: '#2563eb',
strokeWidth: 2,
borderRadius: 8
}
})
);

// Apply preset keyframes to preview layer
const previewLayerWithKeyframes = $derived.by(() => {
if (!hoveredPreset) return previewLayer;

const keyframes = hoveredPreset.keyframes.map((kf) => ({
id: crypto.randomUUID(),
time: kf.time,
property: kf.property as any,
value: kf.value,
interpolation: kf.interpolation
}));
Comment on lines +55 to +61
Copy link

@coderabbitai coderabbitai bot Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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.

Suggested change
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.


return { ...previewLayer, keyframes };
});

// Get current transform and style for preview at current time
const previewTransform = $derived(getLayerTransform(previewLayerWithKeyframes, previewTime, 1));
const previewStyle = $derived(getLayerStyle(previewLayerWithKeyframes, previewTime, 1));
const previewTransformCSS = $derived(generateTransformCSS(previewTransform));
const previewFilterCSS = $derived(generateFilterCSS(previewStyle));

function startPreview(presetId: string) {
hoveredPresetId = presetId;
previewTime = 0;

// Animate preview
const startTime = performance.now();
const duration = 1000; // 1 second loop

function animate(currentTime: number) {
const elapsed = currentTime - startTime;
previewTime = (elapsed % duration) / duration;
animationFrameId = requestAnimationFrame(animate);
}

animationFrameId = requestAnimationFrame(animate);
}

function stopPreview() {
hoveredPresetId = null;
previewTime = 0;
if (animationFrameId !== null) {
cancelAnimationFrame(animationFrameId);
animationFrameId = null;
}
}

function selectPreset(presetId: string) {
onchange(presetId);
open = false;
stopPreview();
}

$effect(() => {
if (!open) {
stopPreview();
}
});
Comment on lines +104 to +108
Copy link

@coderabbitai coderabbitai bot Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

$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.

Suggested change
$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.

</script>

<Popover.Root bind:open>
<Popover.Trigger asChild let:props>
<Button variant="outline" class="w-full justify-between" {...props}>
{selectedPreset?.name ?? placeholder}
<ChevronDown class="ml-2 h-4 w-4 opacity-50" />
</Button>
</Popover.Trigger>
<Popover.Content class="w-80 p-2" align="start">
<div class="grid grid-cols-1 gap-1">
{#each options as preset (preset.id)}
{@const isSelected = value === preset.id}
{@const isHovered = hoveredPresetId === preset.id}
<button
class="flex items-center justify-between rounded px-3 py-2 text-sm transition-colors hover:bg-accent"
class:bg-accent={isSelected}
onmouseenter={() => startPreview(preset.id)}
onmouseleave={stopPreview}
onclick={() => selectPreset(preset.id)}
>
<span class="flex-1 text-left">{preset.name}</span>
{#if isHovered}
<div
class="relative ml-2 flex h-16 w-16 items-center justify-center overflow-hidden rounded border border-border bg-background"
>
<div
class="absolute top-1/2 left-1/2"
style:transform={previewTransformCSS}
style:filter={previewFilterCSS}
style:opacity={previewStyle.opacity}
>
<div
class="rounded"
style:width="{previewLayer.props.width}px"
style:height="{previewLayer.props.height}px"
style:background={previewLayer.props.fill}
style:border="{previewLayer.props.strokeWidth}px solid {previewLayer.props
.stroke}"
style:border-radius="{previewLayer.props.borderRadius}px"
></div>
</div>
</div>
{/if}
</button>
{/each}
</div>
</Popover.Content>
</Popover.Root>
108 changes: 100 additions & 8 deletions src/lib/components/editor/panels/properties/groups/time-range-group.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,48 @@
<script lang="ts">
import { Label } from '$lib/components/ui/label';
import { Button } from '$lib/components/ui/button';
import ScrubInput from '../../scrub-input.svelte';
import { Label } from '$lib/components/ui/label';
import { getEditorState } from '$lib/contexts/editor.svelte';
import type { TypedLayer } from '$lib/layers/typed-registry';
import { getEnterPresets, getExitPresets } from '$lib/engine/presets';
import type { LayerTransition } from '$lib/schemas/animation';
import InputsWrapper from '../../inputs-wrapper.svelte';
import ScrubInput from '../../scrub-input.svelte';
import AnimationPresetSelect from '../animation-preset-select.svelte';

const { layer }: { layer: TypedLayer } = $props();

const editorState = $derived(getEditorState());
const projectStore = $derived(editorState.project);

const enterPresets = $derived([
{ id: '', name: 'None', category: 'enter' as const, keyframes: [] },
...getEnterPresets()
]);

const exitPresets = $derived([
{ id: '', name: 'None', category: 'exit' as const, keyframes: [] },
...getExitPresets()
]);

let enterPresetId = $derived<string>(layer.enterTransition?.presetId ?? '');
let enterDuration = $derived<number>(layer.enterTransition?.duration ?? 0.5);
let exitPresetId = $derived<string>(layer.exitTransition?.presetId ?? '');
let exitDuration = $derived<number>(layer.exitTransition?.duration ?? 0.5);

function updateTransition(type: 'enter' | 'exit', presetId: string, duration: number) {
const transition: LayerTransition | undefined = presetId ? { presetId, duration } : undefined;
const key = type === 'enter' ? 'enterTransition' : 'exitTransition';
projectStore.updateLayer(layer.id, { [key]: transition });
}
</script>

<div class="grid grid-cols-2 gap-2">
<div class="space-y-1">
<Label class="text-xs text-muted-foreground">Enter (s)</Label>
<div class="space-y-3">
<InputsWrapper
fields={[
{ for: 'enter-time', labels: 'Enter (s)' },
{ for: 'exit-time', labels: 'Exit (s)' }
]}
>
<ScrubInput
id="enter-time"
value={layer.enterTime ?? 0}
Expand All @@ -27,9 +56,6 @@
step={0.1}
onchange={(v) => projectStore.setLayerEnterTime(layer.id, v)}
/>
</div>
<div class="space-y-1">
<Label class="text-xs text-muted-foreground">Exit (s)</Label>
<ScrubInput
id="exit-time"
value={layer.exitTime ?? projectStore.state.duration}
Expand All @@ -38,6 +64,72 @@
step={0.1}
onchange={(v) => projectStore.setLayerExitTime(layer.id, v)}
/>
</InputsWrapper>

<div class="space-y-2">
<Label class="text-xs text-muted-foreground">Enter Transition</Label>
<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>
Comment on lines +71 to +99
Copy link

@coderabbitai coderabbitai bot Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

fd -e svelte "inputs-wrapper" -H

Repository: epavanello/devmotion

Length of output: 118

🏁 Script executed:

cat -n src/lib/components/editor/panels/inputs-wrapper.svelte

Repository: 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.

</div>

<div class="space-y-2">
<Label class="text-xs text-muted-foreground">Exit Transition</Label>
<InputsWrapper
fields={[
{ for: 'exit-preset', labels: 'Animation' },
{ for: 'exit-duration', labels: 'Duration (s)' }
]}
>
<AnimationPresetSelect
value={exitPresetId}
options={exitPresets}
placeholder="None"
onchange={(v) => {
exitPresetId = v;
updateTransition('exit', v, exitDuration);
}}
/>
{#if exitPresetId}
<ScrubInput
id="exit-duration"
value={exitDuration}
min={0.1}
max={5}
step={0.1}
onchange={(v) => {
exitDuration = v;
updateTransition('exit', exitPresetId, v);
}}
/>
{/if}
</InputsWrapper>
</div>
</div>

Expand Down
Loading