diff --git a/src/components/video-editor/VideoEditor.tsx b/src/components/video-editor/VideoEditor.tsx index f0bfbd3fe..e0cb2f154 100644 --- a/src/components/video-editor/VideoEditor.tsx +++ b/src/components/video-editor/VideoEditor.tsx @@ -109,6 +109,7 @@ import { type BlurData, type CameraFullscreenRegion, clampFocusToDepth, + createTextAnnotationRegion, DEFAULT_ANNOTATION_POSITION, DEFAULT_ANNOTATION_SIZE, DEFAULT_ANNOTATION_STYLE, @@ -119,6 +120,7 @@ import { type FigureData, type PlaybackSpeed, type Rotation3DPreset, + resolveTextAnnotationContent, type SpeedRegion, type TrimRegion, ZOOM_DEPTH_SCALES, @@ -406,6 +408,8 @@ export default function VideoEditor() { } setIsPlaying(false); setCurrentTime(0); + // This inferred duration is only a placeholder until the video element's real + // metadata resolves (see VideoPlayback's syncResolvedDuration). setDuration(inferredDurationMs > 0 ? inferredDurationMs / 1000 : 0); setError(null); @@ -415,6 +419,13 @@ export default function VideoEditor() { setWebcamVideoPath(webcamSourcePath ? toFileUrl(webcamSourcePath) : null); setRecordingCursorCaptureMode(projectCursorCaptureMode); setCurrentProjectPath(path ?? null); + // Reset the memoized last-resolved-duration guard so resolution isn't skipped + // just because the real duration happens to match a value already seen from + // before this load (e.g. reloading a project referencing the same video file, + // whose src may not change and so never re-fires `loadedmetadata`). Must run + // after the setDuration placeholder above, or its correction gets clobbered by + // the same-tick placeholder assignment winning the state-batch race. + videoPlaybackRef.current?.resetDurationResolution(); // A loaded project keeps its zooms exactly as saved, so never auto-suggest // over it (even if it has zero zooms because the user deleted them all). @@ -1472,17 +1483,12 @@ export default function VideoEditor() { (span: Span) => { const id = `annotation-${nextAnnotationIdRef.current++}`; const zIndex = nextAnnotationZIndexRef.current++; - const newRegion: AnnotationRegion = { + const newRegion = createTextAnnotationRegion({ id, startMs: Math.round(span.start), endMs: Math.round(span.end), - type: "text", - content: "Enter text...", - position: { ...DEFAULT_ANNOTATION_POSITION }, - size: { ...DEFAULT_ANNOTATION_SIZE }, - style: { ...DEFAULT_ANNOTATION_STYLE }, zIndex, - }; + }); pushState((prev) => ({ annotationRegions: [...prev.annotationRegions, newRegion], })); @@ -1616,7 +1622,7 @@ export default function VideoEditor() { if (region.id !== id) return region; const updatedRegion = { ...region, type }; if (type === "text") { - updatedRegion.content = region.textContent || "Enter text..."; + updatedRegion.content = resolveTextAnnotationContent(region.textContent); } else if (type === "image") { updatedRegion.content = region.imageContent || ""; } else if (type === "figure") { diff --git a/src/components/video-editor/VideoPlayback.tsx b/src/components/video-editor/VideoPlayback.tsx index 8b7f2e829..f1da1295c 100644 --- a/src/components/video-editor/VideoPlayback.tsx +++ b/src/components/video-editor/VideoPlayback.tsx @@ -165,6 +165,14 @@ export interface VideoPlaybackRef { containerRef: React.RefObject; play: () => Promise; pause: () => void; + /** + * Clears the memoized last-resolved-duration guard so the next metadata load + * re-syncs `duration` even if the video's real (resolved) duration happens to + * match a value already seen — needed when a caller (e.g. loading a saved + * project) sets `duration` to something else in between, which the guard has + * no other way to detect. + */ + resetDurationResolution: () => void; } function getResolvedVideoDuration(video: HTMLVideoElement): number | null { @@ -695,6 +703,19 @@ const VideoPlayback = forwardRef( video.pause(); supplementalAudioRef.current?.pause(); }, + resetDurationResolution: () => { + lastResolvedDurationRef.current = null; + // If the video element is already loaded (e.g. reloading a project that + // references the same file, so its src never actually changes and + // `loadedmetadata` won't fire again), clearing the guard alone leaves + // nothing to trigger a re-sync. Resolve immediately in that case too. + const video = videoRef.current; + if (video && video.readyState >= HTMLMediaElement.HAVE_METADATA) { + if (!syncResolvedDuration(video)) { + forceResolveDuration(video); + } + } + }, })); const updateFocusFromClientPoint = (clientX: number, clientY: number) => { diff --git a/src/components/video-editor/types.test.ts b/src/components/video-editor/types.test.ts new file mode 100644 index 000000000..bd9738744 --- /dev/null +++ b/src/components/video-editor/types.test.ts @@ -0,0 +1,29 @@ +import { describe, expect, it } from "vitest"; +import { createTextAnnotationRegion, resolveTextAnnotationContent } from "./types"; + +// Regression coverage for #127: a freshly created text annotation must start with +// truly empty content so the properties panel's placeholder shows and typing +// replaces rather than appends to baked-in text. +describe("createTextAnnotationRegion", () => { + it("starts with empty content, not a baked-in placeholder string", () => { + const region = createTextAnnotationRegion({ + id: "annotation-1", + startMs: 1000, + endMs: 2000, + zIndex: 1, + }); + + expect(region.content).toBe(""); + expect(region.type).toBe("text"); + }); +}); + +describe("resolveTextAnnotationContent", () => { + it("falls back to empty content when no prior text was stored", () => { + expect(resolveTextAnnotationContent(undefined)).toBe(""); + }); + + it("preserves existing text content when converting an existing region to text", () => { + expect(resolveTextAnnotationContent("hello world")).toBe("hello world"); + }); +}); diff --git a/src/components/video-editor/types.ts b/src/components/video-editor/types.ts index 27e740211..00666a8c6 100644 --- a/src/components/video-editor/types.ts +++ b/src/components/video-editor/types.ts @@ -337,6 +337,37 @@ export const DEFAULT_ANNOTATION_STYLE: AnnotationTextStyle = { textAnimation: "none", }; +/** + * A freshly created text annotation starts with no content: the properties panel's + * textarea has a real `placeholder` attribute for the empty-state hint, so the + * actual value must be empty for it to show and for typing to replace rather than + * append to baked-in text (see #127). + */ +export function createTextAnnotationRegion(params: { + id: string; + startMs: number; + endMs: number; + zIndex: number; +}): AnnotationRegion { + return { + id: params.id, + startMs: params.startMs, + endMs: params.endMs, + type: "text", + content: "", + position: { ...DEFAULT_ANNOTATION_POSITION }, + size: { ...DEFAULT_ANNOTATION_SIZE }, + style: { ...DEFAULT_ANNOTATION_STYLE }, + zIndex: params.zIndex, + }; +} + +/** Resolves the content for a region whose type is being switched to "text" -- same + * empty-by-default rule as a freshly created one when no prior text was stored. */ +export function resolveTextAnnotationContent(existingTextContent?: string): string { + return existingTextContent || ""; +} + export const DEFAULT_FIGURE_DATA: FigureData = { arrowDirection: "right", color: "#34B27B",