-
Notifications
You must be signed in to change notification settings - Fork 4.4k
Feat/keybinding and drag drop #674
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Feat/keybinding and drag drop #674
Conversation
|
@tusharchawre 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
|
📝 WalkthroughWalkthroughThis PR implements keyboard and drag-and-drop interactions for the timeline editor, adding Ctrl+X deletion keybinding, moving elements between tracks via drag-and-drop with visual overlay feedback, tracking track creation timestamps, and updating Docker build configuration. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 (2)
apps/web/src/components/editor/timeline/timeline-track.tsx (2)
866-873: Remove debug logging from production code.This
console.logstatement appears to be debug logging that should be removed before merging. The coding guidelines state "Don't use console."🔎 Proposed fix
const handleTrackDrop = (e: React.DragEvent) => { e.preventDefault(); e.stopPropagation(); - // Debug logging - console.log( - JSON.stringify({ - message: "Drop event started in timeline track", - dataTransferTypes: Array.from(e.dataTransfer.types), - trackId: track.id, - trackType: track.type, - }) - ); - // Reset all drag states dragCounterRef.current = 0;
572-588: Add missing dependencies to useEffect hook.The effect uses
rippleEditingEnabledandsnappingEnableddirectly in conditional branches (lines 542, 83, 187), but neither are included in the dependency array. Additionally,track.elementsis accessed for overlap checking (line 525). Add these to the dependency array to ensure the effect re-runs when they change.
🧹 Nitpick comments (2)
apps/web/src/stores/keybindings-store.ts (1)
169-179: Type the merge function parameters instead of usingany.The
anytype violates the coding guidelines. Consider typing the parameters properly:🔎 Proposed fix
- merge: (persistedState: any, currentState: any) => { + merge: (persistedState: unknown, currentState: KeybindingsState) => { + const persisted = persistedState as Partial<KeybindingsState> | undefined; // Merge defaults with persisted state to ensure new keybindings are included return { ...currentState, - ...persistedState, + ...persisted, keybindings: { ...defaultKeybindings, - ...persistedState?.keybindings, + ...persisted?.keybindings, }, }; },apps/web/src/components/editor/timeline/timeline-track.tsx (1)
278-299: Consider extracting duplicated time calculation logic.The time calculation (getting mouseX, computing mouseTime, applying frame snapping) is duplicated here and in lines 180-183 within the same function, and again in handleMouseUp (lines 396-420). Consider extracting this to a helper function to reduce duplication and maintenance burden.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.dockerignoreapps/web/src/components/editor/timeline/timeline-element.tsxapps/web/src/components/editor/timeline/timeline-track.tsxapps/web/src/stores/keybindings-store.tsapps/web/src/stores/timeline-store.tsapps/web/src/types/timeline.ts
🧰 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/editor/timeline/timeline-element.tsxapps/web/src/components/editor/timeline/timeline-track.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/editor/timeline/timeline-element.tsxapps/web/src/types/timeline.tsapps/web/src/components/editor/timeline/timeline-track.tsxapps/web/src/stores/keybindings-store.tsapps/web/src/stores/timeline-store.ts
**/*.{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/editor/timeline/timeline-element.tsxapps/web/src/types/timeline.tsapps/web/src/components/editor/timeline/timeline-track.tsxapps/web/src/stores/keybindings-store.tsapps/web/src/stores/timeline-store.ts
🧠 Learnings (11)
📚 Learning: 2025-07-27T22:14:46.402Z
Learnt from: CR
Repo: OpenCut-app/OpenCut PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-27T22:14:46.402Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Don't export empty modules that don't change anything.
Applied to files:
.dockerignore
📚 Learning: 2025-07-27T22:14:46.402Z
Learnt from: CR
Repo: OpenCut-app/OpenCut PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-27T22:14:46.402Z
Learning: Applies to **/*.{ts,tsx} : Don't use the TypeScript directive ts-ignore.
Applied to files:
.dockerignore
📚 Learning: 2025-07-27T22:15:27.748Z
Learnt from: CR
Repo: OpenCut-app/OpenCut PR: 0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-07-27T22:15:27.748Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Don't import next/document outside of pages/_document.jsx in Next.js projects.
Applied to files:
.dockerignore
📚 Learning: 2025-07-27T22:14:46.402Z
Learnt from: CR
Repo: OpenCut-app/OpenCut PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-27T22:14:46.402Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Don't use __dirname and __filename in the global scope.
Applied to files:
.dockerignore
📚 Learning: 2025-07-27T22:14:46.402Z
Learnt from: CR
Repo: OpenCut-app/OpenCut PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-27T22:14:46.402Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Don't use configured elements.
Applied to files:
.dockerignore
📚 Learning: 2025-07-27T22:15:27.748Z
Learnt from: CR
Repo: OpenCut-app/OpenCut PR: 0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-07-27T22:15:27.748Z
Learning: Applies to **/*.{test,spec}.{js,jsx,ts,tsx} : Don't use export or module.exports in test files.
Applied to files:
.dockerignore
📚 Learning: 2025-07-27T22:14:46.403Z
Learnt from: CR
Repo: OpenCut-app/OpenCut PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-27T22:14:46.403Z
Learning: Applies to **/*.test.{js,jsx,ts,tsx} : Don't use disabled tests.
Applied to files:
.dockerignore
📚 Learning: 2025-07-27T22:14:46.403Z
Learnt from: CR
Repo: OpenCut-app/OpenCut PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-27T22:14:46.403Z
Learning: Applies to **/*.test.{js,jsx,ts,tsx} : Don't use export or module.exports in test files.
Applied to files:
.dockerignore
📚 Learning: 2025-07-27T22:14:46.403Z
Learnt from: CR
Repo: OpenCut-app/OpenCut PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-27T22:14:46.403Z
Learning: Applies to **/*.{jsx,tsx} : Don't use `<head>` elements in Next.js projects.
Applied to files:
.dockerignore
📚 Learning: 2025-07-27T22:14:46.402Z
Learnt from: CR
Repo: OpenCut-app/OpenCut PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-27T22:14:46.402Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Use Date.now() to get milliseconds since the Unix Epoch.
Applied to files:
apps/web/src/stores/timeline-store.ts
📚 Learning: 2025-07-27T22:15:27.748Z
Learnt from: CR
Repo: OpenCut-app/OpenCut PR: 0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-07-27T22:15:27.748Z
Learning: Applies to **/*.{jsx,tsx} : Include caption tracks for audio and video elements.
Applied to files:
apps/web/src/stores/timeline-store.ts
🧬 Code graph analysis (2)
apps/web/src/components/editor/timeline/timeline-track.tsx (2)
apps/web/src/constants/timeline-constants.ts (2)
TIMELINE_CONSTANTS(72-79)snapTimeToFrame(99-103)apps/web/src/types/timeline.ts (1)
canElementGoOnTrack(139-150)
apps/web/src/stores/timeline-store.ts (2)
apps/web/src/types/timeline.ts (1)
TimelineTrack(83-91)apps/web/src/lib/utils.ts (1)
generateUUID(18-49)
🔇 Additional comments (10)
.dockerignore (1)
1-43: LGTM! Clean Docker build configuration.The
.dockerignorefile is well-organized and follows Docker best practices. The categorized exclusions (dependencies, build outputs, environment files, IDE config, OS files, logs, test artifacts, and markdown) are comprehensive and appropriate for a Next.js/Node.js project. The README.md exception ensures documentation is preserved in the image.apps/web/src/components/editor/timeline/timeline-element.tsx (1)
343-348: LGTM!This is a formatting-only change to the nested ternary expression. The logic for determining the mute/show label text remains unchanged.
apps/web/src/stores/timeline-store.ts (3)
394-394: LGTM!Good addition of
createdAt: Date.now()for track creation timestamp. This is consistent with the learnings and the TimelineTrack interface update. The timestamp is correctly added across all track creation sites.
419-419: LGTM!Consistent with the
addTrackimplementation, properly usingDate.now()per coding guidelines.
1019-1019: LGTM!The
createdAttimestamp is correctly added to the new audio track created during audio separation.apps/web/src/types/timeline.ts (2)
90-90: LGTM!Good addition of the optional
createdAtfield with a helpful inline comment explaining its purpose for race condition prevention.
130-130: LGTM!Correctly populates
createdAtusingDate.now()when creating the main track.apps/web/src/components/editor/timeline/timeline-track.tsx (3)
136-148: LGTM!The drag overlay state and cleanup effect are well-structured. The effect correctly clears the overlay when dragging ends, preventing stale visual state.
487-494: Verify non-null assertions are safe.The non-null assertions on
dragState.elementId!(lines 487, 493) are used after an early return guard on line 324. While the guard does check!dragState.elementId, TypeScript may not narrow the type through the complex control flow. The assertions appear safe given the guard, but consider storing the value in a const after the guard to avoid the assertions.🔎 Suggested improvement
const handleMouseUp = (e: MouseEvent) => { - if (!dragState.elementId || !dragState.trackId || !dragState.isDragging) { + const { elementId, trackId: dragTrackId, isDragging } = dragState; + if (!elementId || !dragTrackId || !isDragging) { return; } + // elementId and dragTrackId are now guaranteed non-null // ... later in the function use elementId instead of dragState.elementId!
1303-1325: LGTM!The drag overlay rendering is well-implemented with appropriate styling (dashed border, transparent background) and correct positioning calculations using the timeline constants and zoom level.
| "ctrl+z": "undo", | ||
| "ctrl+shift+z": "redo", | ||
| "ctrl+y": "redo", | ||
| "ctrl+x": "delete-selected", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider the conventional meaning of Ctrl+X.
Ctrl+X is conventionally the "cut" shortcut (copy to clipboard + delete), not just "delete". This mapping to delete-selected may confuse users who expect cut behavior. Consider either:
- Mapping Ctrl+X to a "cut-selected" action (copy + delete)
- Using a different shortcut for delete-only (e.g., just Delete/Backspace, which are already mapped)
🤖 Prompt for AI Agents
In apps/web/src/stores/keybindings-store.ts around line 31, the keybinding maps
"ctrl+x" to a delete-only action which conflicts with the conventional Cut
behavior; update the mapping to "cut-selected" (and ensure a corresponding
cut-selected handler exists that performs copy-to-clipboard then deletion) or
remove the "ctrl+x" entry so Delete/Backspace remain the delete-only keys—pick
one of these fixes and update any handler registrations/tests accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR enhances timeline usability by introducing a keyboard shortcut for element deletion and improving drag-and-drop interactions for moving elements between tracks. The changes add visual feedback through a drag overlay and refactor the mouse event handling logic to provide clearer drop zones.
- Adds
Ctrl+Xkeybinding mapped to "delete-selected" action - Implements visual drag overlay showing where elements will be dropped when dragging across tracks
- Refactors drag-drop logic with improved bounds checking and clearer handling of edge cases
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
.dockerignore |
Adds Docker ignore patterns for build optimization (unrelated to PR scope) |
apps/web/src/types/timeline.ts |
Adds optional createdAt timestamp field to TimelineTrack interface |
apps/web/src/stores/timeline-store.ts |
Initializes createdAt field when creating tracks; includes formatting fixes for ternary operators |
apps/web/src/stores/keybindings-store.ts |
Adds ctrl+x keybinding for delete action, bumps storage version to 3, and adds merge function for migration |
apps/web/src/components/editor/timeline/timeline-track.tsx |
Major refactoring of drag-drop logic with new drag overlay visualization, improved track boundary detection, and better handling of cross-track element moves |
apps/web/src/components/editor/timeline/timeline-element.tsx |
Formatting fixes for ternary operator consistency |
| type, | ||
| elements: [], | ||
| muted: false, | ||
| createdAt: Date.now(), |
Copilot
AI
Dec 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The createdAt field is being set but never used. This creates unnecessary data that adds no value. Either implement the race condition prevention logic that uses this field, or remove this assignment.
| type, | ||
| elements: [], | ||
| muted: false, | ||
| createdAt: Date.now(), |
Copilot
AI
Dec 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The createdAt field is being set but never used. This creates unnecessary data that adds no value. Either implement the race condition prevention logic that uses this field, or remove this assignment.
| }, | ||
| ], | ||
| muted: false, | ||
| createdAt: Date.now(), |
Copilot
AI
Dec 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The createdAt field is being set but never used. This creates unnecessary data that adds no value. Either implement the race condition prevention logic that uses this field, or remove this assignment.
| createdAt: Date.now(), |
| elements: [], | ||
| muted: false, | ||
| isMain: true, | ||
| createdAt: Date.now(), |
Copilot
AI
Dec 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The createdAt field is being set but never used. This creates unnecessary data that adds no value. Either implement the race condition prevention logic that uses this field, or remove this assignment.
| createdAt: Date.now(), |
| // Merge defaults with persisted state to ensure new keybindings are included | ||
| return { | ||
| ...currentState, | ||
| ...persistedState, | ||
| keybindings: { | ||
| ...defaultKeybindings, | ||
| ...persistedState?.keybindings, | ||
| }, |
Copilot
AI
Dec 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The merge function doesn't preserve other persisted state properties correctly. It spreads persistedState after currentState, which means any properties in persistedState (like isCustomized, keybindingsEnabled, isRecording) will overwrite the current state defaults. This could cause issues if the persisted state has stale or incorrect values for these properties. Consider only merging the keybindings property and preserving other state properties from the persisted state if they exist.
| // Merge defaults with persisted state to ensure new keybindings are included | |
| return { | |
| ...currentState, | |
| ...persistedState, | |
| keybindings: { | |
| ...defaultKeybindings, | |
| ...persistedState?.keybindings, | |
| }, | |
| // Merge default keybindings with persisted keybindings, but | |
| // keep all other state properties from the current state. | |
| const mergedKeybindings: KeybindingConfig = { | |
| ...defaultKeybindings, | |
| ...(persistedState?.keybindings ?? {}), | |
| }; | |
| return { | |
| ...currentState, | |
| keybindings: mergedKeybindings, |
| # Dependencies | ||
| node_modules | ||
| **/node_modules | ||
|
|
||
| # Build outputs | ||
| .next | ||
| .turbo | ||
| dist | ||
| build | ||
| *.tsbuildinfo | ||
|
|
||
| # Environment files | ||
| .env | ||
| .env.local | ||
| .env.*.local | ||
|
|
||
| # Git | ||
| .git | ||
| .gitignore | ||
|
|
||
| # IDE | ||
| .vscode | ||
| .idea | ||
| *.swp | ||
| *.swo | ||
|
|
||
| # OS | ||
| .DS_Store | ||
| Thumbs.db | ||
|
|
||
| # Logs | ||
| *.log | ||
| npm-debug.log* | ||
| yarn-debug.log* | ||
| yarn-error.log* | ||
|
|
||
| # Testing | ||
| coverage | ||
| .nyc_output | ||
|
|
||
| # Misc | ||
| *.md | ||
| !README.md | ||
|
|
Copilot
AI
Dec 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The addition of .dockerignore appears unrelated to the PR's stated purpose of adding keybinding and drag-drop improvements. This file should ideally be in a separate PR focused on Docker/infrastructure improvements to maintain clear separation of concerns and make the changes easier to review and track.
| "ctrl+z": "undo", | ||
| "ctrl+shift+z": "redo", | ||
| "ctrl+y": "redo", | ||
| "ctrl+x": "delete-selected", |
Copilot
AI
Dec 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using Ctrl+X for "delete-selected" conflicts with standard OS behavior where Ctrl+X is universally recognized as "cut" (copy to clipboard and remove). This creates a confusing user experience as users will expect Ctrl+X to cut the element to the clipboard (allowing paste elsewhere), not just delete it. Consider either implementing a proper "cut" action that copies to clipboard before deleting, or using a different keybinding for delete (Delete and Backspace are already available).
| "ctrl+x": "delete-selected", |
| elements: TimelineElement[]; | ||
| muted?: boolean; | ||
| isMain?: boolean; | ||
| createdAt?: number; // Timestamp when track was created, used for race condition prevention |
Copilot
AI
Dec 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The createdAt field is added with a comment stating it's "used for race condition prevention", but it's never actually used anywhere in the codebase. This creates dead code and misleading documentation. Either implement the race condition prevention logic that uses this field, or remove it if it's not needed.
Description
This PR improves timeline usability by adding a keyboard shortcut for element deletion and clarifying the drag-and-drop interaction for moving elements across tracks.
It introduces a faster workflow for power users while resolving confusing drag-drop behavior caused by event-handling edge cases in the current track layout.
Fixes #672
Type of change
How Has This Been Tested?
Manual testing was performed to validate keyboard interactions and drag-drop behavior across different scenarios.
Tested Scenarios
Ctrl + XTest Configuration:
Screenshots (if applicable)
Before
https://github.com/user-attachments/assets/1ada13bb-fddb-48f8-9b32-344c9a780be1
After
https://github.com/user-attachments/assets/494b4e3e-8c39-430a-add1-1a1781404ecb
Checklist
Additional context
Motivation & UX Rationale
Ctrl + Xshortcut for fast element deletionThis delivers a more predictable and polished user experience without introducing architectural complexity.
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.