feature: Scroll + Zoom indicators on the Timeline#409
feature: Scroll + Zoom indicators on the Timeline#409mannix-lei wants to merge 5 commits intoOpenCut-app:mainfrom
Conversation
👷 Deploy request for appcut pending review.Visit the deploys page to approve it
|
|
Someone is attempting to deploy a commit to the OpenCut OSS Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis update introduces new zoom controls and a "fit to window" feature to the timeline component, refactors scroll handling for improved robustness, and enhances accessibility with additional attributes. New constants and hooks are added for managing zoom logic, and UI elements are updated to support these features and improve usability. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TimelineZoomControl
participant TimelineToolbar
participant Timeline
participant useTimelineZoom
participant useTimelineZoomActions
User->>TimelineZoomControl: Clicks zoom in/out/reset/fit
TimelineZoomControl->>TimelineToolbar: Calls onZoomChange / onFitToWindow
TimelineToolbar->>Timeline: Updates zoomLevel or fits timeline
Timeline->>useTimelineZoom: Handles zoom logic (wheel/keyboard)
useTimelineZoom->>useTimelineZoomActions: Registers action handlers
User->>Timeline: Uses keyboard shortcuts (Ctrl + = / - / 0 / 9)
Timeline->>useTimelineZoom: Keyboard event processed
Estimated code review effort3 (~45 minutes) Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🧠 Learnings (2)📓 Common learningsapps/web/src/constants/actions.ts (1)Learnt from: simonorzel26 ✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learningsapps/web/src/constants/actions.ts (1)Learnt from: simonorzel26 ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
apps/web/src/stores/keybindings-store.ts (1)
28-31: Consider the keybinding choice for zoom-fit.The zoom keybindings are mostly standard (
ctrl+=for zoom-in,ctrl+-for zoom-out,ctrl+0for zoom-reset), butctrl+9for zoom-fit might be unconventional. Many applications usectrl+0for fit-to-window, though you've used it for zoom-reset which is also common.Consider if users might expect a different key for zoom-fit functionality.
apps/web/src/components/editor/timeline-zoom-control.tsx (1)
1-1: Remove unused importThe
useStateimport is not used in this component.-import { useState } from "react";apps/web/src/components/editor/timeline-fixed.tsx (1)
62-986: Consider splitting this large component for better maintainabilityThis component is over 1000 lines long, making it difficult to maintain and test. Consider extracting logical sections into separate components.
Suggested refactoring approach:
- Extract the toolbar (lines 585-742) into a
TimelineToolbarcomponent- Extract the drag-and-drop logic into a custom hook
useTimelineDragDrop- Extract the context menu handlers into a
useTimelineContextMenuhook- Extract the time formatting logic into utility functions
This would improve:
- Code maintainability and readability
- Testing capabilities
- Component reusability
- Developer experience
apps/web/src/components/editor/timeline.tsx (1)
587-614: Consider extractinggetScrollableViewportas a utility function.The helper function is well-implemented but could be extracted to a separate utility file for reusability across other components that need similar scroll container detection.
Consider moving this to a utilities file:
// In utils/scroll.ts export function getScrollableViewport(containerRef: React.RefObject<HTMLDivElement>): HTMLElement | null { if (!containerRef.current) return null; // First try to find Radix ScrollArea viewport const radixViewport = containerRef.current.querySelector("[data-radix-scroll-area-viewport]") as HTMLElement; if (radixViewport) return radixViewport; // Fallback to the container itself if it's scrollable const container = containerRef.current; const hasOverflow = container && ( container.style.overflow === 'auto' || container.style.overflow === 'scroll' || // ... rest of the checks ); return hasOverflow ? container : null; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/web/src/components/editor/timeline-clean.tsx(1 hunks)apps/web/src/components/editor/timeline-fixed.tsx(1 hunks)apps/web/src/components/editor/timeline-zoom-control.tsx(1 hunks)apps/web/src/components/editor/timeline.tsx(12 hunks)apps/web/src/constants/actions.ts(1 hunks)apps/web/src/constants/timeline-constants.ts(1 hunks)apps/web/src/hooks/use-keyboard-shortcuts-help.ts(1 hunks)apps/web/src/hooks/use-timeline-zoom-actions.ts(1 hunks)apps/web/src/hooks/use-timeline-zoom.ts(2 hunks)apps/web/src/stores/keybindings-store.ts(1 hunks)apps/web/src/stores/timeline-store.ts(2 hunks)apps/web/src/types/keybinding.ts(1 hunks)
🧠 Learnings (10)
📓 Common learnings
Learnt from: simonorzel26
PR: OpenCut-app/OpenCut#324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.929Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded `ml-48` class in addition to the calculated `leftPosition` for proper alignment. This is intentional and needed for the new timeline layout.
apps/web/src/constants/actions.ts (1)
Learnt from: simonorzel26
PR: #324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.929Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded ml-48 class in addition to the calculated leftPosition for proper alignment. This is intentional and needed for the new timeline layout.
apps/web/src/hooks/use-timeline-zoom-actions.ts (1)
Learnt from: simonorzel26
PR: #324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.929Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded ml-48 class in addition to the calculated leftPosition for proper alignment. This is intentional and needed for the new timeline layout.
apps/web/src/constants/timeline-constants.ts (1)
Learnt from: simonorzel26
PR: #324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.929Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded ml-48 class in addition to the calculated leftPosition for proper alignment. This is intentional and needed for the new timeline layout.
apps/web/src/stores/timeline-store.ts (1)
Learnt from: simonorzel26
PR: #324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.929Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded ml-48 class in addition to the calculated leftPosition for proper alignment. This is intentional and needed for the new timeline layout.
apps/web/src/components/editor/timeline-zoom-control.tsx (1)
Learnt from: simonorzel26
PR: #324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.929Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded ml-48 class in addition to the calculated leftPosition for proper alignment. This is intentional and needed for the new timeline layout.
apps/web/src/hooks/use-timeline-zoom.ts (1)
Learnt from: simonorzel26
PR: #324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.929Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded ml-48 class in addition to the calculated leftPosition for proper alignment. This is intentional and needed for the new timeline layout.
apps/web/src/components/editor/timeline-fixed.tsx (1)
Learnt from: simonorzel26
PR: #324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.929Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded ml-48 class in addition to the calculated leftPosition for proper alignment. This is intentional and needed for the new timeline layout.
apps/web/src/components/editor/timeline.tsx (1)
Learnt from: simonorzel26
PR: #324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.929Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded ml-48 class in addition to the calculated leftPosition for proper alignment. This is intentional and needed for the new timeline layout.
apps/web/src/components/editor/timeline-clean.tsx (1)
Learnt from: simonorzel26
PR: #324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.929Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded ml-48 class in addition to the calculated leftPosition for proper alignment. This is intentional and needed for the new timeline layout.
🧬 Code Graph Analysis (1)
apps/web/src/components/editor/timeline.tsx (5)
apps/web/src/hooks/use-timeline-zoom.ts (1)
useTimelineZoom(18-115)apps/web/src/hooks/use-timeline-zoom-actions.ts (1)
useTimelineZoomActions(12-38)apps/web/src/components/editor/timeline-zoom-control.tsx (1)
TimelineZoomControl(19-127)apps/web/src/components/ui/scroll-area.tsx (1)
ScrollArea(48-48)apps/web/src/components/editor/selection-box.tsx (1)
SelectionBox(12-58)
🪛 GitHub Actions: Bun CI
apps/web/src/components/editor/timeline-fixed.tsx
[error] 263-263: Type error: Block-scoped variable 'handleDragOver' used before its declaration.
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: simonorzel26
PR: OpenCut-app/OpenCut#324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.929Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded `ml-48` class in addition to the calculated `leftPosition` for proper alignment. This is intentional and needed for the new timeline layout.
apps/web/src/constants/actions.ts (1)
Learnt from: simonorzel26
PR: #324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.929Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded ml-48 class in addition to the calculated leftPosition for proper alignment. This is intentional and needed for the new timeline layout.
apps/web/src/hooks/use-timeline-zoom-actions.ts (1)
Learnt from: simonorzel26
PR: #324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.929Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded ml-48 class in addition to the calculated leftPosition for proper alignment. This is intentional and needed for the new timeline layout.
apps/web/src/constants/timeline-constants.ts (1)
Learnt from: simonorzel26
PR: #324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.929Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded ml-48 class in addition to the calculated leftPosition for proper alignment. This is intentional and needed for the new timeline layout.
apps/web/src/stores/timeline-store.ts (1)
Learnt from: simonorzel26
PR: #324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.929Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded ml-48 class in addition to the calculated leftPosition for proper alignment. This is intentional and needed for the new timeline layout.
apps/web/src/components/editor/timeline-zoom-control.tsx (1)
Learnt from: simonorzel26
PR: #324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.929Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded ml-48 class in addition to the calculated leftPosition for proper alignment. This is intentional and needed for the new timeline layout.
apps/web/src/hooks/use-timeline-zoom.ts (1)
Learnt from: simonorzel26
PR: #324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.929Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded ml-48 class in addition to the calculated leftPosition for proper alignment. This is intentional and needed for the new timeline layout.
apps/web/src/components/editor/timeline-fixed.tsx (1)
Learnt from: simonorzel26
PR: #324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.929Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded ml-48 class in addition to the calculated leftPosition for proper alignment. This is intentional and needed for the new timeline layout.
apps/web/src/components/editor/timeline.tsx (1)
Learnt from: simonorzel26
PR: #324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.929Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded ml-48 class in addition to the calculated leftPosition for proper alignment. This is intentional and needed for the new timeline layout.
apps/web/src/components/editor/timeline-clean.tsx (1)
Learnt from: simonorzel26
PR: #324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.929Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded ml-48 class in addition to the calculated leftPosition for proper alignment. This is intentional and needed for the new timeline layout.
🧬 Code Graph Analysis (1)
apps/web/src/components/editor/timeline.tsx (5)
apps/web/src/hooks/use-timeline-zoom.ts (1)
useTimelineZoom(18-115)apps/web/src/hooks/use-timeline-zoom-actions.ts (1)
useTimelineZoomActions(12-38)apps/web/src/components/editor/timeline-zoom-control.tsx (1)
TimelineZoomControl(19-127)apps/web/src/components/ui/scroll-area.tsx (1)
ScrollArea(48-48)apps/web/src/components/editor/selection-box.tsx (1)
SelectionBox(12-58)
🪛 GitHub Actions: Bun CI
apps/web/src/components/editor/timeline-fixed.tsx
[error] 263-263: Type error: Block-scoped variable 'handleDragOver' used before its declaration.
🔇 Additional comments (13)
apps/web/src/constants/timeline-constants.ts (2)
78-81: LGTM! The zoom level constants address the PR objective effectively.The updated
MIN_ZOOMvalue from 0.25 to 0.3 and the addition ofMAX_ZOOMat 5 provide consistent boundaries. TheZOOM_LEVELSarray progression is logical and covers a good range of zoom options for timeline editing.
83-85: Good practice: Individual constant exports improve modularity.Exporting
MIN_ZOOM,MAX_ZOOM, andZOOM_LEVELSindividually makes them easier to import in other modules without importing the entireTIMELINE_CONSTANTSobject.apps/web/src/types/keybinding.ts (1)
23-23: LGTM! Key additions support zoom functionality.Adding
"+","-", and"="to theKeytype union is necessary to support the new zoom keyboard shortcuts (ctrl+=,ctrl+-,ctrl+0).apps/web/src/hooks/use-keyboard-shortcuts-help.ts (1)
63-66: LGTM! Clear and consistent action descriptions.The zoom action descriptions are well-written and properly categorized under "View". The descriptions follow the existing pattern and will provide clear guidance to users in the help system.
apps/web/src/constants/actions.ts (1)
47-50: LGTM! Well-defined zoom actions.The four new zoom actions (
zoom-in,zoom-out,zoom-reset,zoom-fit) are clearly named, properly documented, and follow the existing action naming conventions. They integrate well with the keybindings and help system changes.apps/web/src/stores/timeline-store.ts (1)
1162-1179: Well-implemented zoom calculation methods!The new methods properly use
TIMELINE_CONSTANTSincludingMIN_ZOOMandMAX_ZOOM, ensuring consistency across the application. ThecalculateFitToWindowZoommethod includes thoughtful edge case handling for zero duration.apps/web/src/components/editor/timeline.tsx (5)
59-60: LGTM! Zoom control imports added correctly.The new imports for
TimelineZoomControlanduseTimelineZoomActionsare properly structured and follow the project's import conventions.
109-117: Well-implemented fit-to-window functionality.The
handleFitToWindowcallback correctly calculates the optimal zoom level based on container width and properly usesuseCallbackwith the correct dependency.
158-163: Zoom actions properly integrated.The
useTimelineZoomActionshook is correctly configured with all necessary props to enable keyboard shortcuts and action-based zoom controls.
878-886: Zoom control UI properly integrated into toolbar.The
TimelineZoomControlcomponent is correctly positioned in the toolbar with appropriate visual separation and all required props.
1-1157: MIN_ZOOM Consistency VerifiedAll timeline zoom components import and reference the shared MIN_ZOOM constant from timeline-constants.ts. No hard-coded “0.3” values remain.
• apps/web/src/hooks/use-timeline-zoom.ts
• apps/web/src/hooks/use-timeline-zoom-actions.ts
• apps/web/src/components/editor/timeline-zoom-control.tsxNo further changes needed.
apps/web/src/components/editor/timeline-clean.tsx (2)
1-60: Imports properly structured for the new timeline component.The imports are well-organized and include all necessary dependencies for timeline functionality, zoom controls, and native scroll handling.
195-203: Improved mouse tracking implementation.Using
Date.now()instead ofe.timeStampprovides more consistent timing across different browsers. Good improvement over the original implementation.
mannix-lei
changed the title
mannix-lei
changed the title
mannix-lei
changed the title
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
apps/web/src/hooks/use-timeline-zoom.ts (2)
17-18: Consider implementing zoom persistence or removing unused constantThe
ZOOM_STORAGE_KEYconstant is defined but not used. If zoom persistence is planned, consider implementing it now. Otherwise, remove this unused constant to keep the code clean.If you want to implement zoom persistence:
-export function useTimelineZoom({ +export function useTimelineZoom({ containerRef, isInTimeline = false, onFitToWindow }: UseTimelineZoomProps): UseTimelineZoomReturn { - const [zoomLevel, setZoomLevel] = useState(1); + const [zoomLevel, setZoomLevel] = useState(() => { + const savedZoom = localStorage.getItem(ZOOM_STORAGE_KEY); + return savedZoom ? parseFloat(savedZoom) : 1; + }); + + // Persist zoom level changes + useEffect(() => { + localStorage.setItem(ZOOM_STORAGE_KEY, zoomLevel.toString()); + }, [zoomLevel]);
57-62: Consider using a more standard shortcut for fit-to-windowThe implementation is solid, but using '9' for fit-to-window is unconventional. Consider:
- Using a more standard shortcut like
Ctrl/Cmd+Shift+0orCtrl/Cmd+Shift+F- Adding a comment explaining why '9' was chosen
- Ensuring this shortcut is documented in the UI (tooltip/help)
- case '9': + case '0': + if (e.shiftKey) { + // Ctrl/Cmd+Shift+0 for fit to window + e.preventDefault(); + if (onFitToWindow) { + onFitToWindow(); + } + } else { + // Ctrl/Cmd+0 for reset zoom + e.preventDefault(); + setZoomLevel(1); + } + break; - e.preventDefault(); - if (onFitToWindow) { - onFitToWindow(); - } - break;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/src/components/editor-header.tsx(1 hunks)apps/web/src/components/editor/timeline-zoom-control.tsx(1 hunks)apps/web/src/components/editor/timeline/index.tsx(12 hunks)apps/web/src/constants/timeline-constants.ts(1 hunks)apps/web/src/hooks/use-timeline-zoom.ts(2 hunks)
🧠 Learnings (3)
📓 Common learnings
Learnt from: simonorzel26
PR: OpenCut-app/OpenCut#324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.961Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded `ml-48` class in addition to the calculated `leftPosition` for proper alignment. This is intentional and needed for the new timeline layout.
apps/web/src/components/editor/timeline/index.tsx (1)
Learnt from: simonorzel26
PR: #324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.961Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded ml-48 class in addition to the calculated leftPosition for proper alignment. This is intentional and needed for the new timeline layout.
apps/web/src/hooks/use-timeline-zoom.ts (1)
Learnt from: simonorzel26
PR: #324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.961Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded ml-48 class in addition to the calculated leftPosition for proper alignment. This is intentional and needed for the new timeline layout.
✅ Files skipped from review due to trivial changes (1)
- apps/web/src/components/editor-header.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/src/components/editor/timeline-zoom-control.tsx
- apps/web/src/constants/timeline-constants.ts
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: simonorzel26
PR: OpenCut-app/OpenCut#324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.961Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded `ml-48` class in addition to the calculated `leftPosition` for proper alignment. This is intentional and needed for the new timeline layout.
apps/web/src/components/editor/timeline/index.tsx (1)
Learnt from: simonorzel26
PR: #324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.961Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded ml-48 class in addition to the calculated leftPosition for proper alignment. This is intentional and needed for the new timeline layout.
apps/web/src/hooks/use-timeline-zoom.ts (1)
Learnt from: simonorzel26
PR: #324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.961Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded ml-48 class in addition to the calculated leftPosition for proper alignment. This is intentional and needed for the new timeline layout.
🔇 Additional comments (2)
apps/web/src/hooks/use-timeline-zoom.ts (1)
30-31: Good use of zoom constants!The wheel zoom handler now properly uses the imported
WHEEL_ZOOM_STEP,MIN_ZOOM, andMAX_ZOOMconstants, making the zoom behavior consistent and maintainable.apps/web/src/components/editor/timeline/index.tsx (1)
583-587: Excellent accessibility improvements!Adding ARIA attributes and tabIndex to the timeline ruler improves keyboard navigation and screen reader support. The increased height (4px → 6px) also enhances visual clarity and click target size.
1 participant
Description
Closes #418
fixed the inconsistent MIN_ZOOM value across all timeline zoom components.
add new zoom-related constants
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
Screenshots (if applicable)
2025-07-23.20.26.34.mov
Checklist:
Additional context
Add any other context about the pull request here.
Summary by CodeRabbit
New Features
Improvements