feat: 2 modes, feat: extract audio from media videos, feat: duplicate projects, feat: bookmarks managements refactor: tailwind v4... #502
Conversation
|
@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
|
WalkthroughThis update primarily introduces debug logging to the timeline editor's mouse event handlers, refactors the tooltip component to support multiple visual variants using Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TimelineComponent
participant Console
User->>TimelineComponent: Mouse down/click on timeline
TimelineComponent->>Console: Log debug info (event, target, state)
TimelineComponent-->>User: (Normal timeline interaction proceeds)
sequenceDiagram
participant App
participant Tooltip
participant User
App->>Tooltip: Render TooltipContent with variant prop
Tooltip->>Tooltip: Apply cva-based styling for variant
User->>Tooltip: Hover/Focus
Tooltip-->>User: Show styled tooltip (with optional arrow)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ 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 comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🔭 Outside diff range comments (2)
apps/web/src/components/ui/input.tsx (1)
22-23: Wrapper losesrelativepositioning when the password is hiddenThe absolute-positioned toggle button relies on the parent being
relative.
WhenshowPassword === false, the parent div gets an empty class list, so the button drifts out of place even thoughshowPasswordToggleis still true. Switch the condition to track the presence of the toggle, not the password’s visibility.- <div className={showPassword ? "relative w-full" : ""}> + <div className={showPasswordToggle ? "relative w-full" : ""}>apps/web/src/components/ui/breadcrumb.tsx (1)
95-103:aria-hidden="true"cancels out the accessible “More” label.Because the wrapping
<span>isaria-hidden="true", the entire subtree (including the visually-hidden<span class="sr-only">More</span>) is removed from the accessibility tree, so screen-reader users will never hear the “More” label.- <span - role="presentation" - aria-hidden="true" + <spanEither:
- Drop
aria-hidden(and therole="presentation"that becomes redundant) so the innersr-onlytext is announced, or- Keep
aria-hiddenbut remove thesr-onlynode since it has no effect.Failing to address this leaves the ellipsis completely undiscoverable to assistive technologies.
♻️ Duplicate comments (5)
apps/web/src/components/ui/accordion.tsx (1)
37-37: Samesize-4utility concern as noted insheet.tsxapps/web/src/components/header.tsx (1)
34-34: Samesize-4utility concern as noted insheet.tsxapps/web/src/components/ui/command.tsx (1)
43-43: Samesize-4utility concern as noted insheet.tsxapps/web/src/components/ui/toast.tsx (1)
86-86: Samesize-4utility concern as noted insheet.tsxapps/web/src/app/roadmap/page.tsx (1)
222-224: Same accessibility concern for the “Start Contributing” button iconAdd a descriptive label or hide the SVG from screen readers.
🧹 Nitpick comments (11)
apps/web/src/components/ui/breadcrumb.tsx (1)
105-105: Typo indisplayName.
"BreadcrumbElipssis"has an extra “s”.-BreadcrumbEllipsis.displayName = "BreadcrumbElipssis"; +BreadcrumbEllipsis.displayName = "BreadcrumbEllipsis";apps/web/src/components/ui/slider.tsx (1)
23-23: Confirm new Tailwind v4 utilities are actually enabled & do not regress focus styling
size-4andoutline-hiddenare v4-only classes.
- Double-check the Tailwind config is already bumped to v4 in every package that consumes this component; otherwise the classes will be silently dropped at build-time.
- The
focus-visible:outline-hiddenhides the browser outline, so the ring becomes the only focus indicator. Make sure the ring colour meets WCAG contrast (especially in dark mode) and that high-contrast/forced-colours modes still present a visible focus cue.No functional change needed if both checks pass, just verify.
apps/web/src/components/ui/carousel.tsx (1)
219-220: Consider marking decorative icons as hidden from screen readersThe
<ArrowLeft>and<ArrowRight>SVGs are purely visual, while the actionable text is already provided through thesr-onlyspans.
Addingaria-hidden="true"(andfocusable="false"for Safari/IE compatibility) keeps screen-reader output concise.- <ArrowLeft className="size-4" /> + <ArrowLeft + className="size-4" + aria-hidden="true" + focusable="false" + /> ... - <ArrowRight className="size-4" /> + <ArrowRight + className="size-4" + aria-hidden="true" + focusable="false" + />Also applies to: 248-249
apps/web/src/components/ui/pagination.tsx (2)
69-74: Add an accessible title (or hide the SVG) for the Chevron icon.
ChevronLeftis an inline SVG without a<title>element or anaria-hidden="true"attribute.
The accessibility guidelines for*.{jsx,tsx}in this repo explicitly require either atitleelement for every SVG or that the SVG be hidden from assistive tech if another accessible name is present (the visible “Previous” text already fulfils that).- <ChevronLeft className="size-4" /> + {/* decorative icon – hide from AT while keeping visible text accessible */} + <ChevronLeft + className="size-4" + aria-hidden="true" + focusable="false" + />
85-90: Mirror the accessibility fix for the right-chevron icon.Same concern as above: expose either a
<title>or hide the SVG.- <ChevronRight className="size-4" /> + <ChevronRight + className="size-4" + aria-hidden="true" + focusable="false" + />apps/web/src/lib/ffmpeg-utils.ts (3)
121-124: Consider using fetchFile for consistency.While
generateThumbnailandtrimVideonow usefetchFile, this function still uses manualUint8Arrayconversion. For consistency across the module, consider updating this to usefetchFileas well.- await ffmpeg.writeFile( - inputName, - new Uint8Array(await videoFile.arrayBuffer()) - ); + await ffmpeg.writeFile(inputName, await fetchFile(videoFile));
201-204: Use fetchFile here as well for consistency.The
convertToWebMfunction should also be updated to usefetchFilefor consistency with other functions in this module.- await ffmpeg.writeFile( - inputName, - new Uint8Array(await videoFile.arrayBuffer()) - ); + await ffmpeg.writeFile(inputName, await fetchFile(videoFile));
242-245: Update to use fetchFile for consistency.The
extractAudiofunction should also usefetchFileto maintain consistency across all FFmpeg operations in this module.- await ffmpeg.writeFile( - inputName, - new Uint8Array(await videoFile.arrayBuffer()) - ); + await ffmpeg.writeFile(inputName, await fetchFile(videoFile));apps/web/src/lib/storage/storage-service.ts (1)
228-242: Remove unused targetAdapters variable.The
targetAdaptersvariable is created but never used. ThesaveMediaItemmethod internally gets the adapters it needs.async copyProjectMedia(sourceProjectId: string, targetProjectId: string): Promise<void> { const sourceAdapters = this.getProjectMediaAdapters(sourceProjectId); - const targetAdapters = this.getProjectMediaAdapters(targetProjectId); // Get all media items from source project const mediaIds = await sourceAdapters.mediaMetadataAdapter.list();apps/web/src/stores/timeline-store.ts (1)
1133-1133: Consider moving dynamic import to module level.The dynamic import of ffmpeg-utils happens on every call. For better performance, consider importing at the module level or caching the import result.
+import { extractAudio } from "@/lib/ffmpeg-utils"; + // ... in the method - const { extractAudio } = await import("@/lib/ffmpeg-utils"); const audioBlob = await extractAudio(mediaItem.file, "mp3");apps/web/src/components/editor/timeline/index.tsx (1)
1203-1257: Review bookmark dropdown implementation.The bookmark dropdown functionality is comprehensive but has some structural concerns:
- Complex nesting: The bookmark button and dropdown are nested within multiple Button components, which may cause accessibility issues
- Button sizing: The outer secondary button seems unnecessary and may confuse users
- Event handling: The click handlers are correctly implemented with proper event stopping
Consider simplifying the structure by removing the outer secondary button wrapper:
-<Button variant="secondary" size="sm" className="gap-2 flex items-center justify-center"> -<Tooltip> - <TooltipTrigger asChild> - <Button variant="text" size="icon" className="size-4" - onClick={handleToggleBookmark}> - <Bookmark className={`size-4 ${currentBookmarked ? "fill-primary text-primary" : ""}`} /> - </Button> -</TooltipTrigger> -<TooltipContent> - {currentBookmarked ? "Remove bookmark" : "Add bookmark"} -</TooltipContent> -</Tooltip> -<DropdownMenu> - <DropdownMenuTrigger asChild> - <Button variant="text" size="icon" className="size-4"> - <ChevronDown className=" text-muted-foreground" /> - </Button> - </DropdownMenuTrigger> +<div className="flex items-center"> + <Tooltip> + <TooltipTrigger asChild> + <Button variant="text" size="icon" onClick={handleToggleBookmark}> + <Bookmark className={`size-4 ${currentBookmarked ? "fill-primary text-primary" : ""}`} /> + </Button> + </TooltipTrigger> + <TooltipContent> + {currentBookmarked ? "Remove bookmark" : "Add bookmark"} + </TooltipContent> + </Tooltip> + <DropdownMenu> + <DropdownMenuTrigger asChild> + <Button variant="text" size="icon"> + <ChevronDown className="size-4 text-muted-foreground" /> + </Button> + </DropdownMenuTrigger>
apps/web/src/app/roadmap/page.tsx
Outdated
| <GithubIcon className="size-3" /> | ||
| Open Source | ||
| </Badge> |
There was a problem hiding this comment.
Missing title/aria-label on SVG
<GithubIcon className="size-3" /> needs either a <title> or aria-hidden="true" per accessibility guidelines.
🤖 Prompt for AI Agents
In apps/web/src/app/roadmap/page.tsx around lines 136 to 138, the <GithubIcon>
SVG element lacks an accessible name or indication. To fix this, either add a
<title> element inside the SVG to provide a descriptive label or add
aria-hidden="true" to the <GithubIcon> component if it is purely decorative,
ensuring compliance with accessibility guidelines.
| const handleElementClick = (e: React.MouseEvent) => { | ||
| // Handle split mode | ||
| if (toolMode === "split") { | ||
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
|
|
||
| // Calculate click position relative to element | ||
| const rect = e.currentTarget.getBoundingClientRect(); | ||
| const clickX = e.clientX - rect.left; | ||
| const clickTime = element.startTime + (clickX / (TIMELINE_CONSTANTS.PIXELS_PER_SECOND * zoomLevel)); | ||
|
|
||
| // Ensure click time is within element bounds | ||
| const effectiveStart = element.startTime; | ||
| const effectiveEnd = element.startTime + (element.duration - element.trimStart - element.trimEnd); | ||
|
|
||
| if (clickTime >= effectiveStart && clickTime <= effectiveEnd) { | ||
| const secondElementId = splitElement(track.id, element.id, clickTime); | ||
| if (!secondElementId) { | ||
| toast.error("Failed to split element"); | ||
| } | ||
| } else { | ||
| // If click is outside element bounds, just move playhead to that position | ||
| const { seek } = usePlaybackStore.getState(); | ||
| seek(clickTime); | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| // Normal click handling | ||
| if (onElementClick) { | ||
| onElementClick(e, element); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Review split mode click handling implementation.
The new handleElementClick function implements split mode logic but has some concerns:
- Direct store access: Line 358 uses
usePlaybackStore.getState()which bypasses React's state management system - Click position calculation: The click time calculation looks correct but should validate bounds more robustly
Replace direct store access with proper hook usage:
- const { seek } = usePlaybackStore.getState();
- seek(clickTime);
+ const { seek } = usePlaybackStore();
+ seek(clickTime);However, since this is inside an event handler and seek is already available from the parent component's usePlaybackStore() hook, consider passing it as a prop instead.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleElementClick = (e: React.MouseEvent) => { | |
| // Handle split mode | |
| if (toolMode === "split") { | |
| e.preventDefault(); | |
| e.stopPropagation(); | |
| // Calculate click position relative to element | |
| const rect = e.currentTarget.getBoundingClientRect(); | |
| const clickX = e.clientX - rect.left; | |
| const clickTime = element.startTime + (clickX / (TIMELINE_CONSTANTS.PIXELS_PER_SECOND * zoomLevel)); | |
| // Ensure click time is within element bounds | |
| const effectiveStart = element.startTime; | |
| const effectiveEnd = element.startTime + (element.duration - element.trimStart - element.trimEnd); | |
| if (clickTime >= effectiveStart && clickTime <= effectiveEnd) { | |
| const secondElementId = splitElement(track.id, element.id, clickTime); | |
| if (!secondElementId) { | |
| toast.error("Failed to split element"); | |
| } | |
| } else { | |
| // If click is outside element bounds, just move playhead to that position | |
| const { seek } = usePlaybackStore.getState(); | |
| seek(clickTime); | |
| } | |
| return; | |
| } | |
| // Normal click handling | |
| if (onElementClick) { | |
| onElementClick(e, element); | |
| } | |
| }; | |
| const handleElementClick = (e: React.MouseEvent) => { | |
| // Handle split mode | |
| if (toolMode === "split") { | |
| e.preventDefault(); | |
| e.stopPropagation(); | |
| // Calculate click position relative to element | |
| const rect = e.currentTarget.getBoundingClientRect(); | |
| const clickX = e.clientX - rect.left; | |
| const clickTime = element.startTime + (clickX / (TIMELINE_CONSTANTS.PIXELS_PER_SECOND * zoomLevel)); | |
| // Ensure click time is within element bounds | |
| const effectiveStart = element.startTime; | |
| const effectiveEnd = element.startTime + (element.duration - element.trimStart - element.trimEnd); | |
| if (clickTime >= effectiveStart && clickTime <= effectiveEnd) { | |
| const secondElementId = splitElement(track.id, element.id, clickTime); | |
| if (!secondElementId) { | |
| toast.error("Failed to split element"); | |
| } | |
| } else { | |
| // If click is outside element bounds, just move playhead to that position | |
| - const { seek } = usePlaybackStore.getState(); | |
| - seek(clickTime); | |
| + const { seek } = usePlaybackStore(); | |
| + seek(clickTime); | |
| } | |
| return; | |
| } | |
| // Normal click handling | |
| if (onElementClick) { | |
| onElementClick(e, element); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In apps/web/src/components/editor/timeline/timeline-element.tsx between lines
336 and 368, the handleElementClick function directly accesses the playback
store using usePlaybackStore.getState(), which bypasses React's state
management. To fix this, remove the direct store access and instead pass the
seek function as a prop from the parent component where usePlaybackStore() is
already used. Then use this passed seek prop inside handleElementClick to move
the playhead. Also, review the click time bounds validation to ensure it
robustly checks that clickTime is within the element's effective start and end
times before splitting or seeking.
| <ArrowRight className="relative z-10 ml-0.5 size-4 inline-block" /> | ||
| </Button> |
There was a problem hiding this comment.
Icon needs an accessible label or explicit hiding
Same accessibility gap as in the dialog file – the <ArrowRight> SVG is exposed without a <title>.
- <ArrowRight className="relative z-10 ml-0.5 size-4 inline-block" />
+ <ArrowRight
+ className="relative z-10 ml-0.5 size-4 inline-block"
+ role="img"
+ aria-label="Arrow pointing right"
+ />Add aria-hidden="true" if the arrow is decorative.
🤖 Prompt for AI Agents
In apps/web/src/components/landing/hero.tsx around lines 74 to 75, the
<ArrowRight> SVG icon lacks an accessible label or explicit hiding, which is an
accessibility issue. If the arrow icon is purely decorative, add the attribute
aria-hidden="true" to the <ArrowRight> component to hide it from screen readers.
This ensures the icon does not interfere with accessibility tools.
| <Check className="size-4" /> | ||
| </CheckboxPrimitive.Indicator> |
There was a problem hiding this comment.
Decorative checkmark SVG should be hidden from AT
The checkmark conveys state visually but is redundant for screen readers (Radix already sets the correct aria-checked).
- <Check className="size-4" />
+ <Check className="size-4" aria-hidden="true" focusable="false" />This prevents double-announcement of the same state.
🤖 Prompt for AI Agents
In apps/web/src/components/ui/checkbox.tsx around lines 24 to 25, the Check SVG
used as a decorative checkmark should be hidden from assistive technologies to
avoid redundant state announcements. Add an attribute such as aria-hidden="true"
to the Check component to ensure it is ignored by screen readers, relying on
Radix's aria-checked for conveying state instead.
| <X className="size-4" /> | ||
| <span className="sr-only">Close</span> |
There was a problem hiding this comment.
Add an accessible title (or hide the SVG) on the close icon
lucide-react icons render a bare <svg> with no <title> element.
The project guidelines require every SVG to expose a descriptive title unless it’s explicitly hidden from assistive tech.
- <X className="size-4" />
+ <X
+ className="size-4"
+ role="img"
+ aria-label="Close dialog"
+ />If the icon is purely decorative, add aria-hidden="true" instead.
This avoids the icon being announced as an empty graphic.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/src/components/ui/dialog.tsx around lines 59 to 60, the SVG icon
from lucide-react lacks an accessible title, which is required by project
guidelines. To fix this, either add a descriptive <title> element inside the SVG
or, if the icon is purely decorative, add aria-hidden="true" to the SVG element
to hide it from assistive technologies.
| <span | ||
| aria-hidden | ||
| className={cn("flex h-9 w-9 items-center justify-center", className)} | ||
| className={cn("flex size-9 items-center justify-center", className)} | ||
| {...props} | ||
| > | ||
| <MoreHorizontal className="h-4 w-4" /> | ||
| <MoreHorizontal className="size-4" /> | ||
| <span className="sr-only">More pages</span> | ||
| </span> |
There was a problem hiding this comment.
aria-hidden on the wrapper hides the screen-reader label—move it to the icon only.
Placing aria-hidden on the outer <span> prevents assistive tech from reaching the nested sr-only text, defeating its purpose and making the ellipsis undiscoverable.
- <span
- aria-hidden
- className={cn("flex size-9 items-center justify-center", className)}
- {...props}
- >
- <MoreHorizontal className="size-4" />
- <span className="sr-only">More pages</span>
- </span>
+ <span
+ className={cn("flex size-9 items-center justify-center", className)}
+ {...props}
+ >
+ <span aria-hidden="true">
+ <MoreHorizontal className="size-4" />
+ </span>
+ <span className="sr-only">More pages</span>
+ </span>📝 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.
| <span | |
| aria-hidden | |
| className={cn("flex h-9 w-9 items-center justify-center", className)} | |
| className={cn("flex size-9 items-center justify-center", className)} | |
| {...props} | |
| > | |
| <MoreHorizontal className="h-4 w-4" /> | |
| <MoreHorizontal className="size-4" /> | |
| <span className="sr-only">More pages</span> | |
| </span> | |
| <span | |
| className={cn("flex size-9 items-center justify-center", className)} | |
| {...props} | |
| > | |
| <span aria-hidden="true"> | |
| <MoreHorizontal className="size-4" /> | |
| </span> | |
| <span className="sr-only">More pages</span> | |
| </span> |
🤖 Prompt for AI Agents
In apps/web/src/components/ui/pagination.tsx around lines 98 to 105, the
aria-hidden attribute is incorrectly applied to the outer span, which hides the
nested screen-reader-only text from assistive technologies. To fix this, remove
aria-hidden from the outer span and instead apply aria-hidden only to the
<MoreHorizontal> icon element. This ensures the screen-reader label remains
accessible while hiding the decorative icon.
| <ChevronDown className="size-4 opacity-50" /> | ||
| </SelectPrimitive.Icon> |
There was a problem hiding this comment.
Add aria-hidden & <title> to decorative SVG icons
All three Chevron icons are purely decorative and do not convey new information to assistive technology. Per the project accessibility rules (“Always include a title element for SVG elements” and avoid empty graphics being announced), they should either:
- Be given a meaningful
<title>or - Be hidden with
aria-hidden="true"(andfocusable="false"for IE/Edge-legacy).
Quick patch (option 2):
-<ChevronDown className="size-4 opacity-50" />
+<ChevronDown
+ className="size-4 opacity-50"
+ aria-hidden="true"
+ focusable="false"
/>Apply the same to the ChevronUp (Line 63) and ChevronDown (Line 80) instances.
Also applies to: 63-64, 80-81
🤖 Prompt for AI Agents
In apps/web/src/components/ui/select.tsx around lines 45-46, 63-64, and 80-81,
the Chevron SVG icons are decorative and lack accessibility attributes. Fix this
by adding aria-hidden="true" and focusable="false" to each Chevron icon
component to hide them from assistive technologies, ensuring compliance with
accessibility rules.
| <span className="absolute right-2 flex size-3.5 items-center justify-center"> | ||
| <SelectPrimitive.ItemIndicator> | ||
| <Check className="h-4 w-4" /> | ||
| <Check className="size-4" /> | ||
| </SelectPrimitive.ItemIndicator> | ||
| </span> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Resize or hide the check-mark icon & comply with a11y rules
- The container span is
size-3.5but theCheckicon issize-4, causing a 2 px overflow that can mis-align the glyph on some zoom factors. - As with the chevron icons, add either a
<title>oraria-hidden="true"to keep screen readers silent; the selected state is already conveyed viaaria-selectedon the item.
Suggested diff:
-<span className="absolute right-2 flex size-3.5 items-center justify-center">
+<span className="absolute right-2 flex size-4 items-center justify-center">
<SelectPrimitive.ItemIndicator>
- <Check className="size-4" />
+ <Check className="size-3.5" aria-hidden="true" focusable="false" />
</SelectPrimitive.ItemIndicator>
</span>📝 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.
| <span className="absolute right-2 flex size-3.5 items-center justify-center"> | |
| <SelectPrimitive.ItemIndicator> | |
| <Check className="h-4 w-4" /> | |
| <Check className="size-4" /> | |
| </SelectPrimitive.ItemIndicator> | |
| </span> | |
| <span className="absolute right-2 flex size-4 items-center justify-center"> | |
| <SelectPrimitive.ItemIndicator> | |
| <Check className="size-3.5" aria-hidden="true" focusable="false" /> | |
| </SelectPrimitive.ItemIndicator> | |
| </span> |
🤖 Prompt for AI Agents
In apps/web/src/components/ui/select.tsx around lines 145 to 149, the Check icon
inside the span container overflows by 2px due to size mismatch and lacks
accessibility attributes. Fix this by resizing the Check icon to match the
container size (size-3.5) to prevent overflow and add aria-hidden="true" to the
Check icon to hide it from screen readers, since the selected state is already
announced via aria-selected on the item.
| // Get the newly added audio item | ||
| const updatedMediaItems = useMediaStore.getState().mediaItems; | ||
| const newAudioMediaItem = updatedMediaItems.find(item => | ||
| item.name === audioFileName && item.type === "audio" | ||
| ); | ||
|
|
||
| if (!newAudioMediaItem) { | ||
| throw new Error("Failed to add extracted audio to media library"); | ||
| } |
There was a problem hiding this comment.
Fragile media item lookup by name.
Finding the newly added audio item by name could fail if there are name conflicts or race conditions. Consider having addMediaItem return the new item or its ID.
Instead of searching for the item by name:
- // Get the newly added audio item
- const updatedMediaItems = useMediaStore.getState().mediaItems;
- const newAudioMediaItem = updatedMediaItems.find(item =>
- item.name === audioFileName && item.type === "audio"
- );
+ // addMediaItem should return the new media item
+ const newAudioMediaItem = await addMediaItem(activeProject.id, {
+ name: audioFileName,
+ type: "audio" as const,
+ file: audioFile,
+ url: URL.createObjectURL(audioFile),
+ duration: mediaItem.duration,
+ });Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/src/stores/timeline-store.ts around lines 1154 to 1162, the code
looks up the newly added audio media item by matching its name, which is fragile
due to possible name conflicts or race conditions. To fix this, modify the
addMediaItem function to return the newly added media item or its unique ID
directly, then use that returned value instead of searching by name. This
ensures reliable retrieval of the correct media item without depending on name
matching.
genmnz
changed the title
genmnz
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
apps/web/public/landing-page-bg.pngis excluded by!**/*.pngapps/web/public/landing-page-dark.pngis excluded by!**/*.pngapps/web/public/landing-page-light.pngis excluded by!**/*.png
📒 Files selected for processing (8)
apps/web/src/components/editor/media-panel/tabbar.tsx(1 hunks)apps/web/src/components/editor/timeline/index.tsx(3 hunks)apps/web/src/components/header.tsx(2 hunks)apps/web/src/components/landing/handlebars.tsx(2 hunks)apps/web/src/components/landing/hero.tsx(1 hunks)apps/web/src/components/ui/sponsor-button.tsx(1 hunks)apps/web/src/components/ui/tooltip.tsx(1 hunks)apps/web/src/data/colors/pattern-craft.ts(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- apps/web/src/components/landing/handlebars.tsx
- apps/web/src/components/ui/sponsor-button.tsx
- apps/web/src/data/colors/pattern-craft.ts
- apps/web/src/components/editor/timeline/index.tsx
- apps/web/src/components/editor/media-panel/tabbar.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/src/components/landing/hero.tsx
- apps/web/src/components/header.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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/ui/tooltip.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.{js,jsx,ts,tsx}: Don't use consecutive spaces in regular expression literals.
Don't use theargumentsobject.
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 for loops when you don't need initializer and update expressions.
Don't reassign const variables....
Files:
apps/web/src/components/ui/tooltip.tsx
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Don't use primitive type aliases or misleading types.
Don't use empty type parameters in type aliases and interfaces.
Don't use any or unknown as type constraints.
Don't return a value from a function with the return type 'void'.
Don't use the TypeScript directive @ts-ignore.
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.
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 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...
Files:
apps/web/src/components/ui/tooltip.tsx
🧠 Learnings (10)
📚 Learning: applies to **/*.{ts,tsx} : use consistent accessibility modifiers on class properties and methods....
Learnt from: CR
PR: OpenCut-app/OpenCut#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-27T22:14:46.402Z
Learning: Applies to **/*.{ts,tsx} : Use consistent accessibility modifiers on class properties and methods.
Applied to files:
apps/web/src/components/ui/tooltip.tsx
📚 Learning: applies to **/*.{jsx,tsx} : don't assign interactive aria roles to non-interactive html elements....
Learnt from: CR
PR: OpenCut-app/OpenCut#0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-07-27T22:15:27.748Z
Learning: Applies to **/*.{jsx,tsx} : Don't assign interactive ARIA roles to non-interactive HTML elements.
Applied to files:
apps/web/src/components/ui/tooltip.tsx
📚 Learning: applies to **/*.{jsx,tsx} : use semantic elements instead of role attributes in jsx....
Learnt from: CR
PR: OpenCut-app/OpenCut#0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-07-27T22:15:27.748Z
Learning: Applies to **/*.{jsx,tsx} : Use semantic elements instead of role attributes in JSX.
Applied to files:
apps/web/src/components/ui/tooltip.tsx
📚 Learning: applies to **/*.{jsx,tsx} : use valid, non-abstract aria roles for elements with aria roles....
Learnt from: CR
PR: OpenCut-app/OpenCut#0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-07-27T22:15:27.748Z
Learning: Applies to **/*.{jsx,tsx} : Use valid, non-abstract ARIA roles for elements with ARIA roles.
Applied to files:
apps/web/src/components/ui/tooltip.tsx
📚 Learning: applies to **/*.{jsx,tsx} : don't assign non-interactive aria roles to interactive html elements....
Learnt from: CR
PR: OpenCut-app/OpenCut#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-27T22:14:46.402Z
Learning: Applies to **/*.{jsx,tsx} : Don't assign non-interactive ARIA roles to interactive HTML elements.
Applied to files:
apps/web/src/components/ui/tooltip.tsx
📚 Learning: applies to **/*.{jsx,tsx} : don't use explicit role property that's the same as the implicit/default...
Learnt from: CR
PR: OpenCut-app/OpenCut#0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-07-27T22:15:27.748Z
Learning: Applies to **/*.{jsx,tsx} : Don't use explicit role property that's the same as the implicit/default role.
Applied to files:
apps/web/src/components/ui/tooltip.tsx
📚 Learning: applies to **/*.{jsx,tsx} : don't use dangerous jsx props....
Learnt from: CR
PR: OpenCut-app/OpenCut#0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-07-27T22:15:27.748Z
Learning: Applies to **/*.{jsx,tsx} : Don't use dangerous JSX props.
Applied to files:
apps/web/src/components/ui/tooltip.tsx
📚 Learning: applies to **/*.{jsx,tsx} : don't define react components inside other components....
Learnt from: CR
PR: OpenCut-app/OpenCut#0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-07-27T22:15:27.748Z
Learning: Applies to **/*.{jsx,tsx} : Don't define React components inside other components.
Applied to files:
apps/web/src/components/ui/tooltip.tsx
📚 Learning: applies to **/*.{jsx,tsx} : don't assign to react component props....
Learnt from: CR
PR: OpenCut-app/OpenCut#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-27T22:14:46.402Z
Learning: Applies to **/*.{jsx,tsx} : Don't assign to React component props.
Applied to files:
apps/web/src/components/ui/tooltip.tsx
📚 Learning: applies to **/*.{jsx,tsx} : accompany `onmouseover`/`onmouseout` with `onfocus`/`onblur`....
Learnt from: CR
PR: OpenCut-app/OpenCut#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-27T22:14:46.402Z
Learning: Applies to **/*.{jsx,tsx} : Accompany `onMouseOver`/`onMouseOut` with `onFocus`/`onBlur`.
Applied to files:
apps/web/src/components/ui/tooltip.tsx
🧬 Code Graph Analysis (1)
apps/web/src/components/ui/tooltip.tsx (1)
apps/web/src/lib/utils.ts (1)
cn(6-8)
🔇 Additional comments (4)
apps/web/src/components/ui/tooltip.tsx (4)
1-5: LGTM!The import statements are well-organized and follow the coding guidelines. The use of
class-variance-authorityfor variant management is a good pattern for maintainable styling.
13-39: Well-structured variant system.The
tooltipVariantsconfiguration provides a comprehensive set of semantic variants with consistent styling patterns. The use of arbitrary values like[border-width:0.5px]and proper dark mode support shows attention to design details.
41-43: LGTM!The interface properly extends both the Radix UI component props and variant props, providing full type safety for the component.
72-72: LGTM!The export statement properly exposes all necessary tooltip components with clear, semantic names.
| {variant === 'sidebar' && ( | ||
| <svg | ||
| width="6" | ||
| height="10" | ||
| viewBox="0 0 6 10" | ||
| fill="none" | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| className="absolute left-[-6px] top-1/2 -translate-y-1/2" | ||
| > | ||
| <path d="M6 0L0 5L6 10V0Z" className="fill-white/80 dark:fill-[#413F3E]" /> | ||
| </svg> | ||
| )} |
There was a problem hiding this comment.
Add SVG title element for accessibility.
The SVG arrow is missing a title element, which violates accessibility guidelines for screen readers.
Apply this diff to add the required title element:
<svg
width="6"
height="10"
viewBox="0 0 6 10"
fill="none"
xmlns="http://www.w3.org/2000/svg"
className="absolute left-[-6px] top-1/2 -translate-y-1/2"
>
+ <title>Tooltip arrow</title>
<path d="M6 0L0 5L6 10V0Z" className="fill-white/80 dark:fill-[#413F3E]" />
</svg>📝 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.
| {variant === 'sidebar' && ( | |
| <svg | |
| width="6" | |
| height="10" | |
| viewBox="0 0 6 10" | |
| fill="none" | |
| xmlns="http://www.w3.org/2000/svg" | |
| className="absolute left-[-6px] top-1/2 -translate-y-1/2" | |
| > | |
| <path d="M6 0L0 5L6 10V0Z" className="fill-white/80 dark:fill-[#413F3E]" /> | |
| </svg> | |
| )} | |
| {variant === 'sidebar' && ( | |
| <svg | |
| width="6" | |
| height="10" | |
| viewBox="0 0 6 10" | |
| fill="none" | |
| xmlns="http://www.w3.org/2000/svg" | |
| className="absolute left-[-6px] top-1/2 -translate-y-1/2" | |
| > | |
| + <title>Tooltip arrow</title> | |
| <path d="M6 0L0 5L6 10V0Z" className="fill-white/80 dark:fill-[#413F3E]" /> | |
| </svg> | |
| )} |
🤖 Prompt for AI Agents
In apps/web/src/components/ui/tooltip.tsx around lines 55 to 66, the SVG element
lacks a title element, which is necessary for accessibility to provide screen
readers with a description. Add a <title> element inside the SVG with a
descriptive text like "Sidebar arrow" to improve accessibility compliance.
|
@mazeincoding hello? |
3 participants
Description
LOOK SCREENSHOTS SECTION
Fixes # (issue)
Type of change
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 (if applicable)
BookMarks management, quickly move between or delete them
Two free modes, 1 select mode (normal one, all the time) or split mode (splits elements) freely unless switched back to normal
Split mode selected, all elements are a bit highlighted and anywhere clicked is a split "even out of the playhead's area" splits freely as it should be
Duplicate Projects, with everything within, media, marks, data... works from inside the project and from the /projects area (wasn't working before)
Extract Audio, extracts the stream audio from a video media element and insert it right below it.
Add screenshots to help explain your changes.
Checklist:
Summary by CodeRabbit
New Features
Style
UI Improvements
Chores