Skip to content

Conversation

@tusharchawre
Copy link

@tusharchawre tusharchawre commented Dec 28, 2025

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

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Performance improvement
  • Code refactoring
  • Tests

How Has This Been Tested?

Manual testing was performed to validate keyboard interactions and drag-drop behavior across different scenarios.

Tested Scenarios

  • Deleting selected timeline elements using Ctrl + X
  • Dragging elements above an existing track creates a new track
  • Visual drag overlay appears consistently
  • Multi-selection deletion works as expected
  • No regression in existing drag-and-drop functionality

Test Configuration:

  • Node version: 18.x
  • Browser: Chrome (latest)
  • Operating System: macOS

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

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have added screenshots if UI has been changed
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Additional context

Motivation & UX Rationale

  • Adds visual feedback to guide users toward the correct interaction
  • Introduces a Ctrl + X shortcut for fast element deletion

This delivers a more predictable and polished user experience without introducing architectural complexity.

Summary by CodeRabbit

  • New Features

    • Enhanced drag-and-drop experience with a visual overlay that displays exactly where dragged timeline elements will be placed upon drop, providing better visual feedback and precision.
    • Added Ctrl+X keyboard shortcut for quickly deleting selected items on the timeline.
  • Chores

    • Added Docker configuration file to optimize and streamline container image builds.

✏️ Tip: You can customize this high-level summary in your review settings.

Copilot AI review requested due to automatic review settings December 28, 2025 23:08
@vercel
Copy link

vercel bot commented Dec 28, 2025

@tusharchawre is attempting to deploy a commit to the OpenCut OSS Team on Vercel.

A member of the Team first needs to authorize it.

@netlify
Copy link

netlify bot commented Dec 28, 2025

👷 Deploy request for appcut pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit a5587d6

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 28, 2025

📝 Walkthrough

Walkthrough

This 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

Cohort / File(s) Summary
Docker Configuration
.dockerignore
New file excluding development artifacts, dependencies, build outputs, environment files, and typical non-essential directories from Docker image builds.
Timeline Element UI
apps/web/src/components/editor/timeline/timeline-element.tsx
Formatting-only change to ContextMenuItem label condition; no functional change.
Timeline Track Drag/Drop
apps/web/src/components/editor/timeline/timeline-track.tsx
Adds dragOverlay state and trackContainerRef for visualizing drop previews. Extends drag/mouse-move/mouse-up handlers to compute and display overlay position/duration with frame and element-edge snapping. Adds overlay rendering with dashed border aligned to computed drop time. Handles cross-track element moves via ripple-based updates.
Keybindings Store
apps/web/src/stores/keybindings-store.ts
Adds "ctrl+x" → "delete-selected" keybinding. Bumps store version from 2 to 3 and introduces merge function combining default keybindings with persisted state.
Timeline Store & Types
apps/web/src/stores/timeline-store.ts, apps/web/src/types/timeline.ts
Adds optional createdAt?: number field to TimelineTrack interface. Populates createdAt: Date.now() when creating new tracks in addTrack, insertTrackAt, separateAudio, and ensureMainTrack.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

  • #279: Both PRs modify timeline drag/drop handling with ripple-aware element updates and removal logic within the same drag/drop flow.
  • #266: Both PRs extend timeline drag/dragging logic and snapping behavior in the timeline track component.
  • #153: Both PRs touch the timeline track component's drag/drop and track-content rendering logic.

Suggested reviewers

  • shreyashguptas

Poem

🐰 Whiskers twitch at Ctrl+X,
Drag elements through tracks—no mix!
A dashed-line overlay so fine,
Shows where each timeline piece will align. 🎬

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Feat/keybinding and drag drop' accurately summarizes the two main features being added: keyboard shortcut and drag-and-drop improvements.
Description check ✅ Passed The pull request description is comprehensive, including a clear summary, relevant issue reference (#672), type of change selection, detailed testing scenarios, test configuration, screenshots, and a completed checklist.
Linked Issues check ✅ Passed The PR successfully implements both requirements from issue #672: adds Ctrl+X keybinding for deletion in keybindings-store.ts and implements drag-and-drop visual overlay and cross-track movement logic in timeline-track.tsx.
Out of Scope Changes check ✅ Passed All code changes are within scope: keybinding addition, drag-drop overlay logic, track timestamp metadata, and .dockerignore configuration are all aligned with improving timeline UX and supporting drag-drop functionality.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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.log statement 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 rippleEditingEnabled and snappingEnabled directly in conditional branches (lines 542, 83, 187), but neither are included in the dependency array. Additionally, track.elements is 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 using any.

The any type 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

📥 Commits

Reviewing files that changed from the base of the PR and between 0173db9 and a5587d6.

📒 Files selected for processing (6)
  • .dockerignore
  • apps/web/src/components/editor/timeline/timeline-element.tsx
  • apps/web/src/components/editor/timeline/timeline-track.tsx
  • apps/web/src/stores/keybindings-store.ts
  • apps/web/src/stores/timeline-store.ts
  • apps/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 use accessKey attribute on any HTML element.
Don't set aria-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 the scope prop 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 assign tabIndex to non-interactive HTML elements.
Don't use positive integers for tabIndex property.
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 a title element 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.
Assign tabIndex to non-interactive HTML elements with aria-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 a type attribute for button elements.
Make elements with interactive roles and handlers focusable.
Give heading elements content that's accessible to screen readers (not hidden with aria-hidden).
Always include a lang attribute on the html element.
Always include a title attribute for iframe elements.
Accompany onClick with at least one of: onKeyUp, onKeyDown, or onKeyPress.
Accompany onMouseOver/onMouseOut with onFocus/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.tsx
  • apps/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 the arguments object.
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 useless this aliasing.
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.tsx
  • apps/web/src/types/timeline.ts
  • apps/web/src/components/editor/timeline/timeline-track.tsx
  • apps/web/src/stores/keybindings-store.ts
  • apps/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.
Use as const instead of literal types and type annotations.
Use either T[] or Array<T> consistently.
Initialize each enum member value explicitly.
Use export type for types.
Use import type for 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.
Use as const instead of literal types and...

Files:

  • apps/web/src/components/editor/timeline/timeline-element.tsx
  • apps/web/src/types/timeline.ts
  • apps/web/src/components/editor/timeline/timeline-track.tsx
  • apps/web/src/stores/keybindings-store.ts
  • apps/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 .dockerignore file 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 addTrack implementation, properly using Date.now() per coding guidelines.


1019-1019: LGTM!

The createdAt timestamp 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 createdAt field with a helpful inline comment explaining its purpose for race condition prevention.


130-130: LGTM!

Correctly populates createdAt using Date.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",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

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:

  1. Mapping Ctrl+X to a "cut-selected" action (copy + delete)
  2. 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.

Copy link

Copilot AI left a 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+X keybinding 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(),
Copy link

Copilot AI Dec 28, 2025

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.

Copilot uses AI. Check for mistakes.
type,
elements: [],
muted: false,
createdAt: Date.now(),
Copy link

Copilot AI Dec 28, 2025

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.

Copilot uses AI. Check for mistakes.
},
],
muted: false,
createdAt: Date.now(),
Copy link

Copilot AI Dec 28, 2025

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.

Suggested change
createdAt: Date.now(),

Copilot uses AI. Check for mistakes.
elements: [],
muted: false,
isMain: true,
createdAt: Date.now(),
Copy link

Copilot AI Dec 28, 2025

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.

Suggested change
createdAt: Date.now(),

Copilot uses AI. Check for mistakes.
Comment on lines +170 to +177
// Merge defaults with persisted state to ensure new keybindings are included
return {
...currentState,
...persistedState,
keybindings: {
...defaultKeybindings,
...persistedState?.keybindings,
},
Copy link

Copilot AI Dec 28, 2025

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.

Suggested change
// 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,

Copilot uses AI. Check for mistakes.
Comment on lines +1 to +44
# 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

Copy link

Copilot AI Dec 28, 2025

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.

Copilot uses AI. Check for mistakes.
"ctrl+z": "undo",
"ctrl+shift+z": "redo",
"ctrl+y": "redo",
"ctrl+x": "delete-selected",
Copy link

Copilot AI Dec 28, 2025

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).

Suggested change
"ctrl+x": "delete-selected",

Copilot uses AI. Check for mistakes.
elements: TimelineElement[];
muted?: boolean;
isMain?: boolean;
createdAt?: number; // Timestamp when track was created, used for race condition prevention
Copy link

Copilot AI Dec 28, 2025

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.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[UX] Ctrl + X to delete an element in a track, and drag/drop elements to other tracks

1 participant