feat(editor): Add visual Undo/Redo buttons & Fix tooltip visibility by iknizzz1807 Β· Pull Request #661 Β· OpenCut-app/OpenCut
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
- assigned-actions-to-work-on-selected-clips and added-undo/redo-functionality #23: Implements undo/redo functionality in the timeline store; this PR adds the corresponding toolbar UI to surface and consume those methods.
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
π 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 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-toolbar.tsxapps/web/src/components/ui/tooltip.tsx
**/*.{js,jsx,ts,tsx}
π CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,ts,tsx}: Don't use consecutive spaces in regular expression literals.
Don't use theargumentsobject.
Don't use the comma operator.
Don't write functions that exceed a given Cognitive Complexity score.
Don't use unnecessary boolean casts.
Don't use unnecessary callbacks with flatMap.
Use for...of statements instead of Array.forEach.
Don't create classes that only have static members (like a static namespace).
Don't use this and super in static contexts.
Don't use unnecessary catch clauses.
Don't use unnecessary constructors.
Don't use unnecessary continue statements.
Don't export empty modules that don't change anything.
Don't use unnecessary escape sequences in regular expression literals.
Don't use unnecessary labels.
Don't use unnecessary nested block statements.
Don't rename imports, exports, and destructured assignments to the same name.
Don't use unnecessary string or template literal concatenation.
Don't use String.raw in template literals when there are no escape sequences.
Don't use useless case statements in switch statements.
Don't use ternary operators when simpler alternatives exist.
Don't use uselessthisaliasing.
Don't initialize variables to undefined.
Don't use the void operators (they're not familiar).
Use arrow functions instead of function expressions.
Use Date.now() to get milliseconds since the Unix Epoch.
Use .flatMap() instead of map().flat() when possible.
Use literal property access instead of computed property access.
Don't use parseInt() or Number.parseInt() when binary, octal, or hexadecimal literals work.
Use concise optional chaining instead of chained logical expressions.
Use regular expression literals instead of the RegExp constructor when possible.
Don't use number literal object member names that aren't base 10 or use underscore separators.
Remove redundant terms from logical expressions.
Use while loops instead of for loops when you don't need initializer and update expressions.
Don't reassign const variables....
Files:
apps/web/src/components/editor/timeline/timeline-toolbar.tsxapps/web/src/components/ui/tooltip.tsx
**/*.{ts,tsx}
π CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Don't use primitive type aliases or misleading types.
Don't use empty type parameters in type aliases and interfaces.
Don't use any or unknown as type constraints.
Don't return a value from a function with the return type 'void'.
Don't use the TypeScript directive @ts-ignore.
Don't use TypeScript enums.
Don't export imported variables.
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions.
Don't use TypeScript namespaces.
Don't use non-null assertions with the!postfix operator.
Don't use parameter properties in class constructors.
Don't use user-defined types.
Useas constinstead of literal types and type annotations.
Use eitherT[]orArray<T>consistently.
Initialize each enum member value explicitly.
Useexport typefor types.
Useimport typefor types.
Make sure all enum members are literal values.
Don't use TypeScript const enum.
Don't declare empty interfaces.
Don't let variables evolve into any type through reassignments.
Don't use the any type.
Don't misuse the non-null assertion operator (!) in TypeScript files.
Don't use implicit any type on variable declarations.
Don't merge interfaces and classes unsafely.
Don't use overload signatures that aren't next to each other.
Use the namespace keyword instead of the module keyword to declare TypeScript namespaces.
Use consistent accessibility modifiers on class properties and methods.
Use function types instead of object types with call signatures.
Don't use void type outside of generic or return types.
**/*.{ts,tsx}: Don't use TypeScript enums.
Don't export imported variables.
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions.
Don't use TypeScript namespaces.
Don't use non-null assertions with the!postfix operator.
Don't use parameter properties in class constructors.
Don't use user-defined types.
Useas constinstead of literal types and...
Files:
apps/web/src/components/editor/timeline/timeline-toolbar.tsxapps/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, andredoStackfrom 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-30classes are intentional and necessaryβthey override the Button component's defaultdisabled:opacity-50to achieve a custom opacity level (30% instead of 50%). Removing them would alter the visual appearance.Verification needed: The tooltips reference keyboard shortcuts
Ctrl+ZandCtrl+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.
Comment @coderabbitai help to get the list of available commands and usage tips.