diff --git a/apps/web/src/components/editor/timeline/index.tsx b/apps/web/src/components/editor/timeline/index.tsx index bcaece891..51c9bbc8d 100644 --- a/apps/web/src/components/editor/timeline/index.tsx +++ b/apps/web/src/components/editor/timeline/index.tsx @@ -114,7 +114,11 @@ export function Timeline() { const trackLabelsRef = useRef(null); const playheadRef = useRef(null); const trackLabelsScrollRef = useRef(null); - const isUpdatingRef = useRef(false); + + // Separate sync state for horizontal and vertical scrolling to prevent race conditions + // This fixes issue #481 where shared state caused vertical scroll sync to fail + const isUpdatingHorizontalRef = useRef(false); + const isUpdatingVerticalRef = useRef(false); const lastRulerSync = useRef(0); const lastTracksSync = useRef(0); const lastVerticalSync = useRef(0); @@ -420,7 +424,9 @@ export function Timeline() { onDrop: handleDrop, }; - // --- Scroll synchronization effect --- + // --- Horizontal scroll synchronization effect --- + // Keeps the ruler and tracks content horizontally synchronized + // Separated from vertical sync to prevent race conditions (fixes issue #481) useEffect(() => { const rulerViewport = rulerScrollRef.current?.querySelector( "[data-radix-scroll-area-viewport]" @@ -428,74 +434,106 @@ export function Timeline() { const tracksViewport = tracksScrollRef.current?.querySelector( "[data-radix-scroll-area-viewport]" ) as HTMLElement; - const trackLabelsViewport = trackLabelsScrollRef.current?.querySelector( - "[data-radix-scroll-area-viewport]" - ) as HTMLElement; if (!rulerViewport || !tracksViewport) return; // Horizontal scroll synchronization between ruler and tracks const handleRulerScroll = () => { const now = Date.now(); - if (isUpdatingRef.current || now - lastRulerSync.current < 16) return; + if (isUpdatingHorizontalRef.current || now - lastRulerSync.current < 16) + return; lastRulerSync.current = now; - isUpdatingRef.current = true; - tracksViewport.scrollLeft = rulerViewport.scrollLeft; - isUpdatingRef.current = false; + isUpdatingHorizontalRef.current = true; + + try { + tracksViewport.scrollLeft = rulerViewport.scrollLeft; + } catch (error) { + console.warn("Timeline horizontal scroll sync error:", error); + } finally { + isUpdatingHorizontalRef.current = false; + } }; + const handleTracksScroll = () => { const now = Date.now(); - if (isUpdatingRef.current || now - lastTracksSync.current < 16) return; + if (isUpdatingHorizontalRef.current || now - lastTracksSync.current < 16) + return; lastTracksSync.current = now; - isUpdatingRef.current = true; - rulerViewport.scrollLeft = tracksViewport.scrollLeft; - isUpdatingRef.current = false; + isUpdatingHorizontalRef.current = true; + + try { + rulerViewport.scrollLeft = tracksViewport.scrollLeft; + } catch (error) { + console.warn("Timeline horizontal scroll sync error:", error); + } finally { + isUpdatingHorizontalRef.current = false; + } }; rulerViewport.addEventListener("scroll", handleRulerScroll); tracksViewport.addEventListener("scroll", handleTracksScroll); + return () => { + rulerViewport.removeEventListener("scroll", handleRulerScroll); + tracksViewport.removeEventListener("scroll", handleTracksScroll); + }; + }, []); + + // --- Vertical scroll synchronization effect --- + // Keeps the track labels and tracks content vertically synchronized + // Independent from horizontal sync with separate throttling (fixes issue #481) + useEffect(() => { + const tracksViewport = tracksScrollRef.current?.querySelector( + "[data-radix-scroll-area-viewport]" + ) as HTMLElement; + const trackLabelsViewport = trackLabelsScrollRef.current?.querySelector( + "[data-radix-scroll-area-viewport]" + ) as HTMLElement; + + if (!tracksViewport || !trackLabelsViewport) return; + // Vertical scroll synchronization between track labels and tracks content - if (trackLabelsViewport) { - const handleTrackLabelsScroll = () => { - const now = Date.now(); - if (isUpdatingRef.current || now - lastVerticalSync.current < 16) - return; - lastVerticalSync.current = now; - isUpdatingRef.current = true; + const handleTrackLabelsScroll = () => { + const now = Date.now(); + if (isUpdatingVerticalRef.current || now - lastVerticalSync.current < 16) + return; + lastVerticalSync.current = now; + isUpdatingVerticalRef.current = true; + + try { tracksViewport.scrollTop = trackLabelsViewport.scrollTop; - isUpdatingRef.current = false; - }; - const handleTracksVerticalScroll = () => { - const now = Date.now(); - if (isUpdatingRef.current || now - lastVerticalSync.current < 16) - return; - lastVerticalSync.current = now; - isUpdatingRef.current = true; - trackLabelsViewport.scrollTop = tracksViewport.scrollTop; - isUpdatingRef.current = false; - }; + } catch (error) { + console.warn("Timeline vertical scroll sync error:", error); + } finally { + isUpdatingVerticalRef.current = false; + } + }; - trackLabelsViewport.addEventListener("scroll", handleTrackLabelsScroll); - tracksViewport.addEventListener("scroll", handleTracksVerticalScroll); + const handleTracksVerticalScroll = () => { + const now = Date.now(); + if (isUpdatingVerticalRef.current || now - lastVerticalSync.current < 16) + return; + lastVerticalSync.current = now; + isUpdatingVerticalRef.current = true; - return () => { - rulerViewport.removeEventListener("scroll", handleRulerScroll); - tracksViewport.removeEventListener("scroll", handleTracksScroll); - trackLabelsViewport.removeEventListener( - "scroll", - handleTrackLabelsScroll - ); - tracksViewport.removeEventListener( - "scroll", - handleTracksVerticalScroll - ); - }; - } + try { + trackLabelsViewport.scrollTop = tracksViewport.scrollTop; + } catch (error) { + console.warn("Timeline vertical scroll sync error:", error); + } finally { + isUpdatingVerticalRef.current = false; + } + }; + + trackLabelsViewport.addEventListener("scroll", handleTrackLabelsScroll); + tracksViewport.addEventListener("scroll", handleTracksVerticalScroll); return () => { - rulerViewport.removeEventListener("scroll", handleRulerScroll); - tracksViewport.removeEventListener("scroll", handleTracksScroll); + trackLabelsViewport.removeEventListener( + "scroll", + handleTrackLabelsScroll + ); + tracksViewport.removeEventListener("scroll", handleTracksVerticalScroll); }; }, []); diff --git a/apps/web/src/stores/playback-store.ts b/apps/web/src/stores/playback-store.ts index 543865f99..1943db8ab 100644 --- a/apps/web/src/stores/playback-store.ts +++ b/apps/web/src/stores/playback-store.ts @@ -1,9 +1,24 @@ import { create } from "zustand"; import type { PlaybackState, PlaybackControls } from "@/types/playback"; +// Type definitions for timeline store integration +interface TimelineStoreState { + getTotalDuration: () => number; +} + +interface TimelineStore { + getState: () => TimelineStoreState | null; +} + +// Global timeline store type for avoiding circular dependencies +declare global { + var __timelineStore: TimelineStore | undefined; +} + interface PlaybackStore extends PlaybackState, PlaybackControls { setDuration: (duration: number) => void; setCurrentTime: (time: number) => void; + getEffectivePlaybackDuration: () => number; } let playbackTimer: number | null = null; @@ -20,8 +35,16 @@ const startTimer = (store: () => PlaybackStore) => { lastUpdate = now; const newTime = state.currentTime + delta * state.speed; - if (newTime >= state.duration) { - // When video completes, pause and reset playhead to start + + // AUDIO CURSOR FIX: Use effective playback duration (actual content duration) for stopping logic + // This fixes GitHub issue #490 where audio files shorter than 10 seconds would continue + // playing until the 10-second timeline minimum instead of stopping at the actual audio end. + // The getEffectivePlaybackDuration() method safely accesses the timeline store to get + // the actual content duration while falling back to timeline duration if unavailable. + const effectiveDuration = state.getEffectivePlaybackDuration(); + + if (newTime >= effectiveDuration) { + // When content completes, pause and reset playhead to start state.pause(); state.setCurrentTime(0); // Notify video elements to sync with reset @@ -131,4 +154,79 @@ export const usePlaybackStore = create((set, get) => ({ get().mute(); } }, + + /** + * Gets the effective playback duration for stopping logic. + * + * This method resolves the audio cursor bug (GitHub issue #490) by using the actual + * content duration instead of the timeline duration (which has a 10-second minimum). + * + * @returns {number} The duration at which playback should stop: + * - Actual content duration if content exists and is valid + * - Timeline duration (with 10s minimum) as fallback + * + * @example + * // For a 5-second audio file: + * // - Timeline duration: 10 seconds (UI minimum) + * // - Content duration: 5 seconds (actual audio length) + * // - Returns: 5 seconds (playback stops at audio end) + */ + getEffectivePlaybackDuration: () => { + try { + // Safely access timeline store to avoid circular dependencies + // Uses global reference instead of direct import to prevent circular imports + const timelineStore = globalThis.__timelineStore; + + if (!timelineStore) { + // Timeline store not available, use fallback + return get().duration; + } + + if (typeof timelineStore.getState !== 'function') { + console.warn("Timeline store getState is not a function, using fallback duration"); + return get().duration; + } + + const state = timelineStore.getState(); + if (!state) { + // Timeline store state is null/undefined + return get().duration; + } + + if (typeof state.getTotalDuration !== 'function') { + console.warn("Timeline store getTotalDuration is not a function, using fallback duration"); + return get().duration; + } + + const actualContentDuration = state.getTotalDuration(); + + // Comprehensive validation of the duration value + if (typeof actualContentDuration !== 'number') { + console.warn("Timeline store returned non-number duration:", typeof actualContentDuration, "using fallback"); + return get().duration; + } + + if (isNaN(actualContentDuration)) { + console.warn("Timeline store returned NaN duration, using fallback"); + return get().duration; + } + + if (!isFinite(actualContentDuration)) { + console.warn("Timeline store returned non-finite duration:", actualContentDuration, "using fallback"); + return get().duration; + } + + if (actualContentDuration <= 0) { + // Zero or negative duration means no content, use timeline minimum + return get().duration; + } + + // Valid content duration found + return actualContentDuration; + + } catch (error) { + console.warn("Error accessing timeline store for content duration:", error); + return get().duration; + } + }, })); diff --git a/apps/web/src/stores/timeline-store.ts b/apps/web/src/stores/timeline-store.ts index 9e3f56d60..ab547d60c 100644 --- a/apps/web/src/stores/timeline-store.ts +++ b/apps/web/src/stores/timeline-store.ts @@ -1487,3 +1487,16 @@ export const useTimelineStore = create((set, get) => { }, }; }); + +// AUDIO CURSOR FIX: Expose timeline store globally to avoid circular dependencies +// +// This enables the playback store to access getTotalDuration() for the audio cursor fix +// without creating circular imports (playback-store ↔ timeline-store). +// +// The playback store uses this to determine when to stop playback based on actual +// content duration rather than the timeline's 10-second minimum duration. +// +// Related to GitHub issue #490: "[BUG] Editor cursor does not stop at the end of an audio file" +if (typeof globalThis !== 'undefined') { + globalThis.__timelineStore = useTimelineStore; +}