From 3a241d91120c61fd671073d99d6099f026f7bdec Mon Sep 17 00:00:00 2001
From: Maze Winther <mazewinther@gmail.com>
Date: Fri, 11 Jul 2025 00:25:07 +0200
Subject: [PATCH] feat: improve playhead

---
 .../components/editor/timeline-playhead.tsx   |  80 ++++------
 apps/web/src/components/editor/timeline.tsx   | 137 +++++++++++++-----
 2 files changed, 125 insertions(+), 92 deletions(-)

diff --git a/apps/web/src/components/editor/timeline-playhead.tsx b/apps/web/src/components/editor/timeline-playhead.tsx
index 51dab0f..4f0e20c 100644
--- a/apps/web/src/components/editor/timeline-playhead.tsx
+++ b/apps/web/src/components/editor/timeline-playhead.tsx
@@ -16,6 +16,8 @@ interface TimelinePlayheadProps {
   rulerRef: React.RefObject<HTMLDivElement>;
   rulerScrollRef: React.RefObject<HTMLDivElement>;
   tracksScrollRef: React.RefObject<HTMLDivElement>;
+  trackLabelsRef?: React.RefObject<HTMLDivElement>;
+  timelineRef: React.RefObject<HTMLDivElement>;
 }
 
 export function TimelinePlayhead({
@@ -27,6 +29,8 @@ export function TimelinePlayhead({
   rulerRef,
   rulerScrollRef,
   tracksScrollRef,
+  trackLabelsRef,
+  timelineRef,
 }: TimelinePlayheadProps) {
   const { playheadPosition, handlePlayheadMouseDown } = useTimelinePlayhead({
     currentTime,
@@ -38,64 +42,36 @@ export function TimelinePlayhead({
     tracksScrollRef,
   });
 
-  return (
-    <>
-      {/* Playhead in ruler (scrubbable) */}
-      <div
-        className="playhead absolute top-0 bottom-0 w-0.5 bg-red-500 pointer-events-auto z-50 cursor-col-resize"
-        style={{
-          left: `${playheadPosition * TIMELINE_CONSTANTS.PIXELS_PER_SECOND * zoomLevel}px`,
-        }}
-        onMouseDown={handlePlayheadMouseDown}
-      >
-        <div className="absolute top-1 left-1/2 transform -translate-x-1/2 w-3 h-3 bg-red-500 rounded-full border-2 border-white shadow-sm" />
-      </div>
-    </>
-  );
-}
+  // Use timeline container height minus a few pixels for breathing room
+  const timelineContainerHeight = timelineRef.current?.offsetHeight || 400;
+  const totalHeight = timelineContainerHeight - 8; // 8px padding from edges
 
-interface TimelinePlayheadTracksProps {
-  currentTime: number;
-  duration: number;
-  zoomLevel: number;
-  tracks: TimelineTrack[];
-  seek: (time: number) => void;
-  rulerRef: React.RefObject<HTMLDivElement>;
-  rulerScrollRef: React.RefObject<HTMLDivElement>;
-  tracksScrollRef: React.RefObject<HTMLDivElement>;
-}
-
-export function TimelinePlayheadTracks({
-  currentTime,
-  duration,
-  zoomLevel,
-  tracks,
-  seek,
-  rulerRef,
-  rulerScrollRef,
-  tracksScrollRef,
-}: TimelinePlayheadTracksProps) {
-  const { playheadPosition, handlePlayheadMouseDown } = useTimelinePlayhead({
-    currentTime,
-    duration,
-    zoomLevel,
-    seek,
-    rulerRef,
-    rulerScrollRef,
-    tracksScrollRef,
-  });
-
-  if (tracks.length === 0) return null;
+  // Get dynamic track labels width, fallback to 0 if no tracks or no ref
+  const trackLabelsWidth =
+    tracks.length > 0 && trackLabelsRef?.current
+      ? trackLabelsRef.current.offsetWidth
+      : 0;
+  const leftPosition =
+    trackLabelsWidth +
+    playheadPosition * TIMELINE_CONSTANTS.PIXELS_PER_SECOND * zoomLevel;
 
   return (
     <div
-      className="absolute top-0 w-0.5 bg-red-500 pointer-events-auto z-50 cursor-col-resize"
+      className="playhead absolute pointer-events-auto z-50"
       style={{
-        left: `${playheadPosition * TIMELINE_CONSTANTS.PIXELS_PER_SECOND * zoomLevel}px`,
-        height: `${getTotalTracksHeight(tracks)}px`,
+        left: `${leftPosition}px`,
+        top: 0,
+        height: `${totalHeight}px`,
+        width: "2px", // Slightly wider for better click target
       }}
       onMouseDown={handlePlayheadMouseDown}
-    />
+    >
+      {/* The red line spanning full height */}
+      <div className="absolute left-0 w-0.5 bg-foreground cursor-col-resize h-full" />
+
+      {/* Red dot indicator at the top (in ruler area) */}
+      <div className="absolute top-1 left-1/2 transform -translate-x-1/2 w-3 h-3 bg-foreground rounded-full border-2 border-foreground shadow-sm" />
+    </div>
   );
 }
 
@@ -108,7 +84,7 @@ export function useTimelinePlayheadRuler({
   rulerRef,
   rulerScrollRef,
   tracksScrollRef,
-}: Omit<TimelinePlayheadProps, "tracks" | "dynamicTimelineWidth">) {
+}: Omit<TimelinePlayheadProps, "tracks" | "trackLabelsRef" | "timelineRef">) {
   const { handleRulerMouseDown, isDraggingRuler } = useTimelinePlayhead({
     currentTime,
     duration,
diff --git a/apps/web/src/components/editor/timeline.tsx b/apps/web/src/components/editor/timeline.tsx
index 15c5187..e302ac1 100644
--- a/apps/web/src/components/editor/timeline.tsx
+++ b/apps/web/src/components/editor/timeline.tsx
@@ -46,7 +46,6 @@ import {
 import { TimelineTrackContent } from "./timeline-track";
 import {
   TimelinePlayhead,
-  TimelinePlayheadTracks,
   useTimelinePlayheadRuler,
 } from "./timeline-playhead";
 import type { DragData, TimelineTrack } from "@/types/timeline";
@@ -124,6 +123,7 @@ export function Timeline() {
   // Scroll synchronization and auto-scroll to playhead
   const rulerScrollRef = useRef<HTMLDivElement>(null);
   const tracksScrollRef = useRef<HTMLDivElement>(null);
+  const trackLabelsRef = useRef<HTMLDivElement>(null);
   const isUpdatingRef = useRef(false);
   const lastRulerSync = useRef(0);
   const lastTracksSync = useRef(0);
@@ -139,6 +139,77 @@ export function Timeline() {
     tracksScrollRef,
   });
 
+  // Timeline content click to seek handler
+  const handleTimelineContentClick = useCallback(
+    (e: React.MouseEvent) => {
+      // Don't seek if clicking on timeline elements, but still deselect
+      if ((e.target as HTMLElement).closest(".timeline-element")) {
+        return;
+      }
+
+      // Don't seek if clicking on playhead
+      if ((e.target as HTMLElement).closest(".playhead")) {
+        return;
+      }
+
+      // Don't seek if clicking on track labels
+      if ((e.target as HTMLElement).closest("[data-track-labels]")) {
+        clearSelectedElements();
+        return;
+      }
+
+      // Clear selected elements when clicking empty timeline area
+      clearSelectedElements();
+
+      // Determine if we're clicking in ruler or tracks area
+      const isRulerClick = (e.target as HTMLElement).closest(
+        "[data-ruler-area]"
+      );
+
+      let mouseX: number;
+      let scrollLeft = 0;
+
+      if (isRulerClick) {
+        // Calculate based on ruler position
+        const rulerContent = rulerScrollRef.current?.querySelector(
+          "[data-radix-scroll-area-viewport]"
+        ) as HTMLElement;
+        if (!rulerContent) return;
+        const rect = rulerContent.getBoundingClientRect();
+        mouseX = e.clientX - rect.left;
+        scrollLeft = rulerContent.scrollLeft;
+      } else {
+        // Calculate based on tracks content position
+        const tracksContent = tracksScrollRef.current?.querySelector(
+          "[data-radix-scroll-area-viewport]"
+        ) as HTMLElement;
+        if (!tracksContent) return;
+        const rect = tracksContent.getBoundingClientRect();
+        mouseX = e.clientX - rect.left;
+        scrollLeft = tracksContent.scrollLeft;
+      }
+
+      const time = Math.max(
+        0,
+        Math.min(
+          duration,
+          (mouseX + scrollLeft) /
+            (TIMELINE_CONSTANTS.PIXELS_PER_SECOND * zoomLevel)
+        )
+      );
+
+      seek(time);
+    },
+    [
+      duration,
+      zoomLevel,
+      seek,
+      rulerScrollRef,
+      tracksScrollRef,
+      clearSelectedElements,
+    ]
+  );
+
   // Update timeline duration when tracks change
   useEffect(() => {
     const totalDuration = getTotalDuration();
@@ -224,14 +295,6 @@ export function Timeline() {
     }
   };
 
-  // Add new click handler for deselection
-  const handleTimelineClick = (e: React.MouseEvent) => {
-    // If clicking empty area (not on an element) and not starting marquee, deselect all elements
-    if (!(e.target as HTMLElement).closest(".timeline-element")) {
-      clearSelectedElements();
-    }
-  };
-
   // Mouse move to update marquee
   useEffect(() => {
     if (!marquee || !marquee.active) return;
@@ -802,7 +865,22 @@ export function Timeline() {
       </div>
 
       {/* Timeline Container */}
-      <div className="flex-1 flex flex-col overflow-hidden" ref={timelineRef}>
+      <div
+        className="flex-1 flex flex-col overflow-hidden relative"
+        ref={timelineRef}
+      >
+        <TimelinePlayhead
+          currentTime={currentTime}
+          duration={duration}
+          zoomLevel={zoomLevel}
+          tracks={tracks}
+          seek={seek}
+          rulerRef={rulerRef}
+          rulerScrollRef={rulerScrollRef}
+          tracksScrollRef={tracksScrollRef}
+          trackLabelsRef={trackLabelsRef}
+          timelineRef={timelineRef}
+        />
         {/* Timeline Header with Ruler */}
         <div className="flex bg-panel sticky top-0 z-10">
           {/* Track Labels Header */}
@@ -817,17 +895,16 @@ export function Timeline() {
           <div
             className="flex-1 relative overflow-hidden h-4"
             onWheel={handleWheel}
+            onClick={handleTimelineContentClick}
+            data-ruler-area
           >
             <ScrollArea className="w-full" ref={rulerScrollRef}>
               <div
                 ref={rulerRef}
-                className={`relative h-4 select-none ${
-                  isDraggingRuler ? "cursor-grabbing" : "cursor-grab"
-                }`}
+                className="relative h-4 select-none cursor-pointer"
                 style={{
                   width: `${dynamicTimelineWidth}px`,
                 }}
-                onMouseDown={handleRulerMouseDown}
               >
                 {/* Time markers */}
                 {(() => {
@@ -896,18 +973,6 @@ export function Timeline() {
                     );
                   }).filter(Boolean);
                 })()}
-
-                {/* Playhead in ruler */}
-                <TimelinePlayhead
-                  currentTime={currentTime}
-                  duration={duration}
-                  zoomLevel={zoomLevel}
-                  tracks={tracks}
-                  seek={seek}
-                  rulerRef={rulerRef}
-                  rulerScrollRef={rulerScrollRef}
-                  tracksScrollRef={tracksScrollRef}
-                />
               </div>
             </ScrollArea>
           </div>
@@ -917,7 +982,11 @@ export function Timeline() {
         <div className="flex-1 flex overflow-hidden">
           {/* Track Labels */}
           {tracks.length > 0 && (
-            <div className="w-48 flex-shrink-0 border-r bg-panel-accent overflow-y-auto">
+            <div
+              ref={trackLabelsRef}
+              className="w-48 flex-shrink-0 border-r bg-panel-accent overflow-y-auto"
+              data-track-labels
+            >
               <div className="flex flex-col gap-1">
                 {tracks.map((track) => (
                   <div
@@ -943,6 +1012,7 @@ export function Timeline() {
           <div
             className="flex-1 relative overflow-hidden"
             onWheel={handleWheel}
+            onClick={handleTimelineContentClick}
           >
             <ScrollArea className="w-full h-full" ref={tracksScrollRef}>
               <div
@@ -951,7 +1021,6 @@ export function Timeline() {
                   height: `${Math.max(200, Math.min(800, getTotalTracksHeight(tracks)))}px`,
                   width: `${dynamicTimelineWidth}px`,
                 }}
-                onClick={handleTimelineClick}
                 onMouseDown={handleTimelineMouseDown}
               >
                 {tracks.length === 0 ? (
@@ -996,18 +1065,6 @@ export function Timeline() {
                         </ContextMenuContent>
                       </ContextMenu>
                     ))}
-
-                    {/* Playhead for tracks area */}
-                    <TimelinePlayheadTracks
-                      currentTime={currentTime}
-                      duration={duration}
-                      zoomLevel={zoomLevel}
-                      tracks={tracks}
-                      seek={seek}
-                      rulerRef={rulerRef}
-                      rulerScrollRef={rulerScrollRef}
-                      tracksScrollRef={tracksScrollRef}
-                    />
                   </>
                 )}
               </div>