enhancements: update roadmap to the current project state. Increment project names automatically for less confusion #579
Conversation
…dio elements both effective on timeline and preview panel, with overlay icon for indicating so and state storing,
… was occurring because of a race condition during cleanup when the component unmounts.
… storage - Added bookmark management methods in the project store for toggling and checking bookmarks. - Updated the timeline component to display bookmark markers and integrate bookmark actions in the context menu. - Enhanced the storage service to handle bookmarks in project serialization and deserialization. - Updated project types to include bookmarks as an array of numbers.
|
@genmnz is attempting to deploy a commit to the OpenCut OSS Team on Vercel. A member of the Team first needs to authorize it. |
👷 Deploy request for appcut pending review.Visit the deploys page to approve it
|
WalkthroughAdds project bookmarks (store API, persistence, timeline UI), introduces per-element hidden state with UI and store support, filters hidden elements in preview, and refactors audio waveform lifecycle to safely destroy/recreate WaveSurfer instances. Updates types and storage schemas to include bookmarks, fps, and element hidden flags. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant TL as Timeline Toolbar
participant PS as ProjectStore
participant SS as StorageService
participant UI as UI (Timeline/Markers)
U->>TL: Click Bookmark button
TL->>PS: toggleBookmark(currentTime)
PS->>PS: Round to nearest frame (fps)
alt Not bookmarked
PS->>PS: Add frameTime to bookmarks (sorted)
else Already bookmarked
PS->>PS: Remove frameTime from bookmarks
end
PS->>SS: saveProject(project with bookmarks/fps)
SS-->>PS: OK
PS-->>UI: Update state and refresh list
UI-->>U: Marker/tooltip reflects new state
sequenceDiagram
autonumber
actor U as User
participant TE as TimelineElement
participant TS as TimelineStore
participant H as History
participant S as Storage
participant PP as PreviewPanel
U->>TE: Toggle hide/show (click or context menu)
TE->>TS: toggleElementHidden(trackId, elementId)
TS->>H: pushHistory()
TS->>TS: Toggle element.hidden
TS->>S: updateTracksAndSave()
S-->>TS: OK
TS-->>PP: State updated
PP->>PP: Filter out hidden elements
PP-->>U: Render without hidden element
sequenceDiagram
autonumber
participant C as AudioWaveform Component
participant WS as WaveSurfer (old)
participant WN as WaveSurfer (new)
C->>C: init()
opt Existing instance
C->>WS: capture & clear ref
C-->>WS: destroy (deferred via rAF)
end
C->>WN: create()
C->>WN: bind events (ready, error)
C->>WN: load(audioUrl)
alt Component still mounted
C->>C: assign ref to new instance
else Unmounted mid-init
C->>WN: destroy immediately
end
C->>C: cleanup on unmount (clear ref, deferred destroy)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
genmnz
changed the title
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
apps/web/src/app/projects/page.tsx
Outdated
| } | ||
|
|
||
| let newProjectNumber = 2; | ||
| while (savedProjects.some((p) => p.name === `Project ${newProjectNumber}`)) { |
There was a problem hiding this comment.
The project naming logic has an inconsistency bug that creates "Project 2" instead of "New Project 2" when handling name collisions.
View Details
📝 Patch Details
diff --git a/apps/web/src/app/projects/page.tsx b/apps/web/src/app/projects/page.tsx
index 32891d9..8ba1375 100644
--- a/apps/web/src/app/projects/page.tsx
+++ b/apps/web/src/app/projects/page.tsx
@@ -99,11 +99,11 @@ export default function ProjectsPage() {
}
let newProjectNumber = 2;
- while (savedProjects.some((p) => p.name === `Project ${newProjectNumber}`)) {
+ while (savedProjects.some((p) => p.name === `New Project ${newProjectNumber}`)) {
newProjectNumber++;
}
- const projectId = await createNewProject(`Project ${newProjectNumber}`);
+ const projectId = await createNewProject(`New Project ${newProjectNumber}`);
router.push(`/editor/${projectId}`);
};
Analysis
In the handleCreateProject function, when "New Project" already exists, the code attempts to find the next available number by checking for names like Project ${newProjectNumber} (line 102), but it should be checking for New Project ${newProjectNumber} to maintain naming consistency.
The current logic flow:
- If "New Project" doesn't exist → creates "New Project" ✅
- If "New Project" exists → checks for "Project 2", "Project 3", etc. ❌
- Then creates "Project 2" instead of "New Project 2" ❌
This breaks the expected naming pattern and could lead to confusing project names. Users would expect to see "New Project", "New Project 2", "New Project 3", etc., but instead get "New Project", "Project 2", "Project 3".
The fix is to change line 102 from:
while (savedProjects.some((p) => p.name === `Project ${newProjectNumber}`)) {to:
while (savedProjects.some((p) => p.name === `New Project ${newProjectNumber}`)) {And change line 106 from:
const projectId = await createNewProject(`Project ${newProjectNumber}`);to:
const projectId = await createNewProject(`New Project ${newProjectNumber}`);This matches the consistent pattern used in the editor page for "Untitled Project" naming.
genmnz
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/app/projects/page.tsx (1)
67-90: Stale closure in getProjectThumbnail causes redundant loads.useCallback has an empty dep array but reads thumbnailCache; cache lookups will be stale.
Two lightweight fixes (pick one):
- Add deps (simpler, may retrigger effects but returns cached fast):
- }, [] + }, [thumbnailCache]
- Or keep a stable function using a ref-backed cache:
// add near other state const thumbnailCacheRef = useRef<Record<string, string | null>>({}); useEffect(() => { thumbnailCacheRef.current = thumbnailCache }, [thumbnailCache]); const getProjectThumbnail = useCallback(async (projectId: string) => { const cached = thumbnailCacheRef.current[projectId]; if (cached !== undefined) return cached; // ... rest unchanged }, []);
🧹 Nitpick comments (6)
apps/web/src/app/roadmap/page.tsx (1)
144-144: Make the "last updated" timestamp single-sourced.Avoid manual edits by centralizing the date.
+const LAST_UPDATED = "August 30, 2025"; ... - What's coming next for OpenCut (last updated: August 30, 2025) + {`What's coming next for OpenCut (last updated: ${LAST_UPDATED})`}apps/web/src/stores/project-store.ts (1)
342-351: Duplicate-name generator is solid; consider unifying naming across flows and extracting a helper.Editor uses "Untitled Project N", Projects uses "Project N", while duplicates use "BaseName (n)". Centralizing to one helper prevents drift.
Example extraction (outside this method):
function nextName(base: string, existing: Set<string>) { let n = 1, name = base; while (existing.has(name)) { n += 1; name = `${base} (${n})`; } return name; }Then here:
-const baseName = project.name.replace(/ \(\d+\)$/, ""); -let newName = `${baseName} (1)`; -let counter = 1; -const existingNames = new Set(savedProjects.map((p) => p.name)); -while (existingNames.has(newName)) { - counter++; - newName = `${baseName} (${counter})`; -} +const baseName = project.name.replace(/ \(\d+\)$/, ""); +const existingNames = new Set(savedProjects.map((p) => p.name)); +const newName = nextName(baseName, existingNames);Also applies to: 356-356
apps/web/src/app/projects/page.tsx (2)
93-99: Unify default naming and remove console.log; precompute a Set for O(n) checks.Single path avoids double navigation and name drift ("New Project" vs "Project N").
- const projectExists = savedProjects.some((p) => p.name === "New Project"); - if (!projectExists) { - const projectId = await createNewProject("New Project"); - console.log("projectId", projectId); - router.push(`/editor/${projectId}`); - return; - } - - let newProjectNumber = 2; - while (savedProjects.some((p) => p.name === `Project ${newProjectNumber}`)) { - newProjectNumber++; - } - - const projectId = await createNewProject(`Project ${newProjectNumber}`); + const names = new Set(savedProjects.map((p) => p.name)); + const base = "New Project"; + let n = 1; + let newName = base; + while (names.has(newName)) { + n += 1; + newName = `${base} (${n})`; + } + const projectId = await createNewProject(newName); router.push(`/editor/${projectId}`);Also applies to: 101-107
335-351: Unstable keys for skeletons (index + Date.now()).Date.now() breaks key stability and forces unnecessary remounts; avoid index keys per guidelines.
- {Array.from({ length: 8 }, (_, index) => ( - <div - key={`skeleton-${index}-${Date.now()}`} + {Array.from({ length: 8 }, (_, i) => ( + <div + key={`skeleton-${i}`} className="overflow-hidden bg-background border-none p-0" >If you want to avoid index entirely, precompute a stable keys array with useMemo once.
apps/web/src/app/editor/[project_id]/page.tsx (2)
112-134: Not-found recovery may run before savedProjects is loaded, risking duplicate "Untitled Project".Load the list before checking, then compute from a Set. Optional: align suffix to the store’s “(n)” pattern.
- try { - const projectExists = savedProjects.some( - (p) => p.name === "Untitled Project" - ); - if (!projectExists) { - const newProjectId = await createNewProject("Untitled Project"); - router.replace(`/editor/${newProjectId}`); - return; - } - let newProjectNumber = 2; - while ( - savedProjects.some( - (p) => p.name === `Untitled Project ${newProjectNumber}` - ) - ) { - newProjectNumber++; - } - const newProjectId = await createNewProject( - `Untitled Project ${newProjectNumber}` - ); + try { + // Ensure we have a fresh list before computing the name + const { loadAllProjects } = useProjectStore.getState(); + await loadAllProjects(); + const names = new Set(useProjectStore.getState().savedProjects.map(p => p.name)); + const base = "Untitled Project"; + let n = 1; + let next = base; + while (names.has(next)) { + n += 1; + next = `${base} (${n})`; + } + const newProjectId = await createNewProject(next); + // Optional: clear initializing flag before early return + isInitializingRef.current = false; // Check again if component was unmountedWould you like me to propagate the same naming helper to Projects page for consistency?
170-173: Effect deps: consider including activeProject?.id to make the early-return guard reliable.Prevents reading a stale activeProject in long-lived sessions.
- }, [ + }, [ projectId, loadProject, createNewProject, router, savedProjects, isInvalidProjectId, markProjectIdAsInvalid, + activeProject?.id, ]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
apps/web/src/app/editor/[project_id]/page.tsx(3 hunks)apps/web/src/app/projects/page.tsx(1 hunks)apps/web/src/app/roadmap/page.tsx(2 hunks)apps/web/src/stores/project-store.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{jsx,tsx}: Don't useaccessKeyattribute on any HTML element.
Don't setaria-hidden="true"on focusable elements.
Don't add ARIA roles, states, and properties to elements that don't support them.
Don't use distracting elements like<marquee>or<blink>.
Only use thescopeprop on<th>elements.
Don't assign non-interactive ARIA roles to interactive HTML elements.
Make sure label elements have text content and are associated with an input.
Don't assign interactive ARIA roles to non-interactive HTML elements.
Don't assigntabIndexto non-interactive HTML elements.
Don't use positive integers fortabIndexproperty.
Don't include "image", "picture", or "photo" in img alt prop.
Don't use explicit role property that's the same as the implicit/default role.
Make static elements with click handlers use a valid role attribute.
Always include atitleelement for SVG elements.
Give all elements requiring alt text meaningful information for screen readers.
Make sure anchors have content that's accessible to screen readers.
AssigntabIndexto non-interactive HTML elements witharia-activedescendant.
Include all required ARIA attributes for elements with ARIA roles.
Make sure ARIA properties are valid for the element's supported roles.
Always include atypeattribute for button elements.
Make elements with interactive roles and handlers focusable.
Give heading elements content that's accessible to screen readers (not hidden witharia-hidden).
Always include alangattribute on the html element.
Always include atitleattribute for iframe elements.
AccompanyonClickwith at least one of:onKeyUp,onKeyDown, oronKeyPress.
AccompanyonMouseOver/onMouseOutwithonFocus/onBlur.
Include caption tracks for audio and video elements.
Use semantic elements instead of role attributes in JSX.
Make sure all anchors are valid and navigable.
Ensure all ARIA properties (aria-*) are valid.
Use valid, non-abstract ARIA roles for elements with...
Files:
apps/web/src/app/projects/page.tsxapps/web/src/app/roadmap/page.tsxapps/web/src/app/editor/[project_id]/page.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Don't use TypeScript enums.
Don't export imported variables.
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions.
Don't use TypeScript namespaces.
Don't use non-null assertions with the!postfix operator.
Don't use parameter properties in class constructors.
Don't use user-defined types.
Useas constinstead of literal types and type annotations.
Use eitherT[]orArray<T>consistently.
Initialize each enum member value explicitly.
Useexport typefor types.
Useimport typefor types.
Make sure all enum members are literal values.
Don't use TypeScript const enum.
Don't declare empty interfaces.
Don't let variables evolve into any type through reassignments.
Don't use the any type.
Don't misuse the non-null assertion operator (!) in TypeScript files.
Don't use implicit any type on variable declarations.
Don't merge interfaces and classes unsafely.
Don't use overload signatures that aren't next to each other.
Use the namespace keyword instead of the module keyword to declare TypeScript namespaces.
Don't use empty type parameters in type aliases and interfaces.
Don't use any or unknown as type constraints.
Don't use the TypeScript directive @ts-ignore.
Use consistent accessibility modifiers on class properties and methods.
Use function types instead of object types with call signatures.
Don't use void type outside of generic or return types.
**/*.{ts,tsx}: Don't use primitive type aliases or misleading types
Don't use the TypeScript directive @ts-ignore
Don't use TypeScript enums
Use either T[] or Array consistently
Don't use the any type
Files:
apps/web/src/app/projects/page.tsxapps/web/src/app/roadmap/page.tsxapps/web/src/stores/project-store.tsapps/web/src/app/editor/[project_id]/page.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,ts,tsx}: Don't use the return value of React.render.
Don't use consecutive spaces in regular expression literals.
Don't use theargumentsobject.
Don't use primitive type aliases or misleading types.
Don't use the comma operator.
Don't write functions that exceed a given Cognitive Complexity score.
Don't use unnecessary boolean casts.
Don't use unnecessary callbacks with flatMap.
Use for...of statements instead of Array.forEach.
Don't create classes that only have static members (like a static namespace).
Don't use this and super in static contexts.
Don't use unnecessary catch clauses.
Don't use unnecessary constructors.
Don't use unnecessary continue statements.
Don't export empty modules that don't change anything.
Don't use unnecessary escape sequences in regular expression literals.
Don't use unnecessary labels.
Don't use unnecessary nested block statements.
Don't rename imports, exports, and destructured assignments to the same name.
Don't use unnecessary string or template literal concatenation.
Don't use String.raw in template literals when there are no escape sequences.
Don't use useless case statements in switch statements.
Don't use ternary operators when simpler alternatives exist.
Don't use uselessthisaliasing.
Don't initialize variables to undefined.
Don't use the void operators (they're not familiar).
Use arrow functions instead of function expressions.
Use Date.now() to get milliseconds since the Unix Epoch.
Use .flatMap() instead of map().flat() when possible.
Use literal property access instead of computed property access.
Don't use parseInt() or Number.parseInt() when binary, octal, or hexadecimal literals work.
Use concise optional chaining instead of chained logical expressions.
Use regular expression literals instead of the RegExp constructor when possible.
Don't use number literal object member names that aren't base 10 or use underscore separators.
Remove redundant terms from logical expressions.
Use while loops instead of...
Files:
apps/web/src/app/projects/page.tsxapps/web/src/app/roadmap/page.tsxapps/web/src/stores/project-store.tsapps/web/src/app/editor/[project_id]/page.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{tsx,jsx}: Always include a title element for icons unless there's text beside the icon
Always include a type attribute for button elements
Accompany onClick with at least one of: onKeyUp, onKeyDown, or onKeyPress
Accompany onMouseOver/onMouseOut with onFocus/onBlur
Don't import React itself
Don't define React components inside other components
Don't use both children and dangerouslySetInnerHTML on the same element
Don't insert comments as text nodes
Use <>...</> instead of ...
Files:
apps/web/src/app/projects/page.tsxapps/web/src/app/roadmap/page.tsxapps/web/src/app/editor/[project_id]/page.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{ts,tsx,js,jsx}: Don't use the comma operator
Use for...of statements instead of Array.forEach
Don't initialize variables to undefined
Use .flatMap() instead of map().flat() when possible
Don't assign a value to itself
Avoid unused imports and variables
Don't use await inside loops
Don't hardcode sensitive data like API keys and tokens
Don't use the delete operator
Don't use global eval()
Use String.slice() instead of String.substr() and String.substring()
Don't use else blocks when the if block breaks early
Put default function parameters and optional function parameters last
Use new when throwing an error
Use String.trimStart() and String.trimEnd() over String.trimLeft() and String.trimRight()
Files:
apps/web/src/app/projects/page.tsxapps/web/src/app/roadmap/page.tsxapps/web/src/stores/project-store.tsapps/web/src/app/editor/[project_id]/page.tsx
🧬 Code graph analysis (1)
apps/web/src/stores/project-store.ts (2)
apps/web/src/types/project.ts (1)
TProject(3-23)apps/web/src/lib/utils.ts (1)
generateUUID(18-49)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Vade Review
🔇 Additional comments (4)
apps/web/src/app/roadmap/page.tsx (3)
41-42: Status flip to Completed — looks good.No behavior change; display and color mapping remain correct.
50-51: Export/Preview marked Completed — looks good.Consistent with the status-to-style mapping.
59-61: "Text" moved to In Progress — looks good.Copy and badge state align.
apps/web/src/app/editor/[project_id]/page.tsx (1)
41-41: Exposing savedProjects here is fine.Enables conflict checks without extra store calls.
| @@ -50,6 +146,7 @@ export const useProjectStore = create<ProjectStore>((set, get) => ({ | |||
| backgroundColor: "#000000", | |||
| backgroundType: "color", | |||
| blurIntensity: 8, | |||
| bookmarks: [], | |||
There was a problem hiding this comment.
The duplicateProject function creates incomplete project objects missing essential properties, which will cause failures when the project is used.
View Details
Analysis
In the duplicateProject function, the new project object being created is missing several critical properties that are present in the original project and required by the TProject interface. The current implementation only copies thumbnail, createdAt, and updatedAt, but ignores essential properties like:
backgroundColor(defaults to "#000000" in createNewProject)backgroundType(defaults to "color" in createNewProject)blurIntensity(defaults to 8 in createNewProject)bookmarks(defaults to [] in createNewProject)fps(optional but should be copied from original)
This means when a user duplicates a project, the new project will lose all the background settings, bookmarks, and FPS configuration from the original project, effectively creating a broken or incorrect duplicate. The fix is to copy all relevant properties from the original project, similar to how the old implementation worked with the spread operator (...project).
To fix this, the newProject object should include all properties from the original project:
const newProject: TProject = {
id: generateUUID(),
name: `(${nextNumber}) ${baseName}`,
thumbnail: project.thumbnail,
createdAt: new Date(),
updatedAt: new Date(),
backgroundColor: project.backgroundColor,
backgroundType: project.backgroundType,
blurIntensity: project.blurIntensity,
bookmarks: project.bookmarks || [],
fps: project.fps,
};
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 (1)
apps/web/src/components/editor/preview-panel.tsx (1)
4-6: Replace any with a concrete type for text elements.Avoids losing type-safety and matches “Don’t use any”.
-import { TimelineElement, TimelineTrack } from "@/types/timeline"; +import { TimelineElement, TimelineTrack, type TextElement } from "@/types/timeline"; @@ - const handleTextMouseDown = ( + const handleTextMouseDown = ( e: React.MouseEvent<HTMLDivElement>, - element: any, + element: TextElement, trackId: string ) => {Also applies to: 202-206
🧹 Nitpick comments (19)
apps/web/src/types/timeline.ts (1)
1-1: Use type-only import for MediaType.Prevents bundling type metadata and matches guidelines.
Apply:
-import { MediaType } from "@/stores/media-store"; +import type { MediaType } from "@/stores/media-store";apps/web/src/components/editor/audio-waveform.tsx (4)
1-1: Drop default React import and avoid React.FC.Matches TSX guideline “Don’t import React itself” and removes unnecessary React.FC.
-import React, { useEffect, useRef, useState } from "react"; +import { useEffect, useRef, useState } from "react"; @@ -const AudioWaveform: React.FC<AudioWaveformProps> = ({ +const AudioWaveform = ({ audioUrl, height = 32, className = "", }: AudioWaveformProps) => {Also applies to: 10-14
35-46: Set loading state when (re)initializing.Without this, switching audioUrl may not show the loading overlay.
- try { + try { + if (mounted) setIsLoading(true); // Clear any existing instance safely
69-76: Avoid console in production.Route errors to your logger or guard with env check.
- newWaveSurfer.on("error", (err) => { - console.error("WaveSurfer error:", err); + newWaveSurfer.on("error", (err) => { + if (process.env.NODE_ENV !== "production") { + // eslint-disable-next-line no-console + console.error("WaveSurfer error:", err); + } @@ - } catch (err) { - console.error("Failed to initialize WaveSurfer:", err); + } catch (err) { + if (process.env.NODE_ENV !== "production") { + // eslint-disable-next-line no-console + console.error("Failed to initialize WaveSurfer:", err); + }Also applies to: 78-84
77-78: Don’t await non-Promise load.WaveSurfer.load is event-driven; awaiting is unnecessary.
- await newWaveSurfer.load(audioUrl); + newWaveSurfer.load(audioUrl);apps/web/src/types/project.ts (1)
12-13: Clarify bookmark unit and constraints.Please document whether values are seconds or frames, and ensure non-negative, sorted, and deduped.
- bookmarks?: number[]; + /** + * Timeline bookmark positions (in seconds). Must be >= 0. + * Stored sorted ascending and de-duplicated. + */ + bookmarks?: number[];apps/web/src/lib/storage/types.ts (1)
1-2: Use type-only imports for TProject and TimelineTrack.Aligns with guidelines and tree-shaking.
-import { TProject } from "@/types/project"; -import { TimelineTrack } from "@/types/timeline"; +import type { TProject } from "@/types/project"; +import type { TimelineTrack } from "@/types/timeline";apps/web/src/components/editor/preview-panel.tsx (3)
231-258: Prefer for…of over forEach for iteration.Improves readability and early-continue; matches guideline.
- const getActiveElements = (): ActiveElement[] => { - const activeElements: ActiveElement[] = []; - - tracks.forEach((track) => { - track.elements.forEach((element) => { - if (element.hidden) return; - const elementStart = element.startTime; - const elementEnd = - element.startTime + - (element.duration - element.trimStart - element.trimEnd); - - if (currentTime >= elementStart && currentTime < elementEnd) { - let mediaItem = null; - if (element.type === "media") { - mediaItem = - element.mediaId === "test" - ? null - : mediaItems.find((item) => item.id === element.mediaId) || - null; - } - activeElements.push({ element, track, mediaItem }); - } - }); - }); - - return activeElements; - }; + const getActiveElements = (): ActiveElement[] => { + const activeElements: ActiveElement[] = []; + for (const track of tracks) { + for (const element of track.elements) { + if (element.hidden) continue; + const elementStart = element.startTime; + const elementEnd = + element.startTime + + (element.duration - element.trimStart - element.trimEnd); + if (currentTime >= elementStart && currentTime < elementEnd) { + let mediaItem: MediaItem | null = null; + if (element.type === "media") { + mediaItem = + element.mediaId === "test" + ? null + : mediaItems.find((item) => item.id === element.mediaId) || null; + } + activeElements.push({ element, track, mediaItem }); + } + } + } + return activeElements; + };
631-642: Use nullish coalescing for fps fallback.Avoids overriding legitimate 0 with default.
- fps={activeProject?.fps || 30} + fps={activeProject?.fps ?? 30} @@ - activeProject?.fps || 30 + activeProject?.fps ?? 30Also applies to: 870-871
658-669: Add accessible labels to icon-only play/pause buttons.Improves a11y; buttons currently have no text or title.
- <Button + <Button variant="text" size="icon" onClick={toggle} disabled={!hasAnyElements} className="h-auto p-0 text-white hover:text-white/80" + title={isPlaying ? "Pause" : "Play"} + aria-label={isPlaying ? "Pause" : "Play"} > @@ - <Button + <Button variant="text" size="icon" onClick={toggle} disabled={!hasAnyElements} className="h-auto p-0" + title={isPlaying ? "Pause" : "Play"} + aria-label={isPlaying ? "Pause" : "Play"} >Also applies to: 876-887
apps/web/src/lib/storage/storage-service.ts (2)
66-68: Default new fields for backward compatibility.Older projects won’t have
bookmarks/fps. Default them at save time to avoid persistingundefined.Apply this diff:
- bookmarks: project.bookmarks, - fps: project.fps, + bookmarks: project.bookmarks ?? [], + fps: project.fps ?? 30,
88-90: Default loaded fields to safe values.Ensure pre-existing records deserialize safely.
Apply this diff:
- bookmarks: serializedProject.bookmarks, - fps: serializedProject.fps, + bookmarks: serializedProject.bookmarks ?? [], + fps: serializedProject.fps ?? 30,apps/web/src/components/editor/timeline/timeline-element.tsx (2)
79-84: Avoid duplicate media lookup.You already compute
mediaItem/isAudiohere but re-query insiderenderElementContent. Reuse the top-level values to cut redundant work.As these changes touch lines outside this hunk, here’s a minimal adjustment inside
renderElementContent:// inside renderElementContent, remove the re-declaration: // const mediaItem = mediaItems.find((item) => item.id === element.mediaId); // and use the outer-scoped `mediaItem` directly.
351-352: Minor: prefer a data attribute over class toggle for state.Consider adding
data-hidden={element.hidden ? "true" : "false"}to simplify testing and styling.- )} ${element.hidden ? "opacity-50" : ""}`} + )} ${element.hidden ? "opacity-50" : ""}`} + data-hidden={element.hidden ? "true" : "false"}apps/web/src/stores/project-store.ts (3)
49-89: Use frame-sized epsilon instead of fixed 0.001.Make comparisons resilient across FPS by using half-frame tolerance.
Apply this diff:
- const fps = activeProject.fps || 30; - const frameTime = Math.round(time * fps) / fps; + const fps = activeProject.fps || 30; + const frameTime = Math.round(time * fps) / fps; + const epsilon = 1 / (2 * fps); @@ - const bookmarkIndex = bookmarks.findIndex( - bookmark => Math.abs(bookmark - frameTime) < 0.001 - ); + const bookmarkIndex = bookmarks.findIndex( + (bookmark) => Math.abs(bookmark - frameTime) < epsilon + );
91-102: Same epsilon improvement for reads.Apply this diff:
- const fps = activeProject.fps || 30; - const frameTime = Math.round(time * fps) / fps; + const fps = activeProject.fps || 30; + const frameTime = Math.round(time * fps) / fps; + const epsilon = 1 / (2 * fps); @@ - return activeProject.bookmarks.some( - bookmark => Math.abs(bookmark - frameTime) < 0.001 - ); + return activeProject.bookmarks.some( + (bookmark) => Math.abs(bookmark - frameTime) < epsilon + );
104-137: And for removals.Apply this diff:
- const fps = activeProject.fps || 30; - const frameTime = Math.round(time * fps) / fps; + const fps = activeProject.fps || 30; + const frameTime = Math.round(time * fps) / fps; + const epsilon = 1 / (2 * fps); @@ - const updatedBookmarks = activeProject.bookmarks.filter( - bookmark => Math.abs(bookmark - frameTime) >= 0.001 - ); + const updatedBookmarks = activeProject.bookmarks.filter( + (bookmark) => Math.abs(bookmark - frameTime) >= epsilon + );apps/web/src/components/editor/timeline/index.tsx (2)
656-679: Subscribe via hook; avoidgetState()in render.You already have
activeProjectfrom the hook andseekin scope. This simplifies and keeps reactivity explicit.Apply this diff:
- {(() => { - const { activeProject } = useProjectStore.getState(); - if (!activeProject?.bookmarks?.length) return null; - - return activeProject.bookmarks.map((bookmarkTime, i) => ( - <div - key={`bookmark-${i}`} - className="absolute top-0 h-10 w-0.5 !bg-primary cursor-pointer" - style={{ - left: `${bookmarkTime * TIMELINE_CONSTANTS.PIXELS_PER_SECOND * zoomLevel}px`, - }} - onClick={(e) => { - e.stopPropagation(); - usePlaybackStore.getState().seek(bookmarkTime); - }} - > - <div className="absolute top-[-1px] left-[-5px] text-primary"> - <Bookmark className="h-3 w-3 fill-primary" /> - </div> - </div> - )); - })()} + {activeProject?.bookmarks?.length + ? activeProject.bookmarks.map((bookmarkTime, i) => ( + <div + key={`bookmark-${i}`} + className="absolute top-0 h-10 w-0.5 !bg-primary cursor-pointer" + style={{ + left: `${ + bookmarkTime * + TIMELINE_CONSTANTS.PIXELS_PER_SECOND * + zoomLevel + }px`, + }} + onClick={(e) => { + e.stopPropagation(); + seek(bookmarkTime); + }} + > + <div className="absolute top-[-1px] left-[-5px] text-primary"> + <Bookmark className="h-3 w-3 fill-primary" /> + </div> + </div> + )) + : null}
801-817: Use concise optional chaining.Simplify the condition per guidelines.
Apply this diff:
- {activeProject?.bookmarks?.length && activeProject.bookmarks.length > 0 && ( + {activeProject?.bookmarks?.length > 0 && (
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
apps/web/src/components/editor/audio-waveform.tsx(3 hunks)apps/web/src/components/editor/preview-panel.tsx(1 hunks)apps/web/src/components/editor/timeline/index.tsx(7 hunks)apps/web/src/components/editor/timeline/timeline-element.tsx(6 hunks)apps/web/src/lib/storage/storage-service.ts(2 hunks)apps/web/src/lib/storage/types.ts(1 hunks)apps/web/src/stores/project-store.ts(3 hunks)apps/web/src/stores/timeline-store.ts(2 hunks)apps/web/src/types/project.ts(1 hunks)apps/web/src/types/timeline.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Don't use TypeScript enums.
Don't export imported variables.
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions.
Don't use TypeScript namespaces.
Don't use non-null assertions with the!postfix operator.
Don't use parameter properties in class constructors.
Don't use user-defined types.
Useas constinstead of literal types and type annotations.
Use eitherT[]orArray<T>consistently.
Initialize each enum member value explicitly.
Useexport typefor types.
Useimport typefor types.
Make sure all enum members are literal values.
Don't use TypeScript const enum.
Don't declare empty interfaces.
Don't let variables evolve into any type through reassignments.
Don't use the any type.
Don't misuse the non-null assertion operator (!) in TypeScript files.
Don't use implicit any type on variable declarations.
Don't merge interfaces and classes unsafely.
Don't use overload signatures that aren't next to each other.
Use the namespace keyword instead of the module keyword to declare TypeScript namespaces.
Don't use empty type parameters in type aliases and interfaces.
Don't use any or unknown as type constraints.
Don't use the TypeScript directive @ts-ignore.
Use consistent accessibility modifiers on class properties and methods.
Use function types instead of object types with call signatures.
Don't use void type outside of generic or return types.
**/*.{ts,tsx}: Don't use primitive type aliases or misleading types
Don't use the TypeScript directive @ts-ignore
Don't use TypeScript enums
Use either T[] or Array consistently
Don't use the any type
Files:
apps/web/src/types/timeline.tsapps/web/src/lib/storage/types.tsapps/web/src/types/project.tsapps/web/src/components/editor/preview-panel.tsxapps/web/src/stores/timeline-store.tsapps/web/src/lib/storage/storage-service.tsapps/web/src/components/editor/timeline/timeline-element.tsxapps/web/src/components/editor/timeline/index.tsxapps/web/src/components/editor/audio-waveform.tsxapps/web/src/stores/project-store.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,ts,tsx}: Don't use the return value of React.render.
Don't use consecutive spaces in regular expression literals.
Don't use theargumentsobject.
Don't use primitive type aliases or misleading types.
Don't use the comma operator.
Don't write functions that exceed a given Cognitive Complexity score.
Don't use unnecessary boolean casts.
Don't use unnecessary callbacks with flatMap.
Use for...of statements instead of Array.forEach.
Don't create classes that only have static members (like a static namespace).
Don't use this and super in static contexts.
Don't use unnecessary catch clauses.
Don't use unnecessary constructors.
Don't use unnecessary continue statements.
Don't export empty modules that don't change anything.
Don't use unnecessary escape sequences in regular expression literals.
Don't use unnecessary labels.
Don't use unnecessary nested block statements.
Don't rename imports, exports, and destructured assignments to the same name.
Don't use unnecessary string or template literal concatenation.
Don't use String.raw in template literals when there are no escape sequences.
Don't use useless case statements in switch statements.
Don't use ternary operators when simpler alternatives exist.
Don't use uselessthisaliasing.
Don't initialize variables to undefined.
Don't use the void operators (they're not familiar).
Use arrow functions instead of function expressions.
Use Date.now() to get milliseconds since the Unix Epoch.
Use .flatMap() instead of map().flat() when possible.
Use literal property access instead of computed property access.
Don't use parseInt() or Number.parseInt() when binary, octal, or hexadecimal literals work.
Use concise optional chaining instead of chained logical expressions.
Use regular expression literals instead of the RegExp constructor when possible.
Don't use number literal object member names that aren't base 10 or use underscore separators.
Remove redundant terms from logical expressions.
Use while loops instead of...
Files:
apps/web/src/types/timeline.tsapps/web/src/lib/storage/types.tsapps/web/src/types/project.tsapps/web/src/components/editor/preview-panel.tsxapps/web/src/stores/timeline-store.tsapps/web/src/lib/storage/storage-service.tsapps/web/src/components/editor/timeline/timeline-element.tsxapps/web/src/components/editor/timeline/index.tsxapps/web/src/components/editor/audio-waveform.tsxapps/web/src/stores/project-store.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{ts,tsx,js,jsx}: Don't use the comma operator
Use for...of statements instead of Array.forEach
Don't initialize variables to undefined
Use .flatMap() instead of map().flat() when possible
Don't assign a value to itself
Avoid unused imports and variables
Don't use await inside loops
Don't hardcode sensitive data like API keys and tokens
Don't use the delete operator
Don't use global eval()
Use String.slice() instead of String.substr() and String.substring()
Don't use else blocks when the if block breaks early
Put default function parameters and optional function parameters last
Use new when throwing an error
Use String.trimStart() and String.trimEnd() over String.trimLeft() and String.trimRight()
Files:
apps/web/src/types/timeline.tsapps/web/src/lib/storage/types.tsapps/web/src/types/project.tsapps/web/src/components/editor/preview-panel.tsxapps/web/src/stores/timeline-store.tsapps/web/src/lib/storage/storage-service.tsapps/web/src/components/editor/timeline/timeline-element.tsxapps/web/src/components/editor/timeline/index.tsxapps/web/src/components/editor/audio-waveform.tsxapps/web/src/stores/project-store.ts
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{jsx,tsx}: Don't useaccessKeyattribute on any HTML element.
Don't setaria-hidden="true"on focusable elements.
Don't add ARIA roles, states, and properties to elements that don't support them.
Don't use distracting elements like<marquee>or<blink>.
Only use thescopeprop on<th>elements.
Don't assign non-interactive ARIA roles to interactive HTML elements.
Make sure label elements have text content and are associated with an input.
Don't assign interactive ARIA roles to non-interactive HTML elements.
Don't assigntabIndexto non-interactive HTML elements.
Don't use positive integers fortabIndexproperty.
Don't include "image", "picture", or "photo" in img alt prop.
Don't use explicit role property that's the same as the implicit/default role.
Make static elements with click handlers use a valid role attribute.
Always include atitleelement for SVG elements.
Give all elements requiring alt text meaningful information for screen readers.
Make sure anchors have content that's accessible to screen readers.
AssigntabIndexto non-interactive HTML elements witharia-activedescendant.
Include all required ARIA attributes for elements with ARIA roles.
Make sure ARIA properties are valid for the element's supported roles.
Always include atypeattribute for button elements.
Make elements with interactive roles and handlers focusable.
Give heading elements content that's accessible to screen readers (not hidden witharia-hidden).
Always include alangattribute on the html element.
Always include atitleattribute for iframe elements.
AccompanyonClickwith at least one of:onKeyUp,onKeyDown, oronKeyPress.
AccompanyonMouseOver/onMouseOutwithonFocus/onBlur.
Include caption tracks for audio and video elements.
Use semantic elements instead of role attributes in JSX.
Make sure all anchors are valid and navigable.
Ensure all ARIA properties (aria-*) are valid.
Use valid, non-abstract ARIA roles for elements with...
Files:
apps/web/src/components/editor/preview-panel.tsxapps/web/src/components/editor/timeline/timeline-element.tsxapps/web/src/components/editor/timeline/index.tsxapps/web/src/components/editor/audio-waveform.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{tsx,jsx}: Always include a title element for icons unless there's text beside the icon
Always include a type attribute for button elements
Accompany onClick with at least one of: onKeyUp, onKeyDown, or onKeyPress
Accompany onMouseOver/onMouseOut with onFocus/onBlur
Don't import React itself
Don't define React components inside other components
Don't use both children and dangerouslySetInnerHTML on the same element
Don't insert comments as text nodes
Use <>...</> instead of ...
Files:
apps/web/src/components/editor/preview-panel.tsxapps/web/src/components/editor/timeline/timeline-element.tsxapps/web/src/components/editor/timeline/index.tsxapps/web/src/components/editor/audio-waveform.tsx
🧠 Learnings (1)
📚 Learning: 2025-08-09T09:03:49.797Z
Learnt from: CR
PR: OpenCut-app/OpenCut#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-09T09:03:49.797Z
Learning: Applies to **/*.{jsx,tsx} : Don't set `aria-hidden="true"` on focusable elements.
Applied to files:
apps/web/src/components/editor/preview-panel.tsx
🧬 Code graph analysis (2)
apps/web/src/components/editor/timeline/index.tsx (3)
apps/web/src/stores/project-store.ts (1)
useProjectStore(42-457)apps/web/src/constants/timeline-constants.ts (1)
TIMELINE_CONSTANTS(72-79)apps/web/src/stores/playback-store.ts (1)
usePlaybackStore(53-134)
apps/web/src/stores/project-store.ts (3)
apps/web/src/lib/storage/indexeddb-adapter.ts (2)
get(30-40)set(42-52)apps/web/src/lib/storage/opfs-adapter.ts (2)
get(17-28)set(30-37)apps/web/src/lib/storage/storage-service.ts (1)
storageService(282-282)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Vade Review
🔇 Additional comments (17)
apps/web/src/types/timeline.ts (1)
14-15: Hidden flag addition looks good; ensure default + persistence.OK to make it optional. Please confirm new elements default to hidden: false and that serialization/persistence include this field end-to-end.
If helpful, I can scan the store/actions and storage layer to verify defaults and round-trip persistence.
apps/web/src/components/editor/preview-panel.tsx (1)
237-238: Respect hidden elements—LGTM.Skipping hidden items here keeps preview consistent with timeline visibility.
apps/web/src/stores/timeline-store.ts (3)
129-129: API addition looks good.
toggleElementHiddenis a clear, focused addition to the public store API.
872-888: Correct, history-friendly toggle with persistence.The implementation properly snapshots history and persists via
updateTracksAndSave. Using a pure map/clone keeps immutability intact.
872-888: Verify persistence/renderer round-trip forhidden.Please confirm:
BaseTimelineElementincludeshidden?: boolean(types).- Export/preview pipelines skip hidden elements consistently (audio/video/text).
apps/web/src/components/editor/timeline/timeline-element.tsx (5)
16-20: Icon imports — OK.
73-74: Store wiring — OK.
toggleElementHiddenis correctly pulled from the store.
155-158: Handler is clean and side-effect free.
362-371: Clear, accessible overlay state.Overlay choice (EyeOff/VolumeX) communicates hidden/muted well;
pointer-events-noneavoids trapping events.
392-414: Context menu toggle is intuitive and consistent.Icon/label combinations correctly reflect the next action (Show/Hide, Mute/Unmute).
apps/web/src/stores/project-store.ts (2)
31-34: Nice, focused bookmark API.Clear separation of concerns with
toggleBookmark,isBookmarked, andremoveBookmark.
149-150: Initialize bookmarks — good default.apps/web/src/components/editor/timeline/index.tsx (5)
22-23: Import looks good.
873-874: Toolbar store wiring — OK.
1004-1007: Bookmark toggle handler — OK.
1009-1010: Efficient derived state.
isBookmarked(currentTime)is cheap; fine to recompute on render.
1141-1151: Good UX: single-toggle button with contextual tooltip.
| export type SerializedProject = Omit<TProject, "createdAt" | "updatedAt"> & { | ||
| createdAt: string; | ||
| updatedAt: string; | ||
| bookmarks?: number[]; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove redundant bookmarks override in SerializedProject.
bookmarks already comes from TProject via Omit<...>. Duplicating can cause drift.
export type SerializedProject = Omit<TProject, "createdAt" | "updatedAt"> & {
createdAt: string;
updatedAt: string;
- bookmarks?: number[];
};📝 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.
| export type SerializedProject = Omit<TProject, "createdAt" | "updatedAt"> & { | |
| createdAt: string; | |
| updatedAt: string; | |
| bookmarks?: number[]; | |
| }; | |
| export type SerializedProject = Omit<TProject, "createdAt" | "updatedAt"> & { | |
| createdAt: string; | |
| updatedAt: string; | |
| }; |
🤖 Prompt for AI Agents
In apps/web/src/lib/storage/types.ts around lines 37 to 41, the
SerializedProject type unnecessarily re-declares bookmarks?: number[] even
though bookmarks is already included via the Omit<TProject, "createdAt" |
"updatedAt">; remove the redundant bookmarks override from the intersection so
SerializedProject only overrides createdAt and updatedAt to string, keeping
bookmarks inherited from TProject to avoid drift.
Description
enhancements: update roadmap to the current project state. Increment project names automatically for less confusion
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
Screenshots:
Old Naming Way
New Naming Way
last updated was (July 14, 2025)
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Persistence