Skip to content

Conversation

@iknizzz1807
Copy link

@iknizzz1807 iknizzz1807 commented Dec 13, 2025

Description

This PR improves the Timeline accessibility and UX by introducing visual Undo/Redo buttons to the toolbar. Previously, these actions were restricted to keyboard shortcuts (Ctrl+Z/Ctrl+Y).

While implementing this feature, I identified a UI issue where tooltips in the timeline were being obscured by the track headers (due to conflicting z-index layering). Consequently, this PR also includes a fix for the Tooltip component to ensure it properly renders above all editor layers.

Changes included:

  1. Added Undo and Redo buttons to TimelineToolbar.
  2. Integrated logic to disable buttons when history/redo stack is empty.
  3. UI Fix: Increased Tooltip component z-index from 50 to 1000 to prevent them from being clipped by the Track Header (z-100).

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)
  • Performance improvement
  • Code refactoring

How Has This Been Tested?

  1. Functional Test:
    • Opened a project in the Editor.
    • Performed actions (move clip, split clip).
    • Verified "Undo" button becomes enabled and reverts action on click.
    • Verified "Redo" button becomes enabled and reapplies action on click.
  2. UI/Regression Test:
    • Hovered over the new buttons (located near the left edge).
    • Success: Confirmed the tooltip now renders on top of the Track Header panel without being obscured (see screenshots).

Test Configuration:

  • Node version: v18+ (Bun)
  • Browser: Chrome / Edge

Screenshots

1. New Feature: Undo/Redo Buttons

Undo/Redo Buttons

2. Bug Fix: Tooltip Layering

Before this fix, the tooltip text was cut off by the track panel.
Tooltip Z-Index Fix

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
  • My changes generate no new warnings

Summary by CodeRabbit

  • New Features

    • Added Undo and Redo buttons to the timeline editor toolbar, enabling users to reverse and restore recent actions. Buttons automatically disable when no history is available.
  • Bug Fixes

    • Improved tooltip layering to ensure proper visibility over interface elements.

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

@netlify
Copy link

netlify bot commented Dec 13, 2025

👷 Deploy request for appcut pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit d200d03

@vercel
Copy link

vercel bot commented Dec 13, 2025

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

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 13, 2025

Walkthrough

Added undo and redo buttons to the timeline toolbar, consuming store methods and history state. Buttons display disabled states when no history is available. Updated tooltip z-index layer for proper stacking order.

Changes

Cohort / File(s) Summary
Timeline Toolbar Undo/Redo Controls
apps/web/src/components/editor/timeline/timeline-toolbar.tsx
Added Undo2 and Redo2 icon imports; wired toolbar to consume undo, redo, history, and redoStack from timeline store; introduced two new icon buttons with tooltips that disable when history/redoStack is empty; added layout spacer between new controls and subsequent elements.
Tooltip Z-Index
apps/web/src/components/ui/tooltip.tsx
Increased tooltip base z-index from z-50 to z-1000 in tooltipVariants.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Verify that disabled states correctly reflect history availability (history.length === 0, redoStack.length === 0)
  • Confirm tooltip z-index change (z-1000) doesn't cause unintended stacking conflicts with other UI elements
  • Check that icon button event handlers properly invoke store undo/redo methods

Possibly related PRs

Suggested reviewers

  • shreyashguptas

Poem

🐰 Undo, redo, now shining bright,
Buttons dance in toolbar light,
One-two-three steps back and forth,
Timeline magic proves its worth!

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 (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately captures both main changes: adding visual Undo/Redo buttons and fixing tooltip visibility, matching the changeset clearly and concisely.
Description check ✅ Passed The description comprehensively covers all template sections including motivation, type of change, testing details with specific steps, test configuration, screenshots, and a completed checklist.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

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

📒 Files selected for processing (2)
  • apps/web/src/components/editor/timeline/timeline-toolbar.tsx (3 hunks)
  • apps/web/src/components/ui/tooltip.tsx (1 hunks)
🧰 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-toolbar.tsx
  • 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 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-toolbar.tsx
  • 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.
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-toolbar.tsx
  • apps/web/src/components/ui/tooltip.tsx
🧬 Code graph analysis (1)
apps/web/src/components/editor/timeline/timeline-toolbar.tsx (2)
apps/web/src/components/ui/tooltip.tsx (4)
  • TooltipProvider (72-72)
  • Tooltip (72-72)
  • TooltipTrigger (72-72)
  • TooltipContent (72-72)
apps/web/src/components/ui/button.tsx (1)
  • Button (61-61)
🔇 Additional comments (4)
apps/web/src/components/ui/tooltip.tsx (1)

14-14: LGTM! Z-index increase ensures tooltip visibility.

The z-index bump from 50 to 1000 appropriately ensures tooltips render above the Track Header (z-100) and other editor layers. Tooltips should generally be top-layer UI elements, so this change is justified.

apps/web/src/components/editor/timeline/timeline-toolbar.tsx (3)

30-31: LGTM! Icon imports are correct.

The Undo2 and Redo2 icons from lucide-react are appropriate for the undo/redo functionality.


68-71: LGTM! Store integration is correct.

Properly consuming undo, redo, history, and redoStack from the timeline store to enable undo/redo functionality and manage button states.


188-217: LGTM! Undo/Redo buttons are well-implemented.

The implementation follows the existing toolbar button pattern with proper disabled states, tooltips, and visual feedback. The disabled logic correctly checks for empty history (history.length === 0) and empty redo stack (redoStack.length === 0).

Note: The opacity-30 classes are intentional and necessary—they override the Button component's default disabled:opacity-50 to achieve a custom opacity level (30% instead of 50%). Removing them would alter the visual appearance.

Verification needed: The tooltips reference keyboard shortcuts Ctrl+Z and Ctrl+Y, but no keyboard event handlers are visible in this file. Please confirm these shortcuts are implemented elsewhere (e.g., global keyboard listeners) and are functioning correctly with the new undo/redo buttons.

Likely an incorrect or invalid review 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.

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.

1 participant