Conversation
…list Replace the project settings modal with a virtual "project-settings" layer that sits fixed at the top of the layers panel. When selected, the properties panel shows project-level fields (dimensions, duration, background, font) using the same dynamic property system as regular layers, but without transform, style, time range, keyframe, or animation preset sections. Key changes: - New ProjectSettingsLayer.svelte component registered in the layer system - New project-layer.ts with constants, virtual layer synthesis, and helpers - ProjectStore auto-selects the project layer on init and loadProject - Layers panel renders project layer as a fixed, non-draggable, non-deletable item - Properties panel routes project layer property updates to updateProject() - Toolbar and project-settings-dialog modal references removed https://claude.ai/code/session_01REigfXtKvZKtz2JF8sF5HL
Clarify that the empty state refers to animation layers, not the project settings layer which is always visible. Add a visual separator and update the message to be more specific and helpful. Changes: - Message: "No layers yet" → "No animation layers yet" - Action: "Add layers from the toolbar" → "Click the + button above to add your first layer" - Added border-top separator to visually distinguish from project settings layer https://claude.ai/code/session_01REigfXtKvZKtz2JF8sF5HL
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds a virtual "Project Settings" layer accessible in the layers panel and properties panel, backed by a new project-layer type and schema. Project properties (size, duration, background, default font, name) are edited via the properties panel and routed to the project store; the old toolbar dialog is removed. Changes
Sequence DiagramsequenceDiagram
participant User
participant LayersPanel as Layers Panel
participant ProjectStore as Project Store
participant PropertiesPanel as Properties Panel
participant ProjectLayer as Virtual Project Layer
User->>LayersPanel: Open editor / view layers
LayersPanel->>ProjectStore: request layers
ProjectStore->>ProjectLayer: createVirtualProjectLayer(state)
ProjectLayer-->>LayersPanel: synthesized virtual project layer
LayersPanel->>User: render Project Settings entry at top
User->>LayersPanel: select Project Settings
LayersPanel->>ProjectStore: select PROJECT_LAYER_ID
ProjectStore-->>PropertiesPanel: provide virtual project layer as selectedLayer
PropertiesPanel->>User: render project-centric UI (Project Name, Resolution, Duration, Background, Font)
User->>PropertiesPanel: change width/duration/background/font
PropertiesPanel->>PropertiesPanel: mapProjectLayerPropsToProject(props)
PropertiesPanel->>ProjectStore: updateProject(partialProject)
ProjectStore->>ProjectStore: apply project state update
ProjectStore-->>User: project state updated
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/layers/typed-registry.ts (1)
21-41:⚠️ Potential issue | 🟠 MajorFilter
project-settingsfrom AI layer creation tools to prevent real layer instantiation of this virtual-only type.Adding
'project-settings'to theLayerPropsMapintyped-registry.tsis correct, but it now gets auto-registered and exposed as a creatable layer to the AI. However,project-settingsis designed as a virtual layer synthesized from top-level project fields—it should never be instantiated as a real layer inproject.layers[].The issue:
generateLayerCreationTools()insrc/lib/ai/schemas.tslacks a guard. While'group'is explicitly filtered out (line 144),'project-settings'is not. This will create acreate_project_settings_layertool available to the AI, bypassing the UI-level hiding inadd-layer.svelte.Add the check after line 144:
if (layerType === 'project-settings') continue;This follows the same pattern already in place for
'group'and ensures the AI respects the architectural constraint thatproject-settingsexists only as a virtual layer.
🧹 Nitpick comments (3)
src/lib/stores/project.svelte.ts (1)
143-146: Consider falling back toPROJECT_LAYER_IDinstead ofnullwhen a layer is removed.After removing a selected layer,
selectedLayerIdis set tonull, which shows the "No layer selected" empty state. Since the project settings layer is always available, falling back to it would provide a better UX — the user always sees something useful in the properties panel.The same applies to
ungroupLayers(Line 267) and other places that reset tonull.Proposed fix
this.state.layers = this.state.layers.filter((l) => l.id !== layerId); if (this.selectedLayerId === layerId) { - this.selectedLayerId = null; + this.selectedLayerId = PROJECT_LAYER_ID; }src/lib/layers/project-layer.ts (1)
60-69: Type assertions bypass Zod validation.The
as number/as Project['background']casts trust that values are already valid. Since these props flow from the schema-backed UI inputs, this is likely safe in practice. However, per coding guidelines,src/lib/**/*.tsfiles should validate external data with Zod schemas. Consider using the schema fromProjectSettingsLayer.svelteto parse/validate before mapping, especially if this function could be called from other contexts (e.g., AI mutations).As per coding guidelines, "Validate all external data with Zod schemas before using in the application".
src/lib/components/editor/panels/properties-panel.svelte (1)
609-619: X/Y swap in Rotation ScrubXyz appears intentional but could confuse maintainers.
valueXis bound torotationYandvalueYtorotationX, withonchangeXupdatingrotationYand vice versa. This X↔Y swap is presumably for the 3D scrub control's visual axis mapping, but it's not new to this PR — just flagging it as pre-existing and worth a brief comment.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/components/editor/export-dialog.svelte (1)
416-424:⚠️ Potential issue | 🟡 MinorHeight input is editable but changes are silently discarded.
The binding was changed from
bind:valueto a one-wayvalue, yet unlike the FPS (line 401) and Width (line 412) inputs, this one is missing thereadonlyattribute. The user can type into the field, but the value won't propagate back toexportSettings, making it a no-op that misleads the user.Add
readonlyto match the other inputs, or restorebind:valueif user editing is intended.Proposed fix
<Input id="export-height" type="number" value={exportSettings.height} class="col-span-3" + readonly />
🧹 Nitpick comments (2)
src/lib/components/editor/panels/properties-panel.svelte (2)
295-300: Consider guarding all targets for the project settings layer, not justprops.Currently only
target === 'props'is intercepted for the project settings layer. IfupdatePropertyis ever called withtarget === 'transform'or'style'for the virtual layer (e.g., from a custom property component), it would fall through toprojectStore.updateLayerwith the virtual layer ID, which could produce unexpected behavior.The UI currently prevents this, but a defensive early return would be safer:
Suggested improvement
// For the project settings layer, route props directly to updateProject - if (isProjectSettings && target === 'props') { + if (isProjectSettings) { + if (target !== 'props') return; // transform/style not applicable const projectUpdates = mapProjectLayerPropsToProject({ [propertyName]: value }); projectStore.updateProject(projectUpdates); return; }
807-813:addKeyframeis passed unconditionally to custom property components.For the project settings layer, keyframes are not applicable. If a custom property component for this layer type ever calls
addKeyframe, it would attempt to manipulate keyframes on the virtual layer. Consider conditionally passing it like the grouped/ungrouped fields above:<CustomPropertyComponent layer={selectedLayer} onUpdateProp={(name, v) => updateProperty(name, v, 'props')} - {addKeyframe} + addKeyframe={isProjectSettings ? undefined : addKeyframe} />
2 participants
Summary by CodeRabbit
New Features
Changes