feat: add media mirroring controls #638
Conversation
|
@joepduin is attempting to deploy a commit to the OpenCut OSS Team on Vercel. A member of the Team first needs to authorize it. |
👷 Deploy request for appcut pending review.Visit the deploys page to approve it
|
WalkthroughThis PR introduces horizontal and vertical flip support for media elements across the timeline, refactors multiple components and utilities for consistency, implements error handling for async operations, adds store methods for media manipulation, and updates formatting throughout the codebase. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as MediaProperties Panel
participant Store as TimelineStore
participant Cache as FrameCache
participant Renderer as TimelineRenderer
User->>UI: Click flip button
activate UI
UI->>Store: toggleSelectedMediaElements("flipH")
deactivate UI
activate Store
Store->>Store: Update selected media elements flipH state
Store->>Cache: Invalidate frame cache (hash includes flipH/flipV)
deactivate Store
activate Cache
Cache->>Cache: Recompute hash with new flipH value
deactivate Cache
activate Renderer
Renderer->>Renderer: Apply transform:<br/>translate(centerX, centerY)<br/>scale(flipH ? -1 : 1, flipV ? -1 : 1)<br/>translate(-centerX, -centerY)
Renderer->>Renderer: Draw media at negative half-dimensions
deactivate Renderer
Note over UI,Renderer: Media tile renders with flipped orientation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes The PR spans ~70+ files with a mix of formatting, refactoring, and new feature logic. While many changes are formatting-only (trivial), several files contain meaningful logic additions (timeline renderer transform pipeline, media properties flip UI, frame cache invalidation, sounds store full implementation, async error handling in preview panel). The heterogeneity of changes and logic density in core rendering and store files warrant careful review across multiple areas. Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
apps/web/src/components/ui/input.tsx (1)
91-109: Add keyboard event handler to password toggle button.The password toggle button uses
onClickwithout an accompanying keyboard handler. Per accessibility guidelines, addonKeyDownoronKeyPressto support keyboard navigation.Apply this diff to add keyboard support:
{showPasswordToggle && ( <Button type="button" variant="text" size="icon" onClick={() => onShowPasswordChange?.(!showPassword)} + onKeyDown={(e) => { + if (e.key === "Enter" || e.key === " ") { + e.preventDefault(); + onShowPasswordChange?.(!showPassword); + } + }} className={cn( "absolute top-0 h-full px-3 text-muted-foreground hover:text-foreground", showClear ? "right-10" : "right-0" )} aria-label={showPassword ? "Hide password" : "Show password"} >apps/web/src/lib/iconify-api.ts (1)
9-24: Sticky host not used for first attempt; reorder hosts.currentHost is updated but ignored on next calls, causing avoidable latency.
-async function fetchWithFallback(path: string): Promise<Response> { - for (const host of ICONIFY_HOSTS) { +async function fetchWithFallback(path: string): Promise<Response> { + const hostOrder = [currentHost, ...ICONIFY_HOSTS.filter((h) => h !== currentHost)]; + for (const host of hostOrder) { try { const response = await fetch(`${host}${path}`, { signal: AbortSignal.timeout(2000), }); if (response.ok) { currentHost = host; return response; } } catch (error) { - console.warn(`Failed to fetch from ${host}:`, error); + // log at debug/warn via app logger + /* logger.warn */ /* (`Iconify fetch failed`, { host, error }); */ } } throw new Error("All API hosts failed"); }Also replace console.* with your logger (see next comment). Based on learnings.
apps/web/src/components/icons.tsx (2)
167-227: Add accessible <title> to SVG icons.Guidelines: always include a title for icons (no adjacent text).
Apply pattern to SocialsIcon (and replicate across icons in this file):
-export function SocialsIcon({ - className = "", - size = 32, -}: { - className?: string; - size?: number; -}) { +export function SocialsIcon({ + className = "", + size = 32, + title = "Socials", +}: { + className?: string; + size?: number; + title?: string; +}) { return ( <svg xmlns="http://www.w3.org/2000/svg" width={size} height={size} viewBox="0 0 345 243" fill="none" className={className} > + <title>{title}</title> <g opacity="0.5">Also add a title prop and element to TransitionUpIcon similarly.
As per coding guidelines.
231-267: Add <title> to TransitionUpIcon.-export function TransitionUpIcon({ - className = "", - size = 16, -}: { - className?: string; - size?: number; -}) { +export function TransitionUpIcon({ + className = "", + size = 16, + title = "Transition up", +}: { + className?: string; + size?: number; + title?: string; +}) { return ( <svg xmlns="http://www.w3.org/2000/svg" width={size} height={size} viewBox="0 0 16 16" fill="none" className={className} > + <title>{title}</title>As per coding guidelines.
apps/web/src/lib/mediabunny-utils.ts (1)
207-215: Clamp duration and round adelay to ms to prevent FFmpeg errors.Negative/zero durations and fractional ms in adelay can fail or produce silence.
- const actualStart = element.trimStart; - const actualDuration = - element.duration - element.trimStart - element.trimEnd; + const actualStart = Math.max(0, element.trimStart); + const actualDuration = Math.max( + 0, + element.duration - element.trimStart - element.trimEnd + ); + const delayMs = Math.max(0, Math.round(element.startTime * 1000)); @@ - filterInputs.push( - `[${i}:a]atrim=start=${actualStart}:duration=${actualDuration},asetpts=PTS-STARTPTS,adelay=${element.startTime * 1000}|${element.startTime * 1000}[${filterName}]` - ); + if (actualDuration > 0) { + filterInputs.push( + `[${i}:a]atrim=start=${actualStart}:duration=${actualDuration},asetpts=PTS-STARTPTS,adelay=${delayMs}|${delayMs}[${filterName}]` + ); + }This avoids invalid trims and ensures integer millisecond delays.
apps/web/src/components/editor/media-panel/views/captions.tsx (1)
111-113: Remove console.*; use structured logging/instrumentation.Project guidelines disallow console usage. Replace with your logger or observability hooks.
- console.log("Transcription completed:", { text, segments }); + // log: handled via app logger/telemetry (omitted to avoid leaking user content) - console.log( - `✅ ${shortCaptions.length} short-form caption chunks added to timeline!` - ); + // info: captions added; consider emitting a non-PII metric instead - } catch (error) { - console.error("Transcription failed:", error); + } catch (error) { + // capture error with logger/telemetry without PIIAlso applies to: 176-181
apps/web/src/lib/timeline-renderer.ts (1)
205-213: Major perf issue: new Image() per frame for foreground images. Use the existing cache.The image branch recreates and decodes an Image on every render, which is expensive and can jitter. Reuse getImageElement(mediaItem) as in the blur path.
- if (mediaItem.type === "image") { - const img = new Image(); - await new Promise<void>((resolve, reject) => { - img.onload = () => resolve(); - img.onerror = () => reject(new Error("Image load failed")); - img.src = mediaItem.url || URL.createObjectURL(mediaItem.file); - }); + if (mediaItem.type === "image") { + const img = await getImageElement(mediaItem);This keeps the rest (mediaW/H, containScale, flip, draw) unchanged.
🧹 Nitpick comments (42)
apps/web/src/stores/text-properties-store.ts (1)
11-14: Prefersatisfiesover explicit type +as const.Avoid redundant type annotation on a literal; use
satisfiesfor compile‑time checking and keep readonly intent.As per coding guidelines.
-export const TEXT_PROPERTIES_TABS: ReadonlyArray<TextPropertiesTabMeta> = [ - { value: "transform", label: "Transform" }, - { value: "style", label: "Style" }, -] as const; +export const TEXT_PROPERTIES_TABS = [ + { value: "transform", label: "Transform" }, + { value: "style", label: "Style" }, +] as const satisfies readonly TextPropertiesTabMeta[];apps/web/src/lib/export.ts (5)
51-53: SSR-safe AudioContext construction and noany.Guard for SSR, avoid
any, and align context sample rate with your output to reduce resampling artifacts.As per coding guidelines.
- const audioContext = new ( - window.AudioContext || (window as any).webkitAudioContext - )(); + const AudioContextCtor = + typeof window !== "undefined" + ? ("AudioContext" in window + ? window.AudioContext + : (window as typeof window & { webkitAudioContext?: typeof AudioContext }) + .webkitAudioContext) + : null; + if (!AudioContextCtor) { + return null; // Audio not available (SSR or unsupported) + } + // Try to align the sample rate to minimize resampling work. + const audioContext = + // Some browsers support options; fall back if not. + // @ts-expect-error older Safari lacks AudioContextOptions + new AudioContextCtor({ sampleRate }) || new AudioContextCtor();
77-79: Avoid unnecessary ArrayBuffer copy.Decoding accepts the original buffer; copying increases memory pressure during export.
- const audioBuffer = await audioContext.decodeAudioData( - arrayBuffer.slice(0) - ); + const audioBuffer = await audioContext.decodeAudioData(arrayBuffer);
139-145: Prevent clipping when mixing audio.Unbounded summation can exceed [-1, 1]. Clamp (and optionally apply a simple gain) to avoid distortion.
- outputData[outputIndex] += sourceData[sourceIndex]; + const mixed = outputData[outputIndex] + sourceData[sourceIndex]; + // Clamp to [-1, 1] to prevent clipping artifacts. + outputData[outputIndex] = Math.max(-1, Math.min(1, mixed));If you routinely mix many sources, consider applying a per‑element gain (e.g., 1/numActiveAtSample) for headroom. Based on coding guidelines.
240-273: Await inside loop — document necessity or batch.Guidelines discourage
awaitin loops. Ifmediabunnyrequires sequential frame submission, add an inline comment and disable the lint rule locally; otherwise, process in small batches to keep the UI responsive.- for (let frameIndex = 0; frameIndex < totalFrames; frameIndex++) { + /* eslint-disable no-await-in-loop -- Frame rendering/encoding must be sequential for correct ordering */ + for (let frameIndex = 0; frameIndex < totalFrames; frameIndex++) { // ... await renderTimelineFrame({ /* ... */ }); // ... await videoSource.add(time, frameDuration); // ... } + /* eslint-enable no-await-in-loop */Please confirm sequential writes are required; if not, I can propose a micro‑batching approach. As per coding guidelines.
89-90: Replaceconsole.*with project logger.Project guidelines disallow console usage. Route through a logging utility.
As per coding guidelines.
+import { logger } from "@/lib/logger"; @@ - console.warn(`Failed to decode audio file ${mediaItem.name}:`, error); + logger.warn({ err: error, file: mediaItem.name }, "Failed to decode audio file"); @@ - console.error("Export failed:", error); + logger.error({ err: error }, "Export failed");If a logger utility doesn’t exist, I can add a minimal wrapper (pino/console-bridged in dev) and wire it here.
Also applies to: 288-289
apps/web/src/constants/text-constants.ts (1)
4-4: Usesatisfieson the literal instead of annotating.Keeps strong checking without over-constraining the value types.
As per coding guidelines.
-export const DEFAULT_TEXT_ELEMENT: Omit<TextElement, "id"> = { +export const DEFAULT_TEXT_ELEMENT = { // … -}; +} satisfies Omit<TextElement, "id">;apps/web/src/components/editor/audio-waveform.tsx (2)
1-1: Don’t import React default; use type-only import for FC.Align with the guideline and keep types explicit.
As per coding guidelines.
-import React, { useEffect, useRef, useState } from "react"; +import { useEffect, useRef, useState } from "react"; +import type { FC } from "react"; @@ -const AudioWaveform: React.FC<AudioWaveformProps> = ({ +const AudioWaveform: FC<AudioWaveformProps> = ({Also applies to: 10-14
71-75: Replaceconsole.errorwith logger.Console is disallowed; route errors via a logger for consistency.
As per coding guidelines.
+import { logger } from "@/lib/logger"; @@ - console.error("WaveSurfer error:", err); + logger.error({ err }, "WaveSurfer error"); @@ - console.error("Failed to initialize WaveSurfer:", err); + logger.error({ err }, "Failed to initialize WaveSurfer");I can wire a minimal logger if not present.
Also applies to: 80-83
apps/web/src/components/editor/properties-panel/text-properties.tsx (1)
134-134: Consider rendering null instead of an empty div.The empty div for the "transform" tab creates an unnecessary DOM element. Rendering
nullwould be more efficient.t.value === "transform" ? ( - <div className="space-y-6" /> + null ) : (apps/web/src/app/api/transcribe/route.ts (2)
57-57: Remove unused variable.The
originvariable is retrieved but never used in the function, violating the coding guideline to avoid unused variables.As per coding guidelines
const ip = request.headers.get("x-forwarded-for") ?? "anonymous"; const { success } = await baseRateLimit.limit(ip); - const origin = request.headers.get("origin");
103-112: Replaceanytype with proper type definition.The
modalRequestBodyuses an explicitanytype, which violates the coding guideline prohibiting the use ofany. Define a proper type or interface for the request body structure.As per coding guidelines
+ interface ModalRequestBody { + filename: string; + language: string; + decryptionKey?: string; + iv?: string; + } + // Prepare request body for Modal - const modalRequestBody: any = { + const modalRequestBody: ModalRequestBody = { filename, language, };apps/web/src/lib/iconify-api.ts (2)
80-82: Replace console. with app logger.*Guidelines: don’t use console in app code; use a centralized logger.
- console.error("Failed to fetch collections:", error); + /* logger.error */ /* ("Failed to fetch collections", { error }); */ @@ - console.error(`Failed to fetch collection ${prefix}:`, error); + /* logger.error */ /* ("Failed to fetch collection", { prefix, error }); */ @@ - console.error("Failed to search icons:", error); + /* logger.error */ /* ("Failed to search icons", { query, prefixes, category, error }); */As per coding guidelines.
Also applies to: 94-96, 120-124
155-157: Encode color robustly.Replace manual “#” substitution with encodeURIComponent to handle all cases.
- if (params?.color) { - urlParams.append("color", params.color.replace("#", "%23")); - } + if (params?.color) { + urlParams.append("color", encodeURIComponent(params.color)); + }apps/web/src/lib/video-cache.ts (2)
19-23: Defensive clamp for invalid timestamps.Prevent negative/NaN time inputs from cascading through the iterator/seek logic.
- async getFrameAt( - mediaId: string, - file: File, - time: number - ): Promise<WrappedCanvas | null> { + async getFrameAt( + mediaId: string, + file: File, + time: number + ): Promise<WrappedCanvas | null> { + if (!Number.isFinite(time) || time < 0) time = 0;Also applies to: 26-47
74-76: Replace console. with app logger.*Per guidelines, avoid console in library code.
- console.warn("Iterator failed, will restart:", error); + /* logger.warn */ /* ("Video iterator failed; restarting", { error }); */ @@ - console.warn("Failed to seek video:", error); + /* logger.warn */ /* ("Failed to seek video", { error, time }); */ @@ - console.error(`Failed to initialize video sink for ${mediaId}:`, error); + /* logger.error */ /* ("Failed to init video sink", { mediaId, error }); */As per coding guidelines.
Also applies to: 100-101, 151-153
apps/web/src/lib/mediabunny-utils.ts (3)
190-205: Avoid await-in-loop when writing inputs; parallelize writes.Reduces total latency and aligns with guidelines.
- for (let i = 0; i < audioElements.length; i++) { - const element = audioElements[i]; - const inputName = `input_${i}.${element.file.name.split(".").pop()}`; - inputFiles.push(inputName); - try { - await ffmpeg.writeFile( - inputName, - new Uint8Array(await element.file.arrayBuffer()) - ); - } catch (error) { - ... - } - ... - } + await Promise.all( + audioElements.map(async (element, i) => { + const ext = element.file.name.includes(".") + ? element.file.name.split(".").pop() + : "bin"; + const inputName = `input_${i}.${ext}`; + inputFiles[i] = inputName; + try { + await ffmpeg.writeFile( + inputName, + new Uint8Array(await element.file.arrayBuffer()) + ); + } catch (error) { + console.error(`Failed to write file ${element.file.name}:`, error); + throw new Error( + `Unable to process file: ${element.file.name}. The file may be corrupted or in an unsupported format.` + ); + } + const actualStart = Math.max(0, element.trimStart); + const actualDuration = Math.max( + 0, + element.duration - element.trimStart - element.trimEnd + ); + const delayMs = Math.max(0, Math.round(element.startTime * 1000)); + const filterName = `audio_${i}`; + if (actualDuration > 0) { + filterInputs[i] = + `[${i}:a]atrim=start=${actualStart}:duration=${actualDuration},` + + `asetpts=PTS-STARTPTS,adelay=${delayMs}|${delayMs}[${filterName}]`; + } + }) + );Note: consider a concurrency limiter if many inputs are expected.
192-193: Handle missing file extension.split(".").pop() can be undefined; provide a safe default.
- const inputName = `input_${i}.${element.file.name.split(".").pop()}`; + const ext = element.file.name.includes(".") + ? element.file.name.split(".").pop() + : "bin"; + const inputName = `input_${i}.${ext}`;
119-121: Replace console. with app logger.*Per guidelines.
- console.error("Failed to load fresh FFmpeg instance:", error); + /* logger.error */ /* ("FFmpeg init failed", { error }); */ @@ - console.error(`Failed to write file ${element.file.name}:`, error); + /* logger.error */ /* ("FFmpeg writeFile failed", { name: element.file.name, error }); */ @@ - console.error("FFmpeg execution failed:", error); + /* logger.error */ /* ("FFmpeg exec failed", { error, args: ffmpegArgs }); */ @@ - console.error("Failed to generate silent audio:", error); + /* logger.error */ /* ("Silent audio generation failed", { error, durationSeconds }); */As per coding guidelines.
Also applies to: 201-205, 243-247, 302-304
apps/web/src/components/ui/tooltip.tsx (2)
3-3: Avoid React namespace import; use type-only + named import.Conform to “Don’t import React itself” for TSX and keep types as type-only.
-import * as React from "react"; +import { forwardRef } from "react"; +import type { ComponentPropsWithoutRef, ElementRef } from "react"; interface TooltipContentProps - extends React.ComponentPropsWithoutRef<typeof TooltipPrimitive.Content>, + extends ComponentPropsWithoutRef<typeof TooltipPrimitive.Content>, VariantProps<typeof tooltipVariants> {} -const TooltipContent = React.forwardRef< - React.ElementRef<typeof TooltipPrimitive.Content>, +const TooltipContent = forwardRef< + ElementRef<typeof TooltipPrimitive.Content>, TooltipContentProps >(({ className, sideOffset = 4, variant, ...props }, ref) => ( <TooltipPrimitive.ContentAlso applies to: 45-53, 73-73
55-68: SVG needs a title for accessibility.Add a descriptive <title> to comply with SVG a11y rules.
<svg width="6" height="10" viewBox="0 0 6 10" fill="none" xmlns="http://www.w3.org/2000/svg" className="absolute left-[-6px] top-1/2 -translate-y-1/2" > + <title>Tooltip pointer</title> <path d="M6 0L0 5L6 10V0Z" className="fill-white/80 dark:fill-[#413F3E]" /> </svg>apps/web/src/components/editor/media-panel/views/captions.tsx (2)
211-224: Explicitly set button type.Add type="button" to prevent unintended form submission and to satisfy button requirements.
- <Button + <Button + type="button" className="w-full" onClick={() => {- <Button + <Button + type="button" variant="outline" onClick={() => setShowPrivacyDialog(false)} disabled={isProcessing} >- <Button + <Button + type="button" onClick={() => { localStorage.setItem(PRIVACY_DIALOG_KEY, "true");Also applies to: 282-288, 289-299
121-131: Prefer for...of over Array.forEach for clarity and control flow.Improves readability and avoids nested callbacks.
- segments.forEach((segment: any) => { + for (const segment of segments as any[]) { const words = segment.text.trim().split(/\s+/); const segmentDuration = segment.end - segment.start; const wordsPerSecond = words.length / segmentDuration; // Split into chunks of 2-4 words const chunks: string[] = []; - for (let i = 0; i < words.length; i += 3) { + for (let i = 0; i < words.length; i += 3) { chunks.push(words.slice(i, i + 3).join(" ")); } // Calculate timing for each chunk to place them sequentially let chunkStartTime = segment.start; - chunks.forEach((chunk) => { + for (const chunk of chunks) { const chunkWords = chunk.split(/\s+/).length; const chunkDuration = Math.max(0.8, chunkWords / wordsPerSecond); // Minimum 0.8s per chunk let adjustedStartTime = chunkStartTime; // Prevent overlapping... if (adjustedStartTime < globalEndTime) { adjustedStartTime = globalEndTime; } shortCaptions.push({ text: chunk, startTime: adjustedStartTime, duration: chunkDuration, }); // Update global end time globalEndTime = adjustedStartTime + chunkDuration; // Next chunk starts when this one ends (for within-segment timing) chunkStartTime += chunkDuration; - }); - }); + } + }- shortCaptions.forEach((caption, index) => { + for (let index = 0; index < shortCaptions.length; index++) { + const caption = shortCaptions[index]; addElementToTrack(captionTrackId, { ...DEFAULT_TEXT_ELEMENT, name: `Caption ${index + 1}`, content: caption.text, duration: caption.duration, startTime: caption.startTime, fontSize: 65, fontWeight: "bold", } as TextElement); - }); + }Also applies to: 132-158, 164-174
apps/web/src/components/ui/editable-timecode.tsx (1)
104-121: Expose error state to assistive tech.Add aria-invalid when parse fails.
<input ref={inputRef} type="text" value={inputValue} onChange={handleInputChange} onKeyDown={handleKeyDown} onBlur={handleBlur} className={cn( "text-xs font-mono bg-transparent border-none outline-none", "focus:bg-background focus:border focus:border-primary focus:px-1 focus:rounded", "tabular-nums text-primary", hasError && "text-destructive focus:border-destructive", className )} + aria-invalid={hasError || undefined} style={{ width: `${formattedTime.length + 1}ch` }} placeholder={formattedTime} />apps/web/src/hooks/use-infinite-scroll.ts (1)
20-31: LGTM — simple, correct scroll threshold logic.Optional: consider IntersectionObserver to avoid frequent scroll events on large lists; keeps CPU/UI thread calmer.
apps/web/src/components/ui/color-picker.tsx (1)
41-41: Prefer Number.isNaN over global isNaN.Replace isNaN(h) with Number.isNaN(h) for correctness and clarity. As per coding guidelines.
- if (isNaN(h)) h = 0; + if (Number.isNaN(h)) h = 0;apps/web/src/components/editor/media-panel/views/media.tsx (5)
98-104: Avoid await in loops; batch writes.Replace sequential awaits with Promise.all (or a limited-concurrency batch) to improve throughput and UX on many files. As per coding guidelines.
- for (const item of processedItems) { - await addMediaFile(activeProject.id, item); - } + await Promise.all( + processedItems.map((item) => addMediaFile(activeProject.id, item)) + );
105-107: Remove console usage; use app logger or telemetry.Replace console.error with your logging/telemetry util (or remove) and keep user feedback via toast. As per coding guidelines.
- console.error("Error processing files:", error); + // logger.error?.("Error processing files", { error });
185-257: Use for...of instead of Array.forEach.Refactor to for...of for consistency and readability. As per coding guidelines.
- filteredMediaItems.forEach((item) => { + for (const item of filteredMediaItems) { let preview: React.ReactNode; // ...body unchanged... - previews.set(item.id, preview); - }); + previews.set(item.id, preview); + }
191-197: Use Next.js Image instead of.
Switch to next/image for optimization and layout stability; avoid
in Next.js. Keep lucide Image icon unchanged by importing next/image as NextImage. As per coding guidelines.
+import NextImage from "next/image"; @@ - <img - src={item.url} - alt={item.name} - className="w-full max-h-full object-cover" - loading="lazy" - /> + <div className="relative w-full h-full"> + <NextImage + src={item.url} + alt={item.name} + fill + sizes="160px" + className="object-cover" + priority={false} + /> + </div> @@ - <img - src={item.thumbnailUrl} - alt={item.name} - className="w-full h-full object-cover rounded" - loading="lazy" - /> + <NextImage + src={item.thumbnailUrl} + alt={item.name} + fill + sizes="160px" + className="object-cover rounded" + priority={false} + />Also applies to: 203-209
210-211: Add accessible titles to icons.Provide title or aria-label on decorative icons without adjacent text to improve a11y. As per coding guidelines.
- <Video className="h-6 w-6 text-white drop-shadow-md" /> + <Video className="h-6 w-6 text-white drop-shadow-md" title="Video" /> @@ - <Video className="h-6 w-6 mb-1" /> + <Video className="h-6 w-6 mb-1" title="Video" /> @@ - <Music className="h-6 w-6 mb-1" /> + <Music className="h-6 w-6 mb-1" title="Audio" /> @@ - <Image className="h-6 w-6" /> + <Image className="h-6 w-6" title="Unknown media" />Also applies to: 222-224, 235-236, 247-248
apps/web/src/components/editor/timeline/timeline-marker.tsx (1)
39-64: Extract formatTime; avoid inline IIFE in JSX.Move formatTime outside render for readability and to avoid reallocation on each render.
-export function TimelineMarker({ +const formatTime = (seconds: number, interval: number) => { + const hours = Math.floor(seconds / 3600); + const minutes = Math.floor((seconds % 3600) / 60); + const secs = seconds % 60; + if (hours > 0) return `${hours}:${minutes.toString().padStart(2,"0")}:${Math.floor(secs).toString().padStart(2,"0")}`; + if (minutes > 0) return `${minutes}:${Math.floor(secs).toString().padStart(2,"0")}`; + if (interval >= 1) return `${Math.floor(secs)}s`; + return `${secs.toFixed(1)}s`; +}; + +export function TimelineMarker({ time, zoomLevel, interval, isMainMarker, }: TimelineMarkerProps) { @@ - <span + <span className={cn( "absolute top-1 left-1 text-[0.6rem]", isMainMarker ? "text-muted-foreground font-medium" : "text-muted-foreground/70" )} > - {(() => { - const formatTime = (seconds: number) => { - const hours = Math.floor(seconds / 3600); - const minutes = Math.floor((seconds % 3600) / 60); - const secs = seconds % 60; - if (hours > 0) { - return `${hours}:${minutes - .toString() - .padStart(2, "0")}:${Math.floor(secs) - .toString() - .padStart(2, "0")}`; - } - if (minutes > 0) { - return `${minutes}:${Math.floor(secs) - .toString() - .padStart(2, "0")}`; - } - if (interval >= 1) { - return `${Math.floor(secs)}s`; - } - return `${secs.toFixed(1)}s`; - }; - return formatTime(time); - })()} + {formatTime(time, interval)} </span>apps/web/src/types/timeline.ts (1)
22-24: flipH/flipV plumbing verified; minor boolean cast cleanup suggested.Verification confirms:
- ✓ Defaults:
flipH: false, flipV: falseon new media (timeline-store.ts:563)- ✓ Frame cache: Both fields included in hash (use-frame-cache.ts:90–91)
- ✓ Renderer: Applied correctly via
ctx.scale()(timeline-renderer.ts:113–114, 190–191, 226–227)- ✓ UI: Wired to toggle buttons (media-properties.tsx, timeline-element.tsx)
Minor: Replace unnecessary
!!boolean casts in timeline-renderer.ts (lines 113, 114, 190, 191, 226, 227) with?? falsefor consistency—matches frame-cache.ts pattern and aligns with coding guidelines (avoid unnecessary boolean casts).apps/web/src/components/editor/timeline/timeline-element.tsx (1)
193-195: Flip transform logic looks correct; consider CSS class toggle to reduce inline style churn.Keep the behavior, but consider setting data attributes (e.g., data-flip-h/v) and express transforms via CSS classes for better memoization and fewer re-renders.
Also applies to: 201-203
apps/web/src/components/editor/properties-panel/index.tsx (1)
41-41: trackId is passed but unused by MediaProperties.Either destructure and use it, or drop it from both the callsite and the component’s prop type to avoid dead props.
apps/web/src/components/editor/media-panel/views/stickers.tsx (3)
100-109: Type the CSS variables precisely.Narrow the index signature so TS catches typos in custom props.
-const gridStyle: CSSProperties & Record<string, string> = { +const gridStyle: CSSProperties & Record<"--sticker-min" | "--sticker-max", string> = {Also applies to: 112-112
298-305: Simplify duplicated state updates in useEffect.setShowCollectionItems(false) is called in both branches. Collapse to a single assignment before the branch.
useEffect(() => { - if (isInCollection) { - setShowCollectionItems(false); - const timer = setTimeout(() => setShowCollectionItems(true), 350); - return () => clearTimeout(timer); - } - setShowCollectionItems(false); + setShowCollectionItems(false); + if (isInCollection) { + const timer = setTimeout(() => setShowCollectionItems(true), 350); + return () => clearTimeout(timer); + } }, [isInCollection]);
348-353: Add an accessible name to the icon-only “Clear recent” button.Provide aria-label to ensure SR users have context (tooltips aren’t announced).
-<button +<button type="button" onClick={clearRecentStickers} className="ml-auto h-5 w-5 p-0 rounded hover:bg-accent flex items-center justify-center" + aria-label="Clear recent stickers" >apps/web/src/components/editor/properties-panel/media-properties.tsx (2)
12-17: Prop shape includes trackId but it’s not used.Either use it or remove it from both the type and callsites to prevent drift.
-export function MediaProperties({ - element, -}: { - element: MediaElement; - trackId: string; -}) { +export function MediaProperties({ element }: { element: MediaElement }) {
42-50: Explicitly set button type and (optionally) add icon titles.
- Add type="button" to avoid accidental form submit in nested forms.
- Optional: add title on SVGs to satisfy strict “title for icons” policy.
-<Button +<Button + type="button" size="icon" variant={element.flipH ? "default" : "outline"} onClick={toggleFlipH} aria-pressed={element.flipH ?? false} aria-label="Toggle horizontal mirror" > - <FlipHorizontal className="size-4" /> + <FlipHorizontal className="size-4" title="Horizontal mirror" /> </Button> -<Button +<Button + type="button" size="icon" variant={element.flipV ? "default" : "outline"} onClick={toggleFlipV} aria-pressed={element.flipV ?? false} aria-label="Toggle vertical mirror" > - <FlipVertical className="size-4" /> + <FlipVertical className="size-4" title="Vertical mirror" /> </Button>Also applies to: 58-66
apps/web/src/components/editor/preview-panel.tsx (1)
617-626: Avoid ts-expect-error by narrowing OffscreenCanvas width/height with in-operator.Removes the suppression while staying type-safe.
-// @ts-expect-error width/height exist on OffscreenCanvas in modern browsers -if ( - (c as unknown as { width: number }).width !== displayWidth || - (c as unknown as { height: number }).height !== displayHeight -) { - // @ts-expect-error - (c as unknown as { width: number }).width = displayWidth; - // @ts-expect-error - (c as unknown as { height: number }).height = displayHeight; -} +const hasDims = "width" in (c as any) && "height" in (c as any); +if (hasDims) { + const dims = c as unknown as { width: number; height: number }; + if (dims.width !== displayWidth || dims.height !== displayHeight) { + dims.width = displayWidth; + dims.height = displayHeight; + } +}apps/web/src/stores/sounds-store.ts (1)
227-281: Consider more accurate error messaging.The error thrown on Line 270 assumes the failure is due to overlaps, but
addElementAtTimecould fail for other reasons. Consider making the error message more generic or inspecting the actual failure mode.Apply this diff for more accurate error handling:
if (success) { return true; } - throw new Error("Failed to add to timeline - check for overlaps"); + throw new Error("Failed to add sound to timeline");Alternatively, if you want to preserve specific diagnostics, you could enhance
addElementAtTimeto return an error reason.
| scheduleNow().catch((error) => { | ||
| console.error("Failed to reschedule audio after seek", error); | ||
| }); | ||
| }; | ||
|
|
||
| // Apply volume/mute changes immediately | ||
| void ensureAudioGraph(); | ||
| ensureAudioGraph().catch((error) => { | ||
| console.error("Failed to ensure audio graph", error); | ||
| }); |
There was a problem hiding this comment.
Replace console. with the project’s logging mechanism.*
Guidelines disallow console usage. Swap to a logger/telemetry util (or gate behind NODE_ENV).
- scheduleNow().catch((error) => {
- console.error("Failed to reschedule audio after seek", error);
- });
+ scheduleNow().catch((error) => {
+ log.error("preview.audio.reschedule_failed", { error });
+ });
- ensureAudioGraph().catch((error) => {
- console.error("Failed to ensure audio graph", error);
- });
+ ensureAudioGraph().catch((error) => {
+ log.error("preview.audio.graph_ensure_failed", { error });
+ });
- scheduleNow().catch((error) => {
- console.error("Failed to start audio playback", error);
- });
+ scheduleNow().catch((error) => {
+ log.error("preview.audio.start_failed", { error });
+ });
- draw().catch((error) => {
- console.error("Failed to render preview frame", error);
- });
+ draw().catch((error) => {
+ log.error("preview.render_failed", { error });
+ });If no logger exists, I can add a minimal wrapper around console under a feature flag. Do you want that?
Also applies to: 457-460, 680-682
🤖 Prompt for AI Agents
In apps/web/src/components/editor/preview-panel.tsx around lines 439-447 (and
also update the similar usages at 457-460 and 680-682), replace direct console.*
calls with the project's logging/telemetry utility (e.g.,
logger.error/telemetry.capture) or guard them behind NODE_ENV so logs are
emitted only in dev; if no logger exists, add a minimal wrapper util that
exposes error/info methods (feature-flagged) and use it here to log the error
messages and error objects consistently.
| "use client"; | ||
|
|
||
| import { Button } from "./ui/button"; | ||
| import { Sun, Moon } from "lucide-react"; |
There was a problem hiding this comment.
Remove unused import Moon.
The Moon icon is imported but never used in the component. Only Sun is rendered.
-import { Sun, Moon } from "lucide-react";
+import { Sun } from "lucide-react";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { Sun, Moon } from "lucide-react"; | |
| import { Sun } from "lucide-react"; |
🤖 Prompt for AI Agents
In apps/web/src/components/theme-toggle.tsx around line 4, the imported Moon
icon is unused; remove Moon from the import list (leave only Sun) or replace the
import with the correctly used icon if the Moon was intended to be rendered,
then run the linter to ensure no unused-import warnings remain.
| return ( | ||
| <span | ||
| onClick={startEditing} | ||
| className={cn( | ||
| "text-xs font-mono tabular-nums text-primary cursor-pointer", | ||
| "hover:bg-muted/50 hover:rounded px-1 -mx-1 transition-colors", | ||
| disabled && "cursor-default hover:bg-transparent", | ||
| className | ||
| )} | ||
| title={disabled ? undefined : "Click to edit time"} | ||
| > | ||
| {formattedTime} | ||
| </span> | ||
| ); |
There was a problem hiding this comment.
Use a button instead of a clickable span (+ type attribute).
Meets a11y rules (keyboard/focus semantics) and button type requirement.
- return (
- <span
- onClick={startEditing}
- className={cn(
- "text-xs font-mono tabular-nums text-primary cursor-pointer",
- "hover:bg-muted/50 hover:rounded px-1 -mx-1 transition-colors",
- disabled && "cursor-default hover:bg-transparent",
- className
- )}
- title={disabled ? undefined : "Click to edit time"}
- >
- {formattedTime}
- </span>
- );
+ return (
+ <button
+ type="button"
+ onClick={startEditing}
+ disabled={disabled}
+ className={cn(
+ "text-xs font-mono tabular-nums text-primary cursor-pointer",
+ "hover:bg-muted/50 hover:rounded px-1 -mx-1 transition-colors",
+ disabled && "cursor-default hover:bg-transparent",
+ className
+ )}
+ title={disabled ? undefined : "Click to edit time"}
+ >
+ {formattedTime}
+ </button>
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return ( | |
| <span | |
| onClick={startEditing} | |
| className={cn( | |
| "text-xs font-mono tabular-nums text-primary cursor-pointer", | |
| "hover:bg-muted/50 hover:rounded px-1 -mx-1 transition-colors", | |
| disabled && "cursor-default hover:bg-transparent", | |
| className | |
| )} | |
| title={disabled ? undefined : "Click to edit time"} | |
| > | |
| {formattedTime} | |
| </span> | |
| ); | |
| return ( | |
| <button | |
| type="button" | |
| onClick={startEditing} | |
| disabled={disabled} | |
| className={cn( | |
| "text-xs font-mono tabular-nums text-primary cursor-pointer", | |
| "hover:bg-muted/50 hover:rounded px-1 -mx-1 transition-colors", | |
| disabled && "cursor-default hover:bg-transparent", | |
| className | |
| )} | |
| title={disabled ? undefined : "Click to edit time"} | |
| > | |
| {formattedTime} | |
| </button> | |
| ); |
🤖 Prompt for AI Agents
In apps/web/src/components/ui/editable-timecode.tsx around lines 124 to 137,
replace the clickable <span> with a <button> element and add type="button" so it
meets accessibility and semantics; keep the same className composition, onClick
handler (startEditing), title logic, and formattedTime content, but map the
disabled prop to the button's disabled attribute and adjust conditional classes
(remove cursor-pointer when disabled) so styling remains the same while
preserving keyboard/focus behavior and correct button semantics.
| interface SplitButtonProps { | ||
| children: ReactNode; | ||
| className?: string; | ||
| } | ||
|
|
||
| interface SplitButtonSideProps extends Omit<ButtonProps, "variant" | "size"> { | ||
| children: ReactNode; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Allow standard div attributes; avoid spreading unknown props.
SplitButtonProps doesn’t include HTML div attributes, yet {...props} is spread onto the div. Extend HTML attributes to type-safely pass ARIA/data props.
Apply:
-interface SplitButtonProps {
+interface SplitButtonProps extends React.HTMLAttributes<HTMLDivElement> {
children: ReactNode;
className?: string;
}
@@
-const SplitButton = forwardRef<HTMLDivElement, SplitButtonProps>(
- ({ children, className, ...props }, ref) => {
+const SplitButton = forwardRef<HTMLDivElement, SplitButtonProps>(
+ ({ children, className, ...rest }, ref) => {
return (
<div
ref={ref}
className={cn(
"inline-flex rounded-full h-7 border border-input bg-panel-accent overflow-hidden",
className
)}
- {...props}
+ {...rest}
>
{children}
</div>
);
}
);Also applies to: 16-25
🤖 Prompt for AI Agents
In apps/web/src/components/ui/split-button.tsx around lines 6-13 and 16-25, the
props interfaces don’t include standard HTML attributes yet component spreads
{...props} onto native elements; update SplitButtonProps to extend
React.HTMLAttributes<HTMLDivElement> so ARIA/data props are allowed when
spreading to the surrounding div, and update SplitButtonSideProps to also extend
React.ButtonHTMLAttributes<HTMLButtonElement> (in addition to the current
Omit<ButtonProps, "variant" | "size">) so button-specific ARIA/data attributes
and native button props are type-safe; ensure relevant React types are imported
if not already.
| const SplitButtonSide = forwardRef< | ||
| HTMLButtonElement, | ||
| SplitButtonSideProps & { paddingClass: string } | ||
| >(({ children, className, paddingClass, onClick, ...props }, ref) => { | ||
| return ( | ||
| <Button | ||
| ref={ref} | ||
| variant="text" | ||
| className={cn( | ||
| "h-full rounded-none bg-panel-accent !opacity-100 border-0 gap-0 font-normal transition-colors disabled:text-muted-foreground", | ||
| onClick | ||
| ? "hover:bg-foreground/10 hover:opacity-100 cursor-pointer" | ||
| : "cursor-default select-text", | ||
| paddingClass, | ||
| className | ||
| )} | ||
| onClick={onClick} | ||
| {...props} | ||
| > |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Set button type explicitly.
Per guidelines, always include a type on buttons to avoid implicit “submit”.
- <Button
+ <Button
+ type="button"As per coding guidelines.
🤖 Prompt for AI Agents
In apps/web/src/components/ui/split-button.tsx around lines 33 to 51, the
rendered Button lacks an explicit type which can cause it to act as an implicit
"submit"; add an explicit type="button" prop to the Button element (place it
before the {...props} spread so the default is enforced) to ensure it never
submits forms unintentionally; no other logic changes required.
| import { Tooltip as TooltipPrimitive } from "radix-ui"; | ||
| import * as React from "react"; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify Radix import path; likely should be @radix-ui/react-tooltip.
The primitives are typically imported from @radix-ui/react-tooltip. Please confirm that "radix-ui" is intentional and resolvable in this repo; otherwise update the import.
Run:
🏁 Script executed:
#!/bin/bash
# Find Tooltip imports to confirm package path
rg -nP --type=ts --type=tsx -C2 $'from\\s+[\'"](@radix-ui/react-tooltip|radix-ui)[\'"]'Length of output: 27
🏁 Script executed:
#!/bin/bash
# First, locate the tooltip.tsx file and check its imports
if [ -f "apps/web/src/components/ui/tooltip.tsx" ]; then
echo "=== File found, showing first 100 lines ==="
head -100 "apps/web/src/components/ui/tooltip.tsx"
else
echo "=== File not found at expected path, searching for it ==="
find . -name "tooltip.tsx" -type f 2>/dev/null | head -5
fiLength of output: 2916
</web_search>
🌐 Web query:
radix-ui tooltip import package path
💡 Result:
You can import Radix Tooltip either from the monolithic entry or the package:
- Monolith: import { Tooltip } from "radix-ui". [1]
- Individual package (recommended): import * as Tooltip from "@radix-ui/react-tooltip"; or import * as TooltipPrimitive from "@radix-ui/react-tooltip". [2]
Sources:
[1] Radix Primitives docs — Introduction.
[2] Radix Primitives docs — Tooltip (installation & examples).
Fix Radix import path, use explicit React imports, and add SVG title for accessibility.
-
Update import to use the recommended individual package:
@radix-ui/react-tooltipinstead of the monolithic "radix-ui" entry point. -
Replace
import * as React from "react"with explicit imports since React.forwardRef, React.ElementRef, and React.ComponentPropsWithoutRef are used (lines 50-52):import { forwardRef, ElementRef, ComponentPropsWithoutRef } from "react";Then update line 50 to use
forwardRefdirectly instead ofReact.forwardRef. -
Add a
<title>element to the SVG (lines 55-68) for accessibility:<svg ...> <title>Tooltip arrow</title> <path ... /> </svg>
🤖 Prompt for AI Agents
In apps/web/src/components/ui/tooltip.tsx around lines 2-3 (and touching lines
~50-52 and ~55-68), the file currently imports from "radix-ui" and uses a
namespace React import; change the Radix import to the specific package
"@radix-ui/react-tooltip"; replace "import * as React from 'react'" with
explicit named imports you use (e.g., import { forwardRef, ElementRef,
ComponentPropsWithoutRef } from 'react') and update the forwardRef usage to call
forwardRef directly (replace React.forwardRef with forwardRef); finally, inside
the SVG (lines ~55-68) add a <title>Tooltip arrow</title> element immediately
after the opening <svg> tag to provide accessible labeling for the arrow.
| clearVideo(mediaId: string): void { | ||
| const sinkData = this.sinks.get(mediaId); | ||
| if (sinkData) { | ||
| if (sinkData.iterator) { | ||
| sinkData.iterator.return(); | ||
| } | ||
|
|
||
| this.sinks.delete(mediaId); | ||
| } | ||
|
|
||
| this.initPromises.delete(mediaId); | ||
| } |
There was a problem hiding this comment.
Avoid unhandled rejection from iterator.return().
return() on an AsyncGenerator returns a Promise; not awaiting can cause unhandled rejections.
Minimal, non-breaking fix:
clearVideo(mediaId: string): void {
const sinkData = this.sinks.get(mediaId);
if (sinkData) {
if (sinkData.iterator) {
- sinkData.iterator.return();
+ void sinkData.iterator.return()?.catch(() => {});
}Optionally make clearVideo async and await/catch instead. Based on learnings.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/src/lib/video-cache.ts around lines 156 to 167, clearVideo calls
sinkData.iterator.return() but does not await or handle the Promise, which can
lead to unhandled rejections; change clearVideo to handle the returned Promise
by making the method async and awaiting the return() call inside a try/catch (or
await sinkData.iterator.return().catch(() => {/*ignore*/}) ) before deleting the
sink and initPromises so any exception is caught and does not bubble as an
unhandled rejection.
| console.error( | ||
| "Project FPS is not set, assuming " + DEFAULT_FPS + "fps" | ||
| ); |
There was a problem hiding this comment.
Replace console.error with proper error handling and use template literals.
Per coding guidelines, avoid console statements and prefer template literals over string concatenation.
Consider using a logging service or error boundary instead of direct console.error:
- console.error(
- "Project FPS is not set, assuming " + DEFAULT_FPS + "fps"
- );
+ // Use app's logging/error service, or silently fall back to DEFAULT_FPS
+ // console.error(`Project FPS is not set, assuming ${DEFAULT_FPS}fps`);If you must log this, use a template literal:
- console.error(
- "Project FPS is not set, assuming " + DEFAULT_FPS + "fps"
- );
+ // Temporary: replace with proper logging service
+ // console.error(`Project FPS is not set, assuming ${DEFAULT_FPS}fps`);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| console.error( | |
| "Project FPS is not set, assuming " + DEFAULT_FPS + "fps" | |
| ); | |
| const logger = /* get from logging service */; | |
| logger.error(`Project FPS is not set, assuming ${DEFAULT_FPS}fps`); |
🤖 Prompt for AI Agents
In apps/web/src/stores/playback-store.ts around lines 40 to 42, replace the
direct console.error call and string concatenation with the app's
logging/error-handling mechanism and a template literal: use the project logger
(or an error boundary) instead of console, call the appropriate level (e.g.,
logger.warn or logger.error) and pass a template literal like `Project FPS is
not set, assuming ${DEFAULT_FPS}fps`; if no logger exists, throw a descriptive
error or create/consume the centralized logger utility rather than using
console.*.
| toggleSelectedMediaElements: (axis) => { | ||
| const { selectedElements, _tracks } = get(); | ||
| if (selectedElements.length === 0) { | ||
| return; | ||
| } | ||
|
|
||
| const selectionsByTrack = selectedElements.reduce((map, selection) => { | ||
| if (!map.has(selection.trackId)) { | ||
| map.set(selection.trackId, new Set<string>()); | ||
| } | ||
| map.get(selection.trackId)!.add(selection.elementId); | ||
| return map; | ||
| }, new Map<string, Set<string>>()); | ||
|
|
||
| let didChange = false; | ||
|
|
||
| const updatedTracks = _tracks.map((track) => { | ||
| const ids = selectionsByTrack.get(track.id); | ||
| if (!ids || ids.size === 0) { | ||
| return track; | ||
| } | ||
|
|
||
| let trackChanged = false; | ||
| const updatedElements = track.elements.map((element) => { | ||
| if (ids.has(element.id) && element.type === "media") { | ||
| trackChanged = true; | ||
| const currentValue = element[axis] ?? false; | ||
| return { | ||
| ...element, | ||
| [axis]: !currentValue, | ||
| }; | ||
| } | ||
| return element; | ||
| }); | ||
|
|
||
| if (!trackChanged) { | ||
| return track; | ||
| } | ||
|
|
||
| didChange = true; | ||
| return { | ||
| ...track, | ||
| elements: updatedElements, | ||
| }; | ||
| }); | ||
|
|
||
| if (!didChange) { | ||
| return; | ||
| } | ||
|
|
||
| get().pushHistory(); | ||
| updateTracksAndSave(updatedTracks); | ||
| }, |
There was a problem hiding this comment.
Avoid non-null assertion operator.
Line 928 uses the non-null assertion operator (!), which violates the coding guideline. While logically safe due to the check on Line 926, this should be refactored.
Apply this diff to avoid the non-null assertion:
const selectionsByTrack = selectedElements.reduce((map, selection) => {
- if (!map.has(selection.trackId)) {
- map.set(selection.trackId, new Set<string>());
- }
- map.get(selection.trackId)!.add(selection.elementId);
+ const elementIds = map.get(selection.trackId);
+ if (elementIds) {
+ elementIds.add(selection.elementId);
+ } else {
+ map.set(selection.trackId, new Set<string>([selection.elementId]));
+ }
return map;
}, new Map<string, Set<string>>());🤖 Prompt for AI Agents
In apps/web/src/stores/timeline-store.ts around lines 918 to 970, the reducer
uses a non-null assertion map.get(selection.trackId)! on line 928; replace the
assert by retrieving the set into a local variable, creating and setting it when
undefined (e.g. const set = map.get(selection.trackId) ?? new Set<string>(); if
(!map.has(selection.trackId)) map.set(selection.trackId, set);
set.add(selection.elementId);) so you never use ! and the logic remains
identical.
|
@coderabbitai generate docstrings |
✅ Actions performedInitiated docstring generation; will generate only if new commits exist. |
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @joepduin. * #638 (comment) The following files were modified: * `apps/web/src/app/api/get-upload-url/route.ts` * `apps/web/src/app/api/sounds/search/route.ts` * `apps/web/src/app/api/transcribe/route.ts` * `apps/web/src/app/api/waitlist/export/route.ts` * `apps/web/src/components/editor/export-button.tsx` * `apps/web/src/components/editor/layout-guide-overlay.tsx` * `apps/web/src/components/editor/media-panel/tabbar.tsx` * `apps/web/src/components/editor/media-panel/views/captions.tsx` * `apps/web/src/components/editor/media-panel/views/media.tsx` * `apps/web/src/components/editor/media-panel/views/stickers.tsx` * `apps/web/src/components/editor/panel-base-view.tsx` * `apps/web/src/components/editor/panel-preset-selector.tsx` * `apps/web/src/components/editor/preview-panel.tsx` * `apps/web/src/components/editor/properties-panel/index.tsx` * `apps/web/src/components/editor/properties-panel/media-properties.tsx` * `apps/web/src/components/editor/properties-panel/text-properties.tsx` * `apps/web/src/components/editor/timeline/timeline-element.tsx` * `apps/web/src/components/editor/timeline/timeline-marker.tsx` * `apps/web/src/components/footer.tsx` * `apps/web/src/components/icons.tsx` * `apps/web/src/components/keyboard-shortcuts-help.tsx` * `apps/web/src/components/language-select.tsx` * `apps/web/src/components/theme-toggle.tsx` * `apps/web/src/components/ui/editable-timecode.tsx` * `apps/web/src/components/ui/font-picker.tsx` * `apps/web/src/components/ui/input-with-back.tsx` * `apps/web/src/hooks/use-edge-auto-scroll.ts` * `apps/web/src/hooks/use-frame-cache.ts` * `apps/web/src/hooks/use-highlight-scroll.ts` * `apps/web/src/hooks/use-infinite-scroll.ts` * `apps/web/src/hooks/use-sound-search.ts` * `apps/web/src/lib/editor-utils.ts` * `apps/web/src/lib/export.ts` * `apps/web/src/lib/iconify-api.ts` * `apps/web/src/lib/timeline-renderer.ts` * `apps/web/src/lib/transcription-utils.ts` * `apps/web/src/lib/zk-encryption.ts` * `apps/web/src/stores/text-properties-store.ts`
1 participant
Description
Fixes: n/a
Type of change
How Has This Been Tested?
bun run dev, opened the editor, selected multiple media clips, toggled both flip buttons and confirmed preview updates for every selected clip.bunx biome format --write .(root),bun run lint(apps/web)Test Configuration:
Screenshots (if applicable)
Transform UI:
Before:
After:
Checklist:
Additional context
The selection-aware toggle ensures mirror state stays consistent when editing multiple clips at once while keeping undo/redo history intact.
Summary by CodeRabbit
Release Notes
New Features
Improvements