Merged
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughCentralizes layer transform/style defaults, extends LayerStyle with filter and drop-shadow fields, broadens easing strategies, refactors the properties panel into modular group components, updates interpolation/rendering to include new style fields, and adds shape/text schema fields (ellipse, borderRadius, letterSpacing, lineHeight). Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (2 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/lib/layers/components/ShapeLayer.svelte (1)
68-83:⚠️ Potential issue | 🟡 MinorUpdate the description to include
ellipse.The
descriptionstring on line 75 still says "rectangle, circle, triangle, polygon" but the schema now also supportsellipse.📝 Proposed fix
description: - 'Geometric shapes (rectangle, circle, triangle, polygon) with background and stroke', + 'Geometric shapes (rectangle, ellipse, circle, triangle, polygon) with background and stroke',src/lib/stores/project.svelte.ts (2)
234-261:⚠️ Potential issue | 🟠 Major
ungroupLayersonly propagatesopacity— new style properties are silently dropped.When dissolving a group, only
opacityis composed into children (line 259). The newly added style properties (blur,brightness,contrast,saturate,dropShadowX/Y/Blur/Color) are ignored, so any group-level filter effects will be lost instead of baked into children.At minimum, the multiplicative filter values (brightness, contrast, saturate) and additive values (blur, dropShadow offsets) should be propagated similarly to how transform offsets are baked in.
Proposed fix (compose all style properties)
style: { ...layer.style, - opacity: layer.style.opacity * gs.opacity + opacity: layer.style.opacity * gs.opacity, + blur: (layer.style.blur ?? 0) + (gs.blur ?? 0), + brightness: (layer.style.brightness ?? 1) * (gs.brightness ?? 1), + contrast: (layer.style.contrast ?? 1) * (gs.contrast ?? 1), + saturate: (layer.style.saturate ?? 1) * (gs.saturate ?? 1), + dropShadowX: (layer.style.dropShadowX ?? 0) + (gs.dropShadowX ?? 0), + dropShadowY: (layer.style.dropShadowY ?? 0) + (gs.dropShadowY ?? 0), + dropShadowBlur: (layer.style.dropShadowBlur ?? 0) + (gs.dropShadowBlur ?? 0), + dropShadowColor: gs.dropShadowColor !== 'transparent' ? gs.dropShadowColor : layer.style.dropShadowColor }
342-347:⚠️ Potential issue | 🟠 MajorSame style propagation gap in
removeLayerFromGroup.This block has the same issue as
ungroupLayers— onlyopacityis composed from the group style. The fix should mirror whatever composition logic is applied inungroupLayersfor the new filter/shadow properties.
🤖 Fix all issues with AI agents
In `@src/lib/components/editor/panels/properties-panel.svelte`:
- Around line 847-854: The color input can't represent 'transparent' so add a
small control to toggle/reset the transparent state: introduce an "Enable drop
shadow" checkbox (or a small "Reset" button) next to the color input that uses
the existing updateProperty function to set dropShadowColor to 'transparent'
(and optionally restore a default hex when enabling); update the color input
rendering to treat currentValues?.style.dropShadowColor === 'transparent' as the
disabled/unchecked state (disable the color picker or show a visual cue) and
only pass a hex value to the input when the value is not 'transparent' (use
currentValues?.style.dropShadowColor when not 'transparent', else a safe
fallback for the input UI), referencing the id "style.dropShadowColor", the
property name 'dropShadowColor', the updateProperty function, currentValues, and
the surrounding InputWrapper to locate and modify the controls.
🧹 Nitpick comments (7)
src/lib/schemas/base.ts (2)
134-141: Consider deriving defaults from the schema to stay DRY.
defaultTransform()duplicates the fallback values already expressed insideTransformSchema.transform(...). If someone changes the schema defaults without updating this function (or vice versa), they'll silently diverge.One option:
export function defaultTransform(): Transform { return TransformSchema.parse({ position: { x: 0, y: 0, z: 0 } }); }That said, explicit literals are faster at runtime and arguably clearer, so this is only a suggestion.
167-179: Same DRY observation applies todefaultLayerStyle().The values here mirror
LayerStyleSchema's.default()values. You could derive them viaLayerStyleSchema.parse({})to guarantee consistency, but the explicit approach is also fine for performance.src/lib/layers/components/ShapeLayer.svelte (1)
42-49:.default(0).optional()widens the output type unnecessarily.In Zod 4,
.default(0)already handlesundefinedinput by returning0. Appending.optional()doesn't change parse behavior but makes the inferred TypeScript output typenumber | undefined, forcing the destructuring default on line 96. If this is intentional (to mark the field as optional in the schema for UI/metadata purposes), it's fine — otherwise remove.optional().src/lib/ai/mutations.ts (1)
502-518: Ungrouping only bakesopacityinto children — new filter properties are silently dropped.When ungrouping, the code merges the group's transform and opacity into children (line 517), but the new style properties (
blur,brightness,contrast,saturate,dropShadow*) are not propagated. If a user sets e.g.blur: 5on a group and then ungroups, that blur is lost.This was a pre-existing limitation for opacity-only style, but with the expanded style surface it becomes more impactful.
💡 Suggested enhancement
layer.style.opacity *= gs.opacity; + layer.style.blur += gs.blur; + layer.style.brightness *= gs.brightness; + layer.style.contrast *= gs.contrast; + layer.style.saturate *= gs.saturate; + layer.style.dropShadowX += gs.dropShadowX; + layer.style.dropShadowY += gs.dropShadowY; + layer.style.dropShadowBlur += gs.dropShadowBlur; + // dropShadowColor composition is non-trivial — may need special handlingsrc/lib/components/editor/panels/add-layer.svelte (1)
18-25: RedundantdefaultTransform()override —createLayeralready applies it internally.
createLayer(inlayer-factory.tsline 75) already spreadsdefaultTransform()as the base, so passing it again as the override just duplicates the same values. You can simplify by omitting thetransformkey entirely unless you intend to provide position offsets.Suggested simplification
const layer = createLayer(type, { - transform: defaultTransform(), projectDimensions: { width: projectStore.state.width, height: projectStore.state.height } });src/lib/engine/interpolation.ts (2)
187-294:BezierEasinginstances are re-created on every call — consider memoizing.
getEasingFunctionis invoked per-property per-frame during animation playback and recording. EachBezierEasing(...)call allocates a new instance and builds an internal lookup table. Since the control points are constant per strategy, you can cache them at module scope:Suggested memoization approach
+const easingCache = new Map<string, (t: number) => number>(); + +function cachedBezier( + strategy: string, + x1: number, + y1: number, + x2: number, + y2: number +): (t: number) => number { + let fn = easingCache.get(strategy); + if (!fn) { + fn = BezierEasing(x1, y1, x2, y2); + easingCache.set(strategy, fn); + } + return fn; +} + function getEasingFunction(strategy: ContinuousInterpolationStrategy): (t: number) => number { switch (strategy) { case 'ease-in': - return BezierEasing(0.42, 0, 1.0, 1.0); + return cachedBezier('ease-in', 0.42, 0, 1.0, 1.0); // ... same for all other bezier cases
360-422:getAnimatedStyleexpansion is correct but verbose — could extract a helper.The repetitive
getPropertyValue→ null-check → assign pattern could be compressed with a small helper, but this is a purely optional cleanup given the code is clear as-is.Optional: helper to reduce repetition
function assignIfAnimated<K extends keyof LayerStyle>( style: Partial<LayerStyle>, keyframes: Keyframe[], property: K & AnimatableProperty, currentTime: number ) { const value = getPropertyValue(keyframes, property, currentTime); if (value !== null) { (style as Record<string, unknown>)[property] = value; } }
Comment on lines
847
to
854
| <input | ||
| id="style.dropShadowColor" | ||
| type="color" | ||
| value={currentValues?.style.dropShadowColor ?? '#000000'} | ||
| onchange={(e) => updateProperty('dropShadowColor', e.currentTarget.value, 'style')} | ||
| class="h-8 w-full cursor-pointer rounded border" | ||
| /> | ||
| </InputWrapper> |
There was a problem hiding this comment.
Native color input can't represent 'transparent' — user has no way to reset drop-shadow color.
The <input type="color"> element only supports opaque hex colors. The schema default for dropShadowColor is 'transparent', but once a user picks a color they have no way to restore it to transparent via this control. The fallback ?? '#000000' also means the picker shows black even when the actual value is 'transparent'.
Consider adding a small toggle/checkbox (e.g., "Enable drop shadow") or a reset button that sets the color back to 'transparent'.
🤖 Prompt for AI Agents
In `@src/lib/components/editor/panels/properties-panel.svelte` around lines 847 -
854, The color input can't represent 'transparent' so add a small control to
toggle/reset the transparent state: introduce an "Enable drop shadow" checkbox
(or a small "Reset" button) next to the color input that uses the existing
updateProperty function to set dropShadowColor to 'transparent' (and optionally
restore a default hex when enabling); update the color input rendering to treat
currentValues?.style.dropShadowColor === 'transparent' as the disabled/unchecked
state (disable the color picker or show a visual cue) and only pass a hex value
to the input when the value is not 'transparent' (use
currentValues?.style.dropShadowColor when not 'transparent', else a safe
fallback for the input UI), referencing the id "style.dropShadowColor", the
property name 'dropShadowColor', the updateProperty function, currentValues, and
the surrounding InputWrapper to locate and modify the controls.
… property management in the editor panel.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/lib/layers/components/ButtonLayer.svelte (1)
122-122:⚠️ Potential issue | 🟡 MinorTypo: leading space in property group id
' appearance'.The group id is
' appearance'(with a leading space) but the schema fields register withgroup: 'appearance'(no space) on lines 63, 68, 77. This mismatch means those fields won't be grouped under "Appearance" in the properties panel.🐛 Proposed fix
- { id: ' appearance', label: 'Appearance' }, + { id: 'appearance', label: 'Appearance' },🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and decide whether a code change is actually needed. If the finding is not valid or no change is required, do not modify code for that item and briefly explain why it was skipped. In `@src/lib/layers/components/ButtonLayer.svelte` at line 122, The group id for the Appearance property group in ButtonLayer.svelte has a leading space (' appearance') which doesn't match the group value used by schema fields ('appearance'); update the group's id to exactly 'appearance' so fields registered with group: 'appearance' are correctly grouped under the Appearance panel. Locate the group definition (id: ' appearance') in ButtonLayer.svelte and remove the leading space from the id string to match the schema fields (group: 'appearance').src/lib/layers/components/ShapeLayer.svelte (1)
74-75:⚠️ Potential issue | 🟡 MinorDescription string is stale — missing "ellipse".
The
descriptionfield still lists "rectangle, circle, triangle, polygon" but the enum now includesellipse.- 'Geometric shapes (rectangle, circle, triangle, polygon) with background and stroke', + 'Geometric shapes (rectangle, ellipse, circle, triangle, polygon) with background and stroke',🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and decide whether a code change is actually needed. If the finding is not valid or no change is required, do not modify code for that item and briefly explain why it was skipped. In `@src/lib/layers/components/ShapeLayer.svelte` around lines 74 - 75, The description string in the ShapeLayer component is out of date: update the description property (the `description` field in ShapeLayer.svelte/component) to include "ellipse" alongside rectangle, circle, triangle, polygon (and preferably match the enum ordering), so the human-readable description reflects the current shape enum values.
🤖 Fix all issues with AI agents
Before applying any fix, first verify the finding against the current code and
decide whether a code change is actually needed. If the finding is not valid or
no change is required, do not modify code for that item and briefly explain why
it was skipped.
In `@src/lib/components/editor/panels/properties-group.svelte`:
- Line 12: The isOpen store is currently created with $derived(defaultOpen)
which re-syncs whenever the defaultOpen prop changes and thus overrides user
toggles; change this to use $state(defaultOpen) so isOpen is initialized from
defaultOpen once and thereafter managed locally by the component (replace the
$derived(...) instantiation with $state(...) and keep all existing reads/writes
to isOpen intact).
In `@src/lib/components/editor/panels/properties-panel.svelte`:
- Around line 116-127: The inline comment in mapToAnimatable is outdated—update
or replace the comment on the return for the 'style' branch in function
mapToAnimatable to reflect the current style properties (e.g. blur, brightness,
contrast, saturate, dropShadowX, etc.) or remove it entirely; locate the
mapToAnimatable function and modify the trailing comment that currently says "//
style: 'opacity'" so it accurately lists or generically describes supported
style keys.
- Around line 245-263: The propertyValueMap duplicates individual style
properties instead of reusing currentValues.style; update the construction of
propertyValueMap to spread currentValues.style into it (e.g., merge
...currentValues.style) and keep only the explicit transform entries
('position.x','position.y','position.z','rotation.x','rotation.y','rotation.z','scale.x','scale.y')
so new style keys are picked up automatically; locate and modify the
propertyValueMap initialization in properties-panel.svelte where
propertyValueMap and currentValues.style are referenced.
- Around line 82-93: The inline manual fallback object for style in
properties-panel.svelte is fragile; replace it with a programmatic merge that
iterates the schema/defaults (use defaultLayerStyle()) and builds each key as
animatedStyle[key] ?? selectedLayer.style[key] ?? defaultLayerStyle()[key],
casting the result to LayerStyle so all style keys (and future additions) are
handled automatically; update the style assignment in the component and remove
redundant manual entries that must be kept in sync with defaultLayerStyle() and
propertyValueMap.
In `@src/lib/components/editor/panels/properties/groups/keyframes-group.svelte`:
- Around line 1-8: The component filename uses kebab-case
(keyframes-group.svelte) but Svelte components must be PascalCase; rename the
file to KeyframesGroup.svelte and update all imports/usages to the new name
(search for imports referencing "keyframes-group" and replace with
"KeyframesGroup"), ensuring the internal exported component (LayerKeyframes
usage) remains unchanged; also apply the same PascalCase rename for the other
new group components (style-group.svelte → StyleGroup.svelte,
time-range-group.svelte → TimeRangeGroup.svelte, animation-presets-group.svelte
→ AnimationPresetsGroup.svelte, property-group-renderer.svelte →
PropertyGroupRenderer.svelte) to keep naming consistent with existing components
like TextLayer.svelte/ShapeLayer.svelte.
In
`@src/lib/components/editor/panels/properties/groups/layer-properties-group.svelte`:
- Around line 8-15: The prop type intersection currently includes an unused
optional isProjectSettings flag when destructuring props from $props() in the
PropertyGroup (PropertyGroupProps & { isProjectSettings?: boolean }); remove the
`{ isProjectSettings?: boolean }` from that intersection so the destructured
variables are just typed as PropertyGroupProps, and keep the derived
isProjectSettings value computed via $derived(isProjectLayer(layer.id)); update
the destructuring line (the one calling $props()) to use only PropertyGroupProps
to avoid the misleading unused prop.
- Around line 28-38: The code inconsistently uses optional chaining on
layerDefinition: change the PropertyGroupRenderer props to use the same style
for both properties — either remove the optional chain on propertyGroups or add
it to customPropertyComponents; since getLayerDefinition never returns
undefined, update the JSX to access layerDefinition.propertyGroups and
layerDefinition.customPropertyComponents consistently (e.g., use direct property
access on both) so both references match and no unnecessary ?. remains.
- Line 1: Rename the Svelte component file from kebab-case to PascalCase
(layer-properties-group.svelte -> LayerPropertiesGroup.svelte) and update every
import/usage to the new filename (e.g., import LayerPropertiesGroup from
'.../LayerPropertiesGroup.svelte'); ensure any references in parent components,
index exports, or route/component registries that used
"layer-properties-group.svelte" are updated to "LayerPropertiesGroup.svelte" and
that the Svelte component's internal name (if declared) matches the PascalCase
convention.
In
`@src/lib/components/editor/panels/properties/groups/property-group-renderer.svelte`:
- Around line 38-66: Replace the Svelte-specific reactive collection with a
plain JS Set: in the propertyLayout derived computation, change the
renderedGroupIds declaration from using SvelteSet to new Set<string>() and leave
the rest of the logic (has/add checks) unchanged; also remove the now-unused
SvelteSet import from the top of the file so there are no unused imports.
In `@src/lib/components/editor/panels/properties/groups/style-group.svelte`:
- Around line 9-11: The current code wraps a static computation in a reactive
helper ($derived.by), which is unnecessary: replace the reactive call with a
plain module-level constant so the metadata is computed once. Specifically,
remove the $derived.by usage around extractPropertyMetadata(LayerStyleSchema)
and assign its result directly to stylePropertyMetadata (referencing
stylePropertyMetadata, $derived.by, LayerStyleSchema, and
extractPropertyMetadata to locate the code).
In `@src/lib/components/editor/panels/properties/groups/time-range-group.svelte`:
- Around line 21-26: The computed max for ScrubInput can go negative when
layer.contentDuration - (layer.contentOffset ?? 0) exceeds
projectStore.state.duration; update the max calculation (the expression using
layer.contentDuration, layer.contentOffset and projectStore.state.duration) to
clamp the result with Math.max to ensure it is at least 0 (or at least the
current enterTime value) so min (0) ≤ max; locate the max prop usage in the
time-range-group Svelte component and replace the existing ternary result with a
clamped value (e.g., Math.max(0, ... ) or Math.max(enterTime, ... )) so the
ScrubInput always receives a non-negative, valid range.
- Around line 10-11: The $derived wrappers are redundant for stable context
refs: replace const editorState = $derived(getEditorState()) with a direct
reference const editorState = getEditorState(), and replace const projectStore =
$derived(editorState.project) with const projectStore = editorState.project;
this removes unnecessary indirection while still relying on ProjectStore's
internal Svelte 5 reactivity (keep using editorState and
projectStore.state/methods as before).
In `@src/lib/components/editor/panels/properties/groups/transform-group.svelte`:
- Around line 183-192: The click handler for toggling scale lock computes ratio
via const ratio = currentScaleX / currentScaleY which can produce Infinity/NaN
when currentScaleY is 0; update the handler (the onclick block that sets
newProps and calls projectStore.updateLayer) to guard against division by zero
by computing ratio as: if currentScaleY === 0 then use a safe fallback (for
example ratio = currentScaleX === 0 ? 1 : currentScaleX) else ratio =
currentScaleX / currentScaleY, and then set _scaleRatio and _scaleLocked on
layer.props as before when calling projectStore.updateLayer.
- Around line 119-131: The Rotation ScrubXyz intentionally swaps X/Y axes
(valueX/valueY use rotation.y/rotation.x and onchangeX/onchangeY write
rotation.y/rotation.x) to match the 3D gizmo screen-space mapping; add a brief
inline comment above the ScrubXyz block referencing this deliberate mapping and
why (e.g., "screen-space X drag maps to model Y rotation — intentional swap"),
and ensure the comment mentions the related symbols ScrubXyz and updateProperty
so future readers understand the valueX/valueY <-> rotation.y/rotation.x
mapping.
In `@src/lib/components/editor/panels/properties/groups/types.ts`:
- Around line 21-24: The exported type UpdateLayerProperty is unused and its
generic restricts to TypedLayer 'name', which mismatches the actual callback
signature used elsewhere (PropertyGroupProps.updateProperty); either remove this
dead type or change it to match the real callback: replace UpdateLayerProperty
with a signature like (propertyName: string, value: unknown, target: 'transform'
| 'props' | 'style') => void and update its export/name accordingly (referencing
UpdateLayerProperty, TypedLayer, and PropertyGroupProps.updateProperty) so the
type is correct and consumable where needed.
In `@src/lib/layers/components/ButtonLayer.svelte`:
- Line 122: The group id for the Appearance property group in ButtonLayer.svelte
has a leading space (' appearance') which doesn't match the group value used by
schema fields ('appearance'); update the group's id to exactly 'appearance' so
fields registered with group: 'appearance' are correctly grouped under the
Appearance panel. Locate the group definition (id: ' appearance') in
ButtonLayer.svelte and remove the leading space from the id string to match the
schema fields (group: 'appearance').
In `@src/lib/layers/components/ShapeLayer.svelte`:
- Around line 74-75: The description string in the ShapeLayer component is out
of date: update the description property (the `description` field in
ShapeLayer.svelte/component) to include "ellipse" alongside rectangle, circle,
triangle, polygon (and preferably match the enum ordering), so the
human-readable description reflects the current shape enum values.
In `@src/lib/layers/properties/field-registry.ts`:
- Around line 1-4: The file imports Zod as a default; change to the named import
syntax used elsewhere (import { z } from 'zod') and reorder imports so external
packages come first (e.g., 'svelte' and 'zod'), then internal lib imports (e.g.,
'$lib/types/animation'), then relative imports (e.g., '../typed-registry');
update the import line referencing z (currently "import z from 'zod'") to
"import { z } from 'zod'" and reorder the import statements accordingly to match
the project's style.
In `@src/lib/schemas/base.ts`:
- Around line 220-237: The stylePropertyGroups array contains two groups with
identical labels which will render as duplicate "Filters" headings; update the
entries with id 'filters-1' and 'filters-2' in stylePropertyGroups to either
merge them into a single group or give each a distinct label that reflects their
purpose (e.g., "Filters — Blur/Brightness" and "Filters — Contrast/Saturate") so
the UI headings are unambiguous; ensure any code that references these ids still
works if you choose to merge or rename.
- Around line 6-8: Reorder the imports so external packages come first: move the
z import (import { z } from 'zod';) above the internal $lib imports
(fieldRegistry and PropertyGroup). Ensure the resulting top-of-file import order
is: external package import for z, then the internal imports of fieldRegistry
and PropertyGroup to conform to the project import ordering rules.
🧹 Nitpick comments (14)
🤖 Fix all nitpicks with AI agents
Before applying any fix, first verify the finding against the current code and decide whether a code change is actually needed. If the finding is not valid or no change is required, do not modify code for that item and briefly explain why it was skipped. In `@src/lib/components/editor/panels/properties-panel.svelte`: - Around line 116-127: The inline comment in mapToAnimatable is outdated—update or replace the comment on the return for the 'style' branch in function mapToAnimatable to reflect the current style properties (e.g. blur, brightness, contrast, saturate, dropShadowX, etc.) or remove it entirely; locate the mapToAnimatable function and modify the trailing comment that currently says "// style: 'opacity'" so it accurately lists or generically describes supported style keys. - Around line 245-263: The propertyValueMap duplicates individual style properties instead of reusing currentValues.style; update the construction of propertyValueMap to spread currentValues.style into it (e.g., merge ...currentValues.style) and keep only the explicit transform entries ('position.x','position.y','position.z','rotation.x','rotation.y','rotation.z','scale.x','scale.y') so new style keys are picked up automatically; locate and modify the propertyValueMap initialization in properties-panel.svelte where propertyValueMap and currentValues.style are referenced. - Around line 82-93: The inline manual fallback object for style in properties-panel.svelte is fragile; replace it with a programmatic merge that iterates the schema/defaults (use defaultLayerStyle()) and builds each key as animatedStyle[key] ?? selectedLayer.style[key] ?? defaultLayerStyle()[key], casting the result to LayerStyle so all style keys (and future additions) are handled automatically; update the style assignment in the component and remove redundant manual entries that must be kept in sync with defaultLayerStyle() and propertyValueMap. In `@src/lib/components/editor/panels/properties/groups/keyframes-group.svelte`: - Around line 1-8: The component filename uses kebab-case (keyframes-group.svelte) but Svelte components must be PascalCase; rename the file to KeyframesGroup.svelte and update all imports/usages to the new name (search for imports referencing "keyframes-group" and replace with "KeyframesGroup"), ensuring the internal exported component (LayerKeyframes usage) remains unchanged; also apply the same PascalCase rename for the other new group components (style-group.svelte → StyleGroup.svelte, time-range-group.svelte → TimeRangeGroup.svelte, animation-presets-group.svelte → AnimationPresetsGroup.svelte, property-group-renderer.svelte → PropertyGroupRenderer.svelte) to keep naming consistent with existing components like TextLayer.svelte/ShapeLayer.svelte. In `@src/lib/components/editor/panels/properties/groups/layer-properties-group.svelte`: - Around line 8-15: The prop type intersection currently includes an unused optional isProjectSettings flag when destructuring props from $props() in the PropertyGroup (PropertyGroupProps & { isProjectSettings?: boolean }); remove the `{ isProjectSettings?: boolean }` from that intersection so the destructured variables are just typed as PropertyGroupProps, and keep the derived isProjectSettings value computed via $derived(isProjectLayer(layer.id)); update the destructuring line (the one calling $props()) to use only PropertyGroupProps to avoid the misleading unused prop. - Around line 28-38: The code inconsistently uses optional chaining on layerDefinition: change the PropertyGroupRenderer props to use the same style for both properties — either remove the optional chain on propertyGroups or add it to customPropertyComponents; since getLayerDefinition never returns undefined, update the JSX to access layerDefinition.propertyGroups and layerDefinition.customPropertyComponents consistently (e.g., use direct property access on both) so both references match and no unnecessary ?. remains. - Line 1: Rename the Svelte component file from kebab-case to PascalCase (layer-properties-group.svelte -> LayerPropertiesGroup.svelte) and update every import/usage to the new filename (e.g., import LayerPropertiesGroup from '.../LayerPropertiesGroup.svelte'); ensure any references in parent components, index exports, or route/component registries that used "layer-properties-group.svelte" are updated to "LayerPropertiesGroup.svelte" and that the Svelte component's internal name (if declared) matches the PascalCase convention. In `@src/lib/components/editor/panels/properties/groups/property-group-renderer.svelte`: - Around line 38-66: Replace the Svelte-specific reactive collection with a plain JS Set: in the propertyLayout derived computation, change the renderedGroupIds declaration from using SvelteSet to new Set<string>() and leave the rest of the logic (has/add checks) unchanged; also remove the now-unused SvelteSet import from the top of the file so there are no unused imports. In `@src/lib/components/editor/panels/properties/groups/style-group.svelte`: - Around line 9-11: The current code wraps a static computation in a reactive helper ($derived.by), which is unnecessary: replace the reactive call with a plain module-level constant so the metadata is computed once. Specifically, remove the $derived.by usage around extractPropertyMetadata(LayerStyleSchema) and assign its result directly to stylePropertyMetadata (referencing stylePropertyMetadata, $derived.by, LayerStyleSchema, and extractPropertyMetadata to locate the code). In `@src/lib/components/editor/panels/properties/groups/time-range-group.svelte`: - Around line 10-11: The $derived wrappers are redundant for stable context refs: replace const editorState = $derived(getEditorState()) with a direct reference const editorState = getEditorState(), and replace const projectStore = $derived(editorState.project) with const projectStore = editorState.project; this removes unnecessary indirection while still relying on ProjectStore's internal Svelte 5 reactivity (keep using editorState and projectStore.state/methods as before). In `@src/lib/components/editor/panels/properties/groups/transform-group.svelte`: - Around line 119-131: The Rotation ScrubXyz intentionally swaps X/Y axes (valueX/valueY use rotation.y/rotation.x and onchangeX/onchangeY write rotation.y/rotation.x) to match the 3D gizmo screen-space mapping; add a brief inline comment above the ScrubXyz block referencing this deliberate mapping and why (e.g., "screen-space X drag maps to model Y rotation — intentional swap"), and ensure the comment mentions the related symbols ScrubXyz and updateProperty so future readers understand the valueX/valueY <-> rotation.y/rotation.x mapping. In `@src/lib/layers/properties/field-registry.ts`: - Around line 1-4: The file imports Zod as a default; change to the named import syntax used elsewhere (import { z } from 'zod') and reorder imports so external packages come first (e.g., 'svelte' and 'zod'), then internal lib imports (e.g., '$lib/types/animation'), then relative imports (e.g., '../typed-registry'); update the import line referencing z (currently "import z from 'zod'") to "import { z } from 'zod'" and reorder the import statements accordingly to match the project's style. In `@src/lib/schemas/base.ts`: - Around line 220-237: The stylePropertyGroups array contains two groups with identical labels which will render as duplicate "Filters" headings; update the entries with id 'filters-1' and 'filters-2' in stylePropertyGroups to either merge them into a single group or give each a distinct label that reflects their purpose (e.g., "Filters — Blur/Brightness" and "Filters — Contrast/Saturate") so the UI headings are unambiguous; ensure any code that references these ids still works if you choose to merge or rename. - Around line 6-8: Reorder the imports so external packages come first: move the z import (import { z } from 'zod';) above the internal $lib imports (fieldRegistry and PropertyGroup). Ensure the resulting top-of-file import order is: external package import for z, then the internal imports of fieldRegistry and PropertyGroup to conform to the project import ordering rules.src/lib/components/editor/panels/properties/groups/layer-properties-group.svelte (3)
8-15: UnusedisProjectSettingsprop in the type intersection.The type on line 13 declares
{ isProjectSettings?: boolean }but the value is never read from props — it's always re-derived on line 15 fromisProjectLayer(layer.id). Remove it from the type intersection to avoid confusion.Proposed fix
const { layer, currentValues, updateProperty, addKeyframe - }: PropertyGroupProps & { isProjectSettings?: boolean } = $props(); + }: PropertyGroupProps = $props();🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and decide whether a code change is actually needed. If the finding is not valid or no change is required, do not modify code for that item and briefly explain why it was skipped. In `@src/lib/components/editor/panels/properties/groups/layer-properties-group.svelte` around lines 8 - 15, The prop type intersection currently includes an unused optional isProjectSettings flag when destructuring props from $props() in the PropertyGroup (PropertyGroupProps & { isProjectSettings?: boolean }); remove the `{ isProjectSettings?: boolean }` from that intersection so the destructured variables are just typed as PropertyGroupProps, and keep the derived isProjectSettings value computed via $derived(isProjectLayer(layer.id)); update the destructuring line (the one calling $props()) to use only PropertyGroupProps to avoid the misleading unused prop.
28-38: Inconsistent optional chaining:layerDefinition?.propertyGroupsvslayerDefinition.customPropertyComponents.Line 34 uses
?.but line 37 does not. SincegetLayerDefinitionthrows on unknown types (never returnsundefined), neither needs optional chaining. Pick one style and be consistent.🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and decide whether a code change is actually needed. If the finding is not valid or no change is required, do not modify code for that item and briefly explain why it was skipped. In `@src/lib/components/editor/panels/properties/groups/layer-properties-group.svelte` around lines 28 - 38, The code inconsistently uses optional chaining on layerDefinition: change the PropertyGroupRenderer props to use the same style for both properties — either remove the optional chain on propertyGroups or add it to customPropertyComponents; since getLayerDefinition never returns undefined, update the JSX to access layerDefinition.propertyGroups and layerDefinition.customPropertyComponents consistently (e.g., use direct property access on both) so both references match and no unnecessary ?. remains.
1-1: Filename should be PascalCase per coding guidelines.This Svelte component uses kebab-case (
layer-properties-group.svelte), but the project convention requires PascalCase for Svelte components (e.g.,LayerPropertiesGroup.svelte). As per coding guidelines, "Use kebab-case for TypeScript/JavaScript filenames, PascalCase for Svelte components".🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and decide whether a code change is actually needed. If the finding is not valid or no change is required, do not modify code for that item and briefly explain why it was skipped. In `@src/lib/components/editor/panels/properties/groups/layer-properties-group.svelte` at line 1, Rename the Svelte component file from kebab-case to PascalCase (layer-properties-group.svelte -> LayerPropertiesGroup.svelte) and update every import/usage to the new filename (e.g., import LayerPropertiesGroup from '.../LayerPropertiesGroup.svelte'); ensure any references in parent components, index exports, or route/component registries that used "layer-properties-group.svelte" are updated to "LayerPropertiesGroup.svelte" and that the Svelte component's internal name (if declared) matches the PascalCase convention.src/lib/components/editor/panels/properties/groups/keyframes-group.svelte (1)
1-8: Svelte component filenames should use PascalCase per coding guidelines.This file and the other new group components (
style-group.svelte,time-range-group.svelte,animation-presets-group.svelte,property-group-renderer.svelte) all use kebab-case. The coding guidelines require PascalCase for Svelte component filenames (e.g.,KeyframesGroup.svelte).If this is an intentional convention for this subdirectory, consider documenting it; otherwise, rename to align with the existing pattern used by
TextLayer.svelte,ShapeLayer.svelte, etc.As per coding guidelines:
src/**/*: "Use kebab-case for TypeScript/JavaScript filenames, PascalCase for Svelte components"🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and decide whether a code change is actually needed. If the finding is not valid or no change is required, do not modify code for that item and briefly explain why it was skipped. In `@src/lib/components/editor/panels/properties/groups/keyframes-group.svelte` around lines 1 - 8, The component filename uses kebab-case (keyframes-group.svelte) but Svelte components must be PascalCase; rename the file to KeyframesGroup.svelte and update all imports/usages to the new name (search for imports referencing "keyframes-group" and replace with "KeyframesGroup"), ensuring the internal exported component (LayerKeyframes usage) remains unchanged; also apply the same PascalCase rename for the other new group components (style-group.svelte → StyleGroup.svelte, time-range-group.svelte → TimeRangeGroup.svelte, animation-presets-group.svelte → AnimationPresetsGroup.svelte, property-group-renderer.svelte → PropertyGroupRenderer.svelte) to keep naming consistent with existing components like TextLayer.svelte/ShapeLayer.svelte.src/lib/components/editor/panels/properties/groups/style-group.svelte (1)
9-11:$derived.byis unnecessary for a static schema.
LayerStyleSchemais a module-level constant that never changes, soextractPropertyMetadata(LayerStyleSchema)will always return the same result. A plainconst(or a top-level module variable) would avoid the reactive overhead.- const stylePropertyMetadata = $derived.by(() => { - return extractPropertyMetadata(LayerStyleSchema); - }); + const stylePropertyMetadata = extractPropertyMetadata(LayerStyleSchema);🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and decide whether a code change is actually needed. If the finding is not valid or no change is required, do not modify code for that item and briefly explain why it was skipped. In `@src/lib/components/editor/panels/properties/groups/style-group.svelte` around lines 9 - 11, The current code wraps a static computation in a reactive helper ($derived.by), which is unnecessary: replace the reactive call with a plain module-level constant so the metadata is computed once. Specifically, remove the $derived.by usage around extractPropertyMetadata(LayerStyleSchema) and assign its result directly to stylePropertyMetadata (referencing stylePropertyMetadata, $derived.by, LayerStyleSchema, and extractPropertyMetadata to locate the code).src/lib/components/editor/panels/properties/groups/time-range-group.svelte (1)
10-11: Remove redundant$derivedwrappers for stable context references.
getEditorState()andeditorState.projectreturn stable references that never change during the component lifecycle. The$derivedwrapper adds unnecessary indirection without providing reactive value tracking benefits. SinceProjectStoremanages its own internal reactivity through Svelte 5 runes, the component will remain reactive when accessingprojectStore.stateand calling methods.Suggested change
- const editorState = $derived(getEditorState()); - const projectStore = $derived(editorState.project); + const editorState = getEditorState(); + const projectStore = editorState.project;🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and decide whether a code change is actually needed. If the finding is not valid or no change is required, do not modify code for that item and briefly explain why it was skipped. In `@src/lib/components/editor/panels/properties/groups/time-range-group.svelte` around lines 10 - 11, The $derived wrappers are redundant for stable context refs: replace const editorState = $derived(getEditorState()) with a direct reference const editorState = getEditorState(), and replace const projectStore = $derived(editorState.project) with const projectStore = editorState.project; this removes unnecessary indirection while still relying on ProjectStore's internal Svelte 5 reactivity (keep using editorState and projectStore.state/methods as before).src/lib/components/editor/panels/properties-panel.svelte (3)
116-127: Stale comment on line 126.The comment
// style: 'opacity'is outdated — style properties now includeblur,brightness,contrast,saturate,dropShadowX, etc.Suggested fix
- return propertyName as AnimatableProperty; // style: 'opacity' + return propertyName as AnimatableProperty; // style properties (opacity, blur, brightness, etc.)🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and decide whether a code change is actually needed. If the finding is not valid or no change is required, do not modify code for that item and briefly explain why it was skipped. In `@src/lib/components/editor/panels/properties-panel.svelte` around lines 116 - 127, The inline comment in mapToAnimatable is outdated—update or replace the comment on the return for the 'style' branch in function mapToAnimatable to reflect the current style properties (e.g. blur, brightness, contrast, saturate, dropShadowX, etc.) or remove it entirely; locate the mapToAnimatable function and modify the trailing comment that currently says "// style: 'opacity'" so it accurately lists or generically describes supported style keys.
245-263:propertyValueMapduplicates what could be derived fromcurrentValues.style.This map manually lists every style key. If you spread
currentValues.styleinto it, new style properties are picked up automatically:Suggested approach
const propertyValueMap: Record<string, number | string> = { 'position.x': currentValues.transform.position.x, 'position.y': currentValues.transform.position.y, 'position.z': currentValues.transform.position.z, 'rotation.x': currentValues.transform.rotation.x, 'rotation.y': currentValues.transform.rotation.y, 'rotation.z': currentValues.transform.rotation.z, 'scale.x': currentValues.transform.scale.x, 'scale.y': currentValues.transform.scale.y, - opacity: currentValues.style.opacity, - blur: currentValues.style.blur, - brightness: currentValues.style.brightness, - contrast: currentValues.style.contrast, - saturate: currentValues.style.saturate, - dropShadowX: currentValues.style.dropShadowX, - dropShadowY: currentValues.style.dropShadowY, - dropShadowBlur: currentValues.style.dropShadowBlur, - dropShadowColor: currentValues.style.dropShadowColor + ...currentValues.style };🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and decide whether a code change is actually needed. If the finding is not valid or no change is required, do not modify code for that item and briefly explain why it was skipped. In `@src/lib/components/editor/panels/properties-panel.svelte` around lines 245 - 263, The propertyValueMap duplicates individual style properties instead of reusing currentValues.style; update the construction of propertyValueMap to spread currentValues.style into it (e.g., merge ...currentValues.style) and keep only the explicit transform entries ('position.x','position.y','position.z','rotation.x','rotation.y','rotation.z','scale.x','scale.y') so new style keys are picked up automatically; locate and modify the propertyValueMap initialization in properties-panel.svelte where propertyValueMap and currentValues.style are referenced.
82-93:??fallback chains for style properties are correct but fragile to maintain.For numeric properties like
blurandbrightness,??(nullish coalescing) is the right choice since0is a valid value. However, every time a new style property is added toLayerStyleSchema, this block,defaultLayerStyle(), and thepropertyValueMap(lines 245-263) must all be updated in sync. Consider deriving the merged style programmatically from the schema defaults to avoid manual bookkeeping:// Example: merge animated values over layer values, with schema defaults as final fallback const mergedStyle = Object.fromEntries( Object.keys(defaultLayerStyle()).map(key => [ key, animatedStyle[key] ?? selectedLayer.style[key] ?? defaultLayerStyle()[key] ]) ) as LayerStyle;🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and decide whether a code change is actually needed. If the finding is not valid or no change is required, do not modify code for that item and briefly explain why it was skipped. In `@src/lib/components/editor/panels/properties-panel.svelte` around lines 82 - 93, The inline manual fallback object for style in properties-panel.svelte is fragile; replace it with a programmatic merge that iterates the schema/defaults (use defaultLayerStyle()) and builds each key as animatedStyle[key] ?? selectedLayer.style[key] ?? defaultLayerStyle()[key], casting the result to LayerStyle so all style keys (and future additions) are handled automatically; update the style assignment in the component and remove redundant manual entries that must be kept in sync with defaultLayerStyle() and propertyValueMap.src/lib/schemas/base.ts (2)
220-237: Two groups share the label "Filters" — consider differentiating or merging.
filters-1andfilters-2both havelabel: 'Filters'. In the UI, users will see two sections with the same heading ("Filters" / "Filters"), which may be confusing. If the split is intentional for layout (e.g., to get two columns), consider distinguishing them (e.g., "Filters (Blur / Brightness)" vs. "Filters (Contrast / Saturate)") or merging into one group.🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and decide whether a code change is actually needed. If the finding is not valid or no change is required, do not modify code for that item and briefly explain why it was skipped. In `@src/lib/schemas/base.ts` around lines 220 - 237, The stylePropertyGroups array contains two groups with identical labels which will render as duplicate "Filters" headings; update the entries with id 'filters-1' and 'filters-2' in stylePropertyGroups to either merge them into a single group or give each a distinct label that reflects their purpose (e.g., "Filters — Blur/Brightness" and "Filters — Contrast/Saturate") so the UI headings are unambiguous; ensure any code that references these ids still works if you choose to merge or rename.
6-8: Import order: external packages should precede internal lib imports.As per coding guidelines, imports should be organized as: External packages → SvelteKit → Internal lib → Relative imports. The
zodimport should come before the$lib/imports.Suggested fix
-import { fieldRegistry } from '$lib/layers/properties/field-registry'; -import type { PropertyGroup } from '$lib/layers/registry'; import { z } from 'zod'; +import { fieldRegistry } from '$lib/layers/properties/field-registry'; +import type { PropertyGroup } from '$lib/layers/registry';🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and decide whether a code change is actually needed. If the finding is not valid or no change is required, do not modify code for that item and briefly explain why it was skipped. In `@src/lib/schemas/base.ts` around lines 6 - 8, Reorder the imports so external packages come first: move the z import (import { z } from 'zod';) above the internal $lib imports (fieldRegistry and PropertyGroup). Ensure the resulting top-of-file import order is: external package import for z, then the internal imports of fieldRegistry and PropertyGroup to conform to the project import ordering rules.src/lib/components/editor/panels/properties/groups/property-group-renderer.svelte (1)
38-66:SvelteSetis unnecessary here — a plainSetsuffices.The
renderedGroupIdsset is local to this$derived.bycomputation and is never observed reactively by anything outside it. UsingSvelteSet(which adds reactive tracking overhead) provides no benefit over a plainSet.Suggested fix
- const renderedGroupIds = new SvelteSet<string>(); + const renderedGroupIds = new Set<string>();And remove the unused import on line 6:
- import { SvelteSet } from 'svelte/reactivity';🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and decide whether a code change is actually needed. If the finding is not valid or no change is required, do not modify code for that item and briefly explain why it was skipped. In `@src/lib/components/editor/panels/properties/groups/property-group-renderer.svelte` around lines 38 - 66, Replace the Svelte-specific reactive collection with a plain JS Set: in the propertyLayout derived computation, change the renderedGroupIds declaration from using SvelteSet to new Set<string>() and leave the rest of the logic (has/add checks) unchanged; also remove the now-unused SvelteSet import from the top of the file so there are no unused imports.src/lib/layers/properties/field-registry.ts (1)
1-4: Inconsistent Zod import style: default import here vs. named import inbase.ts.This file uses
import z from 'zod'(default import) whilesrc/lib/schemas/base.tsusesimport { z } from 'zod'(named import). Zod 4 supports both, but pick one style for consistency across the codebase.Also, import order should be: external packages (
svelte,zod) → internal lib → relative.Suggested fix
-import type { InterpolationFamily } from '$lib/types/animation'; import type { Component } from 'svelte'; +import { z } from 'zod'; +import type { InterpolationFamily } from '$lib/types/animation'; import type { TypedLayer } from '../typed-registry'; -import z from 'zod';🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and decide whether a code change is actually needed. If the finding is not valid or no change is required, do not modify code for that item and briefly explain why it was skipped. In `@src/lib/layers/properties/field-registry.ts` around lines 1 - 4, The file imports Zod as a default; change to the named import syntax used elsewhere (import { z } from 'zod') and reorder imports so external packages come first (e.g., 'svelte' and 'zod'), then internal lib imports (e.g., '$lib/types/animation'), then relative imports (e.g., '../typed-registry'); update the import line referencing z (currently "import z from 'zod'") to "import { z } from 'zod'" and reorder the import statements accordingly to match the project's style.src/lib/components/editor/panels/properties/groups/transform-group.svelte (1)
119-131: Rotation axes are intentionally swapped in the 3D gizmo — consider adding a comment.
valueXreadsrotation.yandonchangeXwritesrotation.y(and vice versa). This is presumably deliberate (screen-space X-drag maps to Y-axis rotation), but it's easy to misread as a bug. A brief inline comment would help future maintainers.🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and decide whether a code change is actually needed. If the finding is not valid or no change is required, do not modify code for that item and briefly explain why it was skipped. In `@src/lib/components/editor/panels/properties/groups/transform-group.svelte` around lines 119 - 131, The Rotation ScrubXyz intentionally swaps X/Y axes (valueX/valueY use rotation.y/rotation.x and onchangeX/onchangeY write rotation.y/rotation.x) to match the 3D gizmo screen-space mapping; add a brief inline comment above the ScrubXyz block referencing this deliberate mapping and why (e.g., "screen-space X drag maps to model Y rotation — intentional swap"), and ensure the comment mentions the related symbols ScrubXyz and updateProperty so future readers understand the valueX/valueY <-> rotation.y/rotation.x mapping.
| defaultOpen = false | ||
| }: { label: string | Snippet; children: Snippet; defaultOpen?: boolean } = $props(); | ||
|
|
||
| let isOpen = $derived(defaultOpen); |
There was a problem hiding this comment.
$derived will override user toggle when defaultOpen prop changes — use $state instead.
$derived(defaultOpen) re-derives whenever the prop changes, which resets the user's toggle state. Even though $derived is assignable in Svelte 5.25+, the derivation still wins when its dependency updates. The intent here is "initialize from prop, then manage locally," which is $state:
- let isOpen = $derived(defaultOpen);
+ let isOpen = $state(defaultOpen);📝 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 isOpen = $derived(defaultOpen); | |
| let isOpen = $state(defaultOpen); |
🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and
decide whether a code change is actually needed. If the finding is not valid or
no change is required, do not modify code for that item and briefly explain why
it was skipped.
In `@src/lib/components/editor/panels/properties-group.svelte` at line 12, The
isOpen store is currently created with $derived(defaultOpen) which re-syncs
whenever the defaultOpen prop changes and thus overrides user toggles; change
this to use $state(defaultOpen) so isOpen is initialized from defaultOpen once
and thereafter managed locally by the component (replace the $derived(...)
instantiation with $state(...) and keep all existing reads/writes to isOpen
intact).
Comment on lines
+21
to
+26
| max={layer.contentDuration !== undefined | ||
| ? Math.min( | ||
| projectStore.state.duration, | ||
| projectStore.state.duration - (layer.contentDuration - (layer.contentOffset ?? 0)) | ||
| ) | ||
| : projectStore.state.duration} |
There was a problem hiding this comment.
Enter-time max can go negative when content is shorter than expected.
If contentDuration - contentOffset exceeds projectStore.state.duration, the computed max becomes negative while min is 0, creating an impossible range for ScrubInput. Consider clamping to at least 0 (or the current enterTime):
max={layer.contentDuration !== undefined
? Math.min(
projectStore.state.duration,
- projectStore.state.duration - (layer.contentDuration - (layer.contentOffset ?? 0))
+ Math.max(0, projectStore.state.duration - (layer.contentDuration - (layer.contentOffset ?? 0)))
)
: projectStore.state.duration}📝 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
| max={layer.contentDuration !== undefined | |
| ? Math.min( | |
| projectStore.state.duration, | |
| projectStore.state.duration - (layer.contentDuration - (layer.contentOffset ?? 0)) | |
| ) | |
| : projectStore.state.duration} | |
| max={layer.contentDuration !== undefined | |
| ? Math.min( | |
| projectStore.state.duration, | |
| Math.max(0, projectStore.state.duration - (layer.contentDuration - (layer.contentOffset ?? 0))) | |
| ) | |
| : projectStore.state.duration} |
🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and
decide whether a code change is actually needed. If the finding is not valid or
no change is required, do not modify code for that item and briefly explain why
it was skipped.
In `@src/lib/components/editor/panels/properties/groups/time-range-group.svelte`
around lines 21 - 26, The computed max for ScrubInput can go negative when
layer.contentDuration - (layer.contentOffset ?? 0) exceeds
projectStore.state.duration; update the max calculation (the expression using
layer.contentDuration, layer.contentOffset and projectStore.state.duration) to
clamp the result with Math.max to ensure it is at least 0 (or at least the
current enterTime value) so min (0) ≤ max; locate the max prop usage in the
time-range-group Svelte component and replace the existing ternary result with a
clamped value (e.g., Math.max(0, ... ) or Math.max(enterTime, ... )) so the
ScrubInput always receives a non-negative, valid range.
Comment on lines
+183
to
+192
| onclick={() => { | ||
| const newLocked = !scaleLocked; | ||
| const ratio = currentScaleX / currentScaleY; | ||
| const newProps = { | ||
| ...layer.props, | ||
| _scaleLocked: newLocked, | ||
| _scaleRatio: ratio | ||
| }; | ||
| projectStore.updateLayer(layer.id, { props: newProps }); | ||
| }} |
There was a problem hiding this comment.
Division by zero when currentScaleY is 0.
const ratio = currentScaleX / currentScaleY will produce Infinity or NaN when currentScaleY is 0. The scale schema allows min(0), so 0 is a valid value (e.g., from a keyframe animation). Guard against this:
Suggested fix
const newLocked = !scaleLocked;
- const ratio = currentScaleX / currentScaleY;
+ const ratio = currentScaleY !== 0 ? currentScaleX / currentScaleY : 1;📝 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
| onclick={() => { | |
| const newLocked = !scaleLocked; | |
| const ratio = currentScaleX / currentScaleY; | |
| const newProps = { | |
| ...layer.props, | |
| _scaleLocked: newLocked, | |
| _scaleRatio: ratio | |
| }; | |
| projectStore.updateLayer(layer.id, { props: newProps }); | |
| }} | |
| onclick={() => { | |
| const newLocked = !scaleLocked; | |
| const ratio = currentScaleY !== 0 ? currentScaleX / currentScaleY : 1; | |
| const newProps = { | |
| ...layer.props, | |
| _scaleLocked: newLocked, | |
| _scaleRatio: ratio | |
| }; | |
| projectStore.updateLayer(layer.id, { props: newProps }); | |
| }} |
🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and
decide whether a code change is actually needed. If the finding is not valid or
no change is required, do not modify code for that item and briefly explain why
it was skipped.
In `@src/lib/components/editor/panels/properties/groups/transform-group.svelte`
around lines 183 - 192, The click handler for toggling scale lock computes ratio
via const ratio = currentScaleX / currentScaleY which can produce Infinity/NaN
when currentScaleY is 0; update the handler (the onclick block that sets
newProps and calls projectStore.updateLayer) to guard against division by zero
by computing ratio as: if currentScaleY === 0 then use a safe fallback (for
example ratio = currentScaleX === 0 ? 1 : currentScaleX) else ratio =
currentScaleX / currentScaleY, and then set _scaleRatio and _scaleLocked on
layer.props as before when calling projectStore.updateLayer.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
UI / Properties