Promote project settings to a special layer at the top of the layers list#22
Promote project settings to a special layer at the top of the layers list#22epavanello wants to merge 4 commits intomainfrom
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
|
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 type to the editor UI, exposing project properties (width, height, duration, background, fontFamily) as a non-draggable layer; routes edits to the project store, removes the toolbar project-settings dialog, and hides project-settings from the add-layer menu. Changes
Sequence DiagramsequenceDiagram
participant User
participant LayersPanel as Layers Panel
participant ProjectStore
participant PropertiesPanel as Properties Panel
User->>LayersPanel: click/select project settings entry
LayersPanel->>ProjectStore: set selectedLayerId = "__project__"
ProjectStore->>ProjectStore: selectedLayer getter -> createVirtualProjectLayer(project)
ProjectStore-->>PropertiesPanel: reactively provide virtual project layer
PropertiesPanel->>User: render "Project Properties" UI
User->>PropertiesPanel: edit property (e.g., width)
PropertiesPanel->>PropertiesPanel: mapProjectLayerPropsToProject(props)
PropertiesPanel->>ProjectStore: updateProject(partialProject)
ProjectStore->>ProjectStore: apply project update
ProjectStore-->>LayersPanel: reactive update (virtual layer reflects new props)
LayersPanel->>User: re-render project layer display
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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/components/editor/panels/layers-panel.svelte (1)
487-492:⚠️ Potential issue | 🟡 Minor"No layers yet" empty state still shows when the project settings layer is visible above.
The empty state message appears when
layers.length === 0, but the virtual project settings layer is always visible at the top. Users might find it slightly confusing to see "No layers yet — Add layers from the toolbar" directly below the project settings entry. Consider adjusting the message or adding a subtle visual separator.
🧹 Nitpick comments (4)
src/lib/layers/project-layer.ts (2)
27-55: Consider returningTypedLayer<'project-settings'>for stronger typing.The return type
TypedLayer(unparameterized) resolves toLayer, losing the type-level association with'project-settings'. Using the parameterized form would give consumers typedprops.♻️ Proposed fix
-export function createVirtualProjectLayer(project: Project): TypedLayer { +export function createVirtualProjectLayer(project: Project): TypedLayer<'project-settings'> {
60-69: Use Zod schema validation instead of unsafe type casting.
mapProjectLayerPropsToProjectcastsunknownvalues without validation (e.g.,props.width as number). TheProjectSchemaalready exists insrc/lib/schemas/animation.tswith proper validation rules (.positive()for numeric fields, enum forfontFamily). Validate the props object againstProjectSchema.partial()before returning updates, per the coding guideline: "Validate all external data with Zod schemas before using in the application."src/lib/stores/project.svelte.ts (1)
143-146: After removing the selected layer, selection falls back tonullinstead of the project layer.
removeLayerandungroupLayers(line 267) resetselectedLayerIdtonullwhen the deleted/dissolved layer was selected. With the new paradigm where the project layer is always available and auto-selected on init/load, falling back toPROJECT_LAYER_IDwould be more consistent and avoid showing the "No layer selected" empty state.♻️ Proposed fix (removeLayer)
if (this.selectedLayerId === layerId) { - this.selectedLayerId = null; + this.selectedLayerId = PROJECT_LAYER_ID; }Apply the same change in
ungroupLayersat line 267.src/lib/components/editor/panels/properties-panel.svelte (1)
309-314: Guard only coverstarget === 'props'— non-props updates for the project layer would silently fail.If
updatePropertyis called withtarget === 'transform'or'style'while the project layer is selected, it falls through toprojectStore.updateLayer(PROJECT_LAYER_ID, ...), which won't find a matching layer and silently does nothing. Currently safe because the Transform/Style UI is hidden, but a broader early return would be more defensive.🛡️ Proposed defensive guard
if (!selectedLayer) return; // For the project settings layer, route props directly to updateProject - if (isProjectSettings && target === 'props') { + if (isProjectSettings) { + if (target !== 'props') return; // Only props are editable on the project layer const projectUpdates = mapProjectLayerPropsToProject({ [propertyName]: value }); projectStore.updateProject(projectUpdates); return; }
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
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/lib/components/editor/panels/layers-panel.svelte`:
- Around line 204-222: The project-settings row (ProjectIcon / selectLayer /
PROJECT_LAYER_ID block) lacks the intended visual separator when animation
layers exist; add the border-top separator so it always appears by applying the
CSS border class (e.g., "border-t" or equivalent utility) to the correct
element—either add the border class to the project-settings div that contains
ProjectIcon and projectStore.state.name or insert a small separator element
immediately after that div and before the {`#each`} animation layers list—so the
separator is consistently shown regardless of the empty-state branch.
| <!-- Project Settings Layer (always fixed at top, non-draggable, non-deletable) --> | ||
| {@const ProjectIcon = getLayerDefinition(PROJECT_LAYER_TYPE).icon} | ||
| <div | ||
| class={cn( | ||
| 'flex cursor-pointer items-center gap-2 rounded-md px-3 py-2 transition-colors hover:bg-muted/50', | ||
| { | ||
| 'bg-muted': isProjectLayer(projectStore.selectedLayerId) | ||
| } | ||
| )} | ||
| onclick={() => selectLayer(PROJECT_LAYER_ID)} | ||
| onkeydown={(e) => handleKeyDown(e, PROJECT_LAYER_ID)} | ||
| role="button" | ||
| tabindex="0" | ||
| > | ||
| <ProjectIcon class="size-4 shrink-0" /> | ||
| <div class="flex-1 truncate text-sm font-medium"> | ||
| {projectStore.state.name} | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Visual separator between project layer and animation layers may be missing.
The commit message mentions adding "a border-top separator to visually distinguish the project settings layer from animation layers," but the border-t is only applied inside the empty-state block (Line 488). When animation layers do exist, there's no visual separator between this project-settings row and the first animation layer.
If the separator is intended to always be present, consider adding it around the {#each} block or after this project-settings div:
Proposed fix
</div>
+
+ <!-- Separator between project settings and animation layers -->
+ <div class="border-t"></div>
{`#each` layerTree as { layer, children, index } (layer.id)}🤖 Prompt for AI Agents
In `@src/lib/components/editor/panels/layers-panel.svelte` around lines 204 - 222,
The project-settings row (ProjectIcon / selectLayer / PROJECT_LAYER_ID block)
lacks the intended visual separator when animation layers exist; add the
border-top separator so it always appears by applying the CSS border class
(e.g., "border-t" or equivalent utility) to the correct element—either add the
border class to the project-settings div that contains ProjectIcon and
projectStore.state.name or insert a small separator element immediately after
that div and before the {`#each`} animation layers list—so the separator is
consistently shown regardless of the empty-state branch.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/lib/components/editor/panels/properties-panel.svelte (2)
306-311: Guard is only fortarget === 'props'— confirm transform/style paths are unreachable.The early return only triggers when
target === 'props'. If a code path somehow callsupdateProperty(...)withtarget === 'transform'or'style'while the project layer is selected, it would fall through toupdateAnimatableValueand attempt keyframe/layer mutation on the virtual layer. This is currently safe because the UI hides those sections (line 432), but the guard would be more robust if it also covered those targets.Suggested defensive guard
// 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; }
811-817: Custom property components still rendered and receiveaddKeyframefor the project settings layer.
layerCustomPropertyComponentsis rendered unconditionally. If the project settings layer definition ever definescustomPropertyComponents, they would receive theaddKeyframecallback which would attempt to mutate the virtual layer's keyframes. Consider gating this block or at least passingundefinedforaddKeyframewhenisProjectSettingsis true, consistent with how you handle it for the schema-driven properties above (lines 764, 800).Suggested fix
{`#each` layerCustomPropertyComponents as [name, { component: CustomPropertyComponent }] (name)} <CustomPropertyComponent layer={selectedLayer} onUpdateProp={(name, v) => updateProperty(name, v, 'props')} - {addKeyframe} + addKeyframe={isProjectSettings ? undefined : addKeyframe} /> {/each}
2 participants
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:
https://claude.ai/code/session_01REigfXtKvZKtz2JF8sF5HL
Summary by CodeRabbit
New Features
Refactor