feat: multiple UI improvements and new features #698
Conversation
- Add theme toggle to Projects page header (OpenCut-app#689) - Fix nested button HTML validation errors in header and hero (OpenCut-app#687) - Use asChild prop to render Links with button styles - Add ability to add multiple keyboard shortcuts per action (OpenCut-app#696) - New "+" button to add additional shortcuts - Replace mode only removes the specific key being edited - Right-click to remove a shortcut when multiple exist Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Add LinkSquare02Icon to grid and list views on Projects page - Icon appears on hover and opens project editor in new tab - Prevents default link navigation to allow new tab opening Co-Authored-By: Claude Opus 4.5 <[email protected]>
Validates file sizes before processing media uploads: - Image limit: 50MB - Video limit: 500MB - Audio limit: 100MB Shows clear error messages with actual file size when exceeded. Valid files still process even when some files are rejected. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Adds a "cut-selected" action that copies selected elements to clipboard and then deletes them, enabling standard cut behavior. Note: Drag/drop elements to other tracks was already implemented. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Creates a new /about page with: - Project mission and vision - Key features overview - Open source information - Community links - Call to action Updates footer to link to /about instead of GitHub README. Co-Authored-By: Claude Opus 4.5 <[email protected]>
👷 Deploy request for appcut pending review.Visit the deploys page to approve it
|
|
@nino-chavez is attempting to deploy a commit to the OpenCut OSS Team on Vercel. A member of the Team first needs to authorize it. |
|
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 new About page, refactors Button/Link composition to use Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
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/dialogs/shortcuts-dialog.tsx (1)
186-226:⚠️ Potential issue | 🟠 MajorAlign displayed keys with raw keys to avoid removing the wrong binding.
displayKeysdrops Cmd variants when Ctrl equivalents exist, butrawKeys[index]is still derived from the unfiltered list. That can remove/replace a different binding than the one shown (e.g., Cmd+Z instead of Ctrl+Z). Use a single filtered raw-key list (or pair{displayKey, rawKey}) so the mapping stays stable.🛠️ Proposed fix
- const displayKeys = shortcut.keys.filter((key: string) => { - if ( - key.includes("Cmd") && - shortcut.keys.includes(key.replace("Cmd", "Ctrl")) - ) - return false; - - return true; - }); - - // Get raw keys for remove functionality (before formatting) - const { keybindings } = useKeybindingsStore(); - const rawKeys = Object.entries(keybindings) - .filter(([, action]) => action === shortcut.action) - .map(([key]) => key as ShortcutKey); + // Get raw keys for remove functionality (before formatting) + const { keybindings } = useKeybindingsStore(); + const rawKeys = Object.entries(keybindings) + .filter(([, action]) => action === shortcut.action) + .map(([key]) => key as ShortcutKey); + const displayKeys = rawKeys.filter((key) => { + if (key.includes("Cmd") && rawKeys.includes(key.replace("Cmd", "Ctrl"))) { + return false; + } + return true; + }); @@ - const rawKey = rawKeys[index]; + const rawKey = key as ShortcutKey;
🤖 Fix all issues with AI agents
In `@apps/web/src/app/projects/page.tsx`:
- Around line 598-614: The icon "open in new tab" control inside the project
card (the button using project.id, HugeiconsIcon and LinkSquare02Icon) must be
removed from inside the main Link and converted into a sibling anchor overlay so
it's not a nested interactive element; replace the inline window.open with a
real <a> element that uses target="_blank" and rel="noopener noreferrer", add an
accessible label (aria-label) and keep a title element for the icon (or include
readable text for screen readers), and position it as a sibling overlay similar
to the checkbox overlay in both grid and list card render paths so it remains
clickable without breaking the parent Link.
In `@apps/web/src/components/editor/dialogs/shortcuts-dialog.tsx`:
- Around line 238-250: Add explicit type and accessibility attributes and expand
short parameter names: update the icon-only Button that calls onStartAdding so
it includes type="button" and aria-label="Add another shortcut", rename the
inline onClick parameter from e to event in that handler, and add title="Add
another shortcut" to the HugeiconsIcon; likewise, add type="button" to the other
Button (the one around line 282) and rename the parameters for handleClick and
handleRightClick from (e) to (event) to use full names consistently.
🧹 Nitpick comments (1)
apps/web/src/hooks/actions/use-editor-actions.ts (1)
276-301: Extract shared clipboard-building logic to avoid duplication.
This block repeats the copy-selected logic; a small helper will keep copy/cut behavior consistent and easier to maintain. As per coding guidelines, Create helper functions for multi-step operations instead of inline complex logic.♻️ Suggested refactor
export function useEditorActions() { const editor = useEditor(); const activeProject = editor.project.getActive(); const { selectedElements, setElementSelection } = useElementSelection(); const { clipboard, setClipboard, toggleSnapping } = useTimelineStore(); + + const buildClipboardItems = (elementsToCopy) => { + const elementsWithTracks = editor.timeline.getElementsWithTracks({ + elements: elementsToCopy, + }); + return elementsWithTracks.map(({ track, element }) => { + const { ...elementWithoutId } = element; + return { + trackId: track.id, + trackType: track.type, + element: elementWithoutId, + }; + }); + }; useActionHandler( "copy-selected", () => { if (selectedElements.length === 0) return; - const results = editor.timeline.getElementsWithTracks({ - elements: selectedElements, - }); - const items = results.map(({ track, element }) => { - const { ...elementWithoutId } = element; - return { - trackId: track.id, - trackType: track.type, - element: elementWithoutId, - }; - }); - - setClipboard({ items }); + const clipboardItems = buildClipboardItems(selectedElements); + setClipboard({ items: clipboardItems }); }, undefined, ); useActionHandler( "cut-selected", () => { if (selectedElements.length === 0) return; - // Copy elements to clipboard - const results = editor.timeline.getElementsWithTracks({ - elements: selectedElements, - }); - const items = results.map(({ track, element }) => { - const { ...elementWithoutId } = element; - return { - trackId: track.id, - trackType: track.type, - element: elementWithoutId, - }; - }); - setClipboard({ items }); + const clipboardItems = buildClipboardItems(selectedElements); + setClipboard({ items: clipboardItems }); // Delete the selected elements editor.timeline.deleteElements({ elements: selectedElements, });
| <div className="flex items-center gap-1.5"> | ||
| <h3 className="group-hover:text-foreground/90 line-clamp-2 text-sm leading-snug font-medium"> | ||
| {project.name} | ||
| </h3> | ||
| <button | ||
| type="button" | ||
| onClick={(e) => { | ||
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
| window.open(`/editor/${project.id}`, "_blank"); | ||
| }} | ||
| className="opacity-0 group-hover:opacity-100 transition-opacity p-0.5 hover:bg-accent rounded shrink-0" | ||
| title="Open in new tab" | ||
| > | ||
| <HugeiconsIcon icon={LinkSquare02Icon} className="size-3.5 text-muted-foreground" /> | ||
| </button> | ||
| </div> |
There was a problem hiding this comment.
Fix nested interactive elements and harden the “open in new tab” control.
The icon button sits inside a Link (both grid and list views), which is invalid HTML and problematic for accessibility. Also, window.open without noopener enables tabnabbing, and the icon-only control needs an accessible label/title. A safer pattern is to move the icon link outside the main Link and use a real anchor with target="_blank" + rel, plus aria-label and an icon title.
🔧 Proposed approach (apply to both grid + list)
- <div className="flex items-center gap-1.5">
+ <div className="flex items-center gap-1.5">
<h3 className="group-hover:text-foreground/90 line-clamp-2 text-sm leading-snug font-medium">
{project.name}
</h3>
- <button
- type="button"
- onClick={(e) => {
- e.preventDefault();
- e.stopPropagation();
- window.open(`/editor/${project.id}`, "_blank");
- }}
- className="opacity-0 group-hover:opacity-100 transition-opacity p-0.5 hover:bg-accent rounded shrink-0"
- title="Open in new tab"
- >
- <HugeiconsIcon icon={LinkSquare02Icon} className="size-3.5 text-muted-foreground" />
- </button>
</div>- <Link href={`/editor/${project.id}`} className="flex-1 min-w-0">
+ <Link href={`/editor/${project.id}`} className="flex-1 min-w-0">
{listRowContent}
</Link>
+ <Link
+ href={`/editor/${project.id}`}
+ target="_blank"
+ rel="noopener noreferrer"
+ aria-label="Open in new tab"
+ className="opacity-0 group-hover:opacity-100 transition-opacity p-0.5 hover:bg-accent rounded shrink-0"
+ >
+ <HugeiconsIcon
+ icon={LinkSquare02Icon}
+ className="size-3.5 text-muted-foreground"
+ title="Open in new tab"
+ />
+ </Link>For grid cards, place the icon link as a sibling overlay (similar to the checkbox) so it’s not nested inside the main Link.
As per coding guidelines: Don't use target="_blank" without rel="noopener"; Always include a title element for icons unless there's text beside the icon; Make sure anchors have content that's accessible to screen readers.
Also applies to: 640-656
🤖 Prompt for AI Agents
In `@apps/web/src/app/projects/page.tsx` around lines 598 - 614, The icon "open in
new tab" control inside the project card (the button using project.id,
HugeiconsIcon and LinkSquare02Icon) must be removed from inside the main Link
and converted into a sibling anchor overlay so it's not a nested interactive
element; replace the inline window.open with a real <a> element that uses
target="_blank" and rel="noopener noreferrer", add an accessible label
(aria-label) and keep a title element for the icon (or include readable text for
screen readers), and position it as a sibling overlay similar to the checkbox
overlay in both grid and list card render paths so it remains clickable without
breaking the parent Link.
| <Button | ||
| variant="outline" | ||
| size="sm" | ||
| onClick={(e) => { | ||
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
| onStartAdding(); | ||
| }} | ||
| title="Add another shortcut" | ||
| className={isRecording && recordingMode === "add" ? "ring-2 ring-primary" : ""} | ||
| > | ||
| <HugeiconsIcon icon={PlusSignIcon} className="size-3" /> | ||
| </Button> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's read the shortcuts-dialog.tsx file around the mentioned lines
wc -l apps/web/src/components/editor/dialogs/shortcuts-dialog.tsxRepository: OpenCut-app/OpenCut
Length of output: 126
🏁 Script executed:
# Read the relevant sections of the file
sed -n '230,300p' apps/web/src/components/editor/dialogs/shortcuts-dialog.tsxRepository: OpenCut-app/OpenCut
Length of output: 1453
🏁 Script executed:
# Find and examine the HugeiconsIcon component
fd HugeiconsIcon --type f -e tsx -e ts | head -20Repository: OpenCut-app/OpenCut
Length of output: 45
🏁 Script executed:
# Search for HugeiconsIcon component definition
rg "HugeiconsIcon" --type tsx --type ts -A 5 | head -100Repository: OpenCut-app/OpenCut
Length of output: 90
🏁 Script executed:
# Also search for where it's defined as a component
rg "export.*HugeiconsIcon|function HugeiconsIcon" --type tsx --type tsRepository: OpenCut-app/OpenCut
Length of output: 90
🏁 Script executed:
# Look for the import statement in the shortcuts-dialog file
head -50 apps/web/src/components/editor/dialogs/shortcuts-dialog.tsxRepository: OpenCut-app/OpenCut
Length of output: 1221
🏁 Script executed:
# Search for HugeiconsIcon usage patterns to see if title prop is ever used
rg "HugeiconsIcon" -A 3 | head -100Repository: OpenCut-app/OpenCut
Length of output: 6537
🏁 Script executed:
# Check the hugeicons package documentation or type definitions
fd "hugeicons" --type dRepository: OpenCut-app/OpenCut
Length of output: 45
🏁 Script executed:
# Look for node_modules or package files that might have hugeicons info
find . -name "package.json" -type f -exec grep -l "hugeicons" {} \;Repository: OpenCut-app/OpenCut
Length of output: 86
🏁 Script executed:
# Check the package.json for hugeicons version
grep -A 2 -B 2 "hugeicons" apps/web/package.jsonRepository: OpenCut-app/OpenCut
Length of output: 291
🏁 Script executed:
# Search for other usages of HugeiconsIcon with multiple props to understand the API better
rg "HugeiconsIcon" -B 1 -A 5 | grep -A 6 "title\|aria-label\|role"Repository: OpenCut-app/OpenCut
Length of output: 1435
🏁 Script executed:
# Check the entire shortcuts-dialog file to see full context
cat -n apps/web/src/components/editor/dialogs/shortcuts-dialog.tsx | tail -70Repository: OpenCut-app/OpenCut
Length of output: 1943
🏁 Script executed:
# Check the Button component to see if it already sets type="button"
rg "export.*Button|function Button" apps/web/src/components/ui/button.tsx -A 30 | head -50Repository: OpenCut-app/OpenCut
Length of output: 689
🏁 Script executed:
# Look for any HugeiconsIcon usage with title prop to see if it's even supported
rg "HugeiconsIcon.*title|title.*HugeiconsIcon"Repository: OpenCut-app/OpenCut
Length of output: 45
🏁 Script executed:
# Check for documentation or type info by looking at imports and how HugeiconsIcon is typed
rg "from.*@hugeicons/react" -B 2 -A 10Repository: OpenCut-app/OpenCut
Length of output: 26963
🏁 Script executed:
# Look for the Button component full implementation
cat apps/web/src/components/ui/button.tsxRepository: OpenCut-app/OpenCut
Length of output: 2206
🏁 Script executed:
# Check if there are other examples of title being used on SVG or icon components in the codebase
rg "title=" apps/web/src/components -A 1 | grep -i "icon\|svg" -B 1 -A 1Repository: OpenCut-app/OpenCut
Length of output: 472
🏁 Script executed:
# Look for HugeiconsIconProps type definition in node_modules or see if it extends common props
find . -name "*hugeicons*" -type d 2>/dev/null | head -5Repository: OpenCut-app/OpenCut
Length of output: 45
🏁 Script executed:
# Check the learnings - they mention title for icons
grep -i "title.*icon\|icon.*title" << 'EOF'
Always include a `title` element for icons unless there's text beside the icon
Always include a `title` element for SVG elements
EOFRepository: OpenCut-app/OpenCut
Length of output: 141
🏁 Script executed:
# Check if the Button's title attribute is sufficient or if icon needs separate title
rg "title=" apps/web/src/components/ui -A 2 -B 2 | head -40Repository: OpenCut-app/OpenCut
Length of output: 972
🏁 Script executed:
# Look at what HugeiconsIconProps might include
rg "HugeiconsIconProps" -A 5Repository: OpenCut-app/OpenCut
Length of output: 803
🏁 Script executed:
# Let's verify HugeiconsIcon accepts standard HTML props by checking one more usage
rg "HugeiconsIcon" apps/web/src/components/ui/spinner.tsx -A 8Repository: OpenCut-app/OpenCut
Length of output: 498
Add button type attributes, use full parameter names, and include accessible titles for icon-only buttons.
All instances need:
type="button"on Button elements (lines 238, 282)- Rename abbreviated
eparameter toevent(lines 241, 267, 273) - Add
aria-label="Add another shortcut"to the Button at line 238 - Add
title="Add another shortcut"to the HugeiconsIcon at line 249
Changes needed
At lines 238-250, add type="button" and aria-label to Button, rename parameter, and add title to icon:
<Button
+ type="button"
variant="outline"
size="sm"
- onClick={(e) => {
- e.preventDefault();
- e.stopPropagation();
+ onClick={(event) => {
+ event.preventDefault();
+ event.stopPropagation();
onStartAdding();
}}
title="Add another shortcut"
+ aria-label="Add another shortcut"
className={isRecording && recordingMode === "add" ? "ring-2 ring-primary" : ""}
>
- <HugeiconsIcon icon={PlusSignIcon} className="size-3" />
+ <HugeiconsIcon icon={PlusSignIcon} className="size-3" title="Add another shortcut" />
</Button>At lines 267-287:
- Button (line 282): add
type="button" - Handlers: rename
(e)to(event)in bothhandleClick(line 267) andhandleRightClick(line 273)
Per guidelines: "Always include a type attribute for button elements", "Never abbreviate variable and parameter names", and "Always include a title element for icons unless there's text beside the icon".
📝 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.
| <Button | |
| variant="outline" | |
| size="sm" | |
| onClick={(e) => { | |
| e.preventDefault(); | |
| e.stopPropagation(); | |
| onStartAdding(); | |
| }} | |
| title="Add another shortcut" | |
| className={isRecording && recordingMode === "add" ? "ring-2 ring-primary" : ""} | |
| > | |
| <HugeiconsIcon icon={PlusSignIcon} className="size-3" /> | |
| </Button> | |
| <Button | |
| type="button" | |
| variant="outline" | |
| size="sm" | |
| onClick={(event) => { | |
| event.preventDefault(); | |
| event.stopPropagation(); | |
| onStartAdding(); | |
| }} | |
| title="Add another shortcut" | |
| aria-label="Add another shortcut" | |
| className={isRecording && recordingMode === "add" ? "ring-2 ring-primary" : ""} | |
| > | |
| <HugeiconsIcon icon={PlusSignIcon} className="size-3" title="Add another shortcut" /> | |
| </Button> |
🤖 Prompt for AI Agents
In `@apps/web/src/components/editor/dialogs/shortcuts-dialog.tsx` around lines 238
- 250, Add explicit type and accessibility attributes and expand short parameter
names: update the icon-only Button that calls onStartAdding so it includes
type="button" and aria-label="Add another shortcut", rename the inline onClick
parameter from e to event in that handler, and add title="Add another shortcut"
to the HugeiconsIcon; likewise, add type="button" to the other Button (the one
around line 282) and rename the parameters for handleClick and handleRightClick
from (e) to (event) to use full names consistently.
…#663) The VideoCache was creating Input objects but not storing them, so input.dispose() was never called. This caused WebCodecs VideoDecoder resources to leak, leading to browser crashes during video playback. Changes: - Store Input object in VideoSinkData - Call input.dispose() on initialization errors - Call input.dispose() when clearing video from cache - Clear frame references when disposing This follows the same pattern as AudioManager which properly manages Input disposal. Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Use linear interpolation for resampling instead of nearest neighbor - Clamp output samples to [-1, 1] range to prevent distortion from overlapping audio clips exceeding valid amplitude range This should reduce audio artifacts in exported videos, especially when multiple audio sources overlap. Co-Authored-By: Claude Opus 4.5 <[email protected]>
The eye icon in the track labels was not responding to clicks because the onClick handler was placed directly on HugeiconsIcon which doesn't forward click events to the underlying SVG. Fixed by wrapping the icon in a button element. Co-Authored-By: Claude Opus 4.5 <[email protected]>
…pp#637) When a video file uses a codec not supported by WebCodecs (like H.265/HEVC from Clipchamp), show a helpful toast message suggesting to convert to H.264/MP4 format instead of failing silently. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Multiple functions were creating mediabunny Input objects without disposing them, causing WebCodecs decoder resources to leak. This led to RAM consumption spiraling until crash when importing video clips. Fixed by adding try/finally blocks to ensure input.dispose() is called: - getVideoInfo(): leaked Input when getting video metadata - generateThumbnail(): leaked Input when generating thumbnails - decodeAndMixAudioSource(): leaked Input when mixing audio Co-Authored-By: Claude Opus 4.5 <[email protected]>
1 participant
Summary
This PR includes several UI improvements and new features:
asChildprop pattern to avoid invalid nested<button>elementsRelated Issues
Closes #624, #644, #653, #672, #687, #689, #696
Test plan
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Navigation
✏️ Tip: You can customize this high-level summary in your review settings.