From bdf900959b4eeee245b2488362f390b917a67c42 Mon Sep 17 00:00:00 2001 From: kenny lopez Date: Fri, 7 Aug 2026 08:54:35 +0100 Subject: [PATCH 1/4] feat(desktop): improve video review controls Signed-off-by: kenny lopez --- .../src/features/channels/ui/ChannelPane.tsx | 10 +- .../messages/lib/videoReviewContext.test.mjs | 52 +++++ .../messages/lib/videoReviewContext.ts | 42 ++++ .../src/features/messages/ui/MessageRow.tsx | 57 ++++-- .../messages/ui/MessageThreadPanel.tsx | 57 +++--- desktop/src/shared/ui/VideoPlayer.tsx | 182 +++++++++--------- .../src/shared/ui/VideoReviewNavigation.tsx | 59 ++++++ .../shared/ui/VideoReviewPosterPreview.tsx | 22 +++ .../shared/ui/VideoReviewTimecodeButton.tsx | 42 ++++ .../shared/ui/videoReviewTimecode.test.mjs | 22 +++ desktop/src/shared/ui/videoReviewTimecode.ts | 41 ++++ desktop/tests/e2e/video-attachment.spec.ts | 176 ++++++++++++++++- 12 files changed, 624 insertions(+), 138 deletions(-) create mode 100644 desktop/src/shared/ui/VideoReviewNavigation.tsx create mode 100644 desktop/src/shared/ui/VideoReviewPosterPreview.tsx create mode 100644 desktop/src/shared/ui/VideoReviewTimecodeButton.tsx create mode 100644 desktop/src/shared/ui/videoReviewTimecode.test.mjs create mode 100644 desktop/src/shared/ui/videoReviewTimecode.ts diff --git a/desktop/src/features/channels/ui/ChannelPane.tsx b/desktop/src/features/channels/ui/ChannelPane.tsx index 9e5152edfe..06b952845d 100644 --- a/desktop/src/features/channels/ui/ChannelPane.tsx +++ b/desktop/src/features/channels/ui/ChannelPane.tsx @@ -22,7 +22,7 @@ import { getDmHuddleMemberPubkeys, hasOtherDmParticipant, } from "@/features/channels/lib/dmHuddleMembers"; -import { buildVideoReviewContextsByMessageId } from "@/features/messages/lib/videoReviewContext"; +import { buildVideoReviewPresentationByMessageId } from "@/features/messages/lib/videoReviewContext"; import { useComposerHeightPadding } from "@/features/messages/ui/useComposerHeightPadding"; import { UserProfilePanel } from "@/features/profile/ui/UserProfilePanel"; import { ChannelFindBar } from "@/features/search/ui/ChannelFindBar"; @@ -464,7 +464,7 @@ export const ChannelPane = React.memo(function ChannelPane({ const activeVideoReviewCommentSender = activeChannel?.archivedAt ? undefined : onSendVideoReviewComment; - const threadVideoReviewContextsByMessageId = React.useMemo(() => { + const threadVideoReviewPresentation = React.useMemo(() => { const messagesById = new Map( messages.map((message) => [message.id, message]), ); @@ -475,7 +475,7 @@ export const ChannelPane = React.memo(function ChannelPane({ messagesById.set(message.id, message); } - return buildVideoReviewContextsByMessageId({ + return buildVideoReviewPresentationByMessageId({ channelId: activeChannel?.id ?? null, channelName: activeChannel?.name, channelType: activeChannel?.channelType ?? null, @@ -883,9 +883,7 @@ export const ChannelPane = React.memo(function ChannelPane({ scrollTargetHighlights={!layoutScrollTargetId} scrollTargetId={layoutScrollTargetId ?? threadScrollTargetId} threadHead={threadHeadMessage} - videoReviewContextsByMessageId={ - threadVideoReviewContextsByMessageId - } + videoReviewPresentation={threadVideoReviewPresentation} widthPx={threadPanelWidthPx} threadReplies={threadMessages} threadRepliesPending={threadMessagesPending} diff --git a/desktop/src/features/messages/lib/videoReviewContext.test.mjs b/desktop/src/features/messages/lib/videoReviewContext.test.mjs index 8ecb5f5798..6c75883da9 100644 --- a/desktop/src/features/messages/lib/videoReviewContext.test.mjs +++ b/desktop/src/features/messages/lib/videoReviewContext.test.mjs @@ -4,6 +4,7 @@ import test from "node:test"; import { buildVideoReviewCommentsByRootId, buildVideoReviewCommentsForRoot, + buildVideoReviewCommentRootIdsByMessageId, buildVideoReviewContextForMessage, buildVideoReviewContextsByMessageId, hasVideoAttachment, @@ -159,6 +160,57 @@ test("buildVideoReviewCommentsForRoot returns descendants for one root", () => { ); }); +test("buildVideoReviewCommentRootIdsByMessageId targets the nearest video ancestor", () => { + const root = message({ id: "root", body: "Review request" }); + const firstVideo = message({ + id: "first-video", + body: "![video](https://relay/media/a.mp4)", + parentId: root.id, + rootId: root.id, + }); + const firstComment = message({ + id: "first-comment", + body: "[00:01] tighten this", + parentId: firstVideo.id, + rootId: root.id, + }); + const nestedVideo = message({ + id: "nested-video", + body: "![video](https://relay/media/b.mp4)", + parentId: firstComment.id, + rootId: root.id, + }); + const nestedComment = message({ + id: "nested-comment", + body: "[00:02] check this frame", + parentId: nestedVideo.id, + rootId: root.id, + }); + const plainReply = message({ + id: "plain-reply", + body: "No video ancestor", + parentId: root.id, + rootId: root.id, + }); + + const rootIds = buildVideoReviewCommentRootIdsByMessageId([ + root, + firstVideo, + firstComment, + nestedVideo, + nestedComment, + plainReply, + ]); + + assert.deepEqual( + [...rootIds.entries()], + [ + [firstComment.id, firstVideo.id], + [nestedComment.id, nestedVideo.id], + ], + ); +}); + test("buildVideoReviewContextForMessage posts against the source video", async () => { const video = message({ id: "video", diff --git a/desktop/src/features/messages/lib/videoReviewContext.ts b/desktop/src/features/messages/lib/videoReviewContext.ts index f605952f5a..78401214a2 100644 --- a/desktop/src/features/messages/lib/videoReviewContext.ts +++ b/desktop/src/features/messages/lib/videoReviewContext.ts @@ -93,6 +93,33 @@ export function buildVideoReviewCommentsForRoot( return comments; } +export function buildVideoReviewCommentRootIdsByMessageId( + messages: TimelineMessage[], +): ReadonlyMap { + const messageById = new Map(messages.map((message) => [message.id, message])); + const videoMessageIds = new Set( + messages.filter(hasVideoAttachment).map((message) => message.id), + ); + const rootIdsByMessageId = new Map(); + + for (const message of messages) { + if (videoMessageIds.has(message.id)) continue; + + let ancestorId = message.parentId ?? null; + const visited = new Set(); + while (ancestorId && !visited.has(ancestorId)) { + if (videoMessageIds.has(ancestorId)) { + rootIdsByMessageId.set(message.id, ancestorId); + break; + } + visited.add(ancestorId); + ancestorId = messageById.get(ancestorId)?.parentId ?? null; + } + } + + return rootIdsByMessageId; +} + export function buildVideoReviewContextForMessage({ channelId, channelName, @@ -193,3 +220,18 @@ export function buildVideoReviewContextsByMessageId({ return contexts; } + +export function buildVideoReviewPresentationByMessageId( + args: Parameters[0], +) { + return { + commentRootIdsByMessageId: buildVideoReviewCommentRootIdsByMessageId( + args.messages, + ), + contextsByMessageId: buildVideoReviewContextsByMessageId(args), + }; +} + +export type VideoReviewPresentation = ReturnType< + typeof buildVideoReviewPresentationByMessageId +>; diff --git a/desktop/src/features/messages/ui/MessageRow.tsx b/desktop/src/features/messages/ui/MessageRow.tsx index 9f55e712f1..22bb355d70 100644 --- a/desktop/src/features/messages/ui/MessageRow.tsx +++ b/desktop/src/features/messages/ui/MessageRow.tsx @@ -40,6 +40,9 @@ import { resolveSnapshotSharedBy } from "@/features/messages/lib/snapshotSharedB import { resolveMentionProps } from "@/shared/lib/resolveMentionNames"; import { Markdown } from "@/shared/ui/markdown"; import type { VideoReviewContext } from "@/shared/ui/VideoPlayer"; +import { useOpenVideoReviewAt } from "@/shared/ui/VideoReviewNavigation"; +import { parseVideoReviewTimecode } from "@/shared/ui/videoReviewTimecode"; +import { VideoReviewTimecodeButton } from "@/shared/ui/VideoReviewTimecodeButton"; import { MessageActionBar } from "./MessageActionBar"; import { MessageAgentOwner } from "./MessageAgentOwner"; import { MessageAuthorText, MessageHeaderRow } from "./MessageHeader"; @@ -95,6 +98,7 @@ export const MessageRow = React.memo( profiles, searchQuery, showDepthGuides = true, + videoReviewCommentRootId, videoReviewContext, }: { channelId?: string | null; @@ -143,6 +147,7 @@ export const MessageRow = React.memo( profiles?: UserProfileLookup; searchQuery?: string; showDepthGuides?: boolean; + videoReviewCommentRootId?: string; videoReviewContext?: VideoReviewContext; }) { // Keep the transient send state with its timestamp rather than collapsing @@ -244,6 +249,7 @@ export const MessageRow = React.memo( const bodyOffsetClass = emojiOnly ? "mt-1" : "-mt-0.5"; const { nonDmChannelNames: channelNames } = useChannelNavigation(); + const openVideoReviewAt = useOpenVideoReviewAt(); const indentRem = getThreadReplyIndentRem(message.depth); const descendantGuideOffsetRem = connectDescendants @@ -340,22 +346,24 @@ export const MessageRow = React.memo( message={message} /> ); - default: - { - const waveMessage = parseWaveMessageContent(message.body); - if (waveMessage) { - return ( - - ); - } + default: { + const waveMessage = parseWaveMessageContent(message.body); + if (waveMessage) { + return ( + + ); } - return ( + const reviewRootEventId = videoReviewCommentRootId; + const reviewTimecode = reviewRootEventId + ? parseVideoReviewTimecode(message.body) + : null; + const markdown = ( ); + if (!reviewRootEventId || !reviewTimecode || !openVideoReviewAt) { + return markdown; + } + + return ( +
+ { + event.stopPropagation(); + openVideoReviewAt(reviewRootEventId, reviewTimecode.seconds); + }} + /> +
{markdown}
+
+ ); + } } }; @@ -893,6 +919,7 @@ export const MessageRow = React.memo( prev.playEntrance === next.playEntrance && prev.profiles === next.profiles && prev.searchQuery === next.searchQuery && + prev.videoReviewCommentRootId === next.videoReviewCommentRootId && prev.videoReviewContext === next.videoReviewContext, ); diff --git a/desktop/src/features/messages/ui/MessageThreadPanel.tsx b/desktop/src/features/messages/ui/MessageThreadPanel.tsx index fb01783bf4..2c82268462 100644 --- a/desktop/src/features/messages/ui/MessageThreadPanel.tsx +++ b/desktop/src/features/messages/ui/MessageThreadPanel.tsx @@ -18,11 +18,13 @@ import { import type { ImetaMedia } from "@/features/messages/lib/imetaMediaMarkdown"; import { canManageMessageForCurrentUser } from "@/features/messages/lib/canManageMessage"; import type { TimelineMessage } from "@/features/messages/types"; +import type { VideoReviewPresentation } from "@/features/messages/lib/videoReviewContext"; import type { UserProfileLookup } from "@/features/profile/lib/identity"; import type { Channel } from "@/shared/api/types"; import type { ThreadPanelLayoutProps } from "@/features/channels/lib/threadPanelLayout"; import { useEscapeKey } from "@/shared/hooks/useEscapeKey"; import { useIsThreadPanelOverlay } from "@/shared/hooks/use-mobile"; +import { VideoReviewNavigationProvider } from "@/shared/ui/VideoReviewNavigation"; import { cn } from "@/shared/lib/cn"; import { AuxiliaryPanel } from "@/shared/layout/AuxiliaryPanel"; import { AuxiliaryPanelBody } from "@/shared/layout/AuxiliaryPanel"; @@ -38,7 +40,6 @@ import { } from "@/features/messages/lib/messageThreadPanelLayout"; import { Button } from "@/shared/ui/button"; import { Separator } from "@/shared/ui/separator"; -import type { VideoReviewContext } from "@/shared/ui/VideoPlayer"; import { ComposerActivityAccessory } from "./ComposerActivityAccessory"; import { ComposerDockBackdrop } from "./ComposerDockBackdrop"; import { MessageComposer } from "./MessageComposer"; @@ -111,7 +112,7 @@ type MessageThreadPanelProps = ThreadPanelLayoutProps & { threadUnreadCount?: number; threadReplyUnreadCounts?: ReadonlyMap; threadTypingPubkeys: string[]; - videoReviewContextsByMessageId?: ReadonlyMap; + videoReviewPresentation?: VideoReviewPresentation; activityAccessoryContent?: React.ReactNode; activityAccessoryVisible: boolean; widthPx: number; @@ -225,7 +226,7 @@ export function MessageThreadPanel({ scrollTargetId, scrollTargetHighlights = true, threadHead, - videoReviewContextsByMessageId, + videoReviewPresentation, threadReplies, threadRepliesPending = false, threadUnreadCount, @@ -617,7 +618,10 @@ export function MessageThreadPanel({ } profiles={profiles} showDepthGuides={shouldShowThreadBranchGuides} - videoReviewContext={videoReviewContextsByMessageId?.get( + videoReviewCommentRootId={videoReviewPresentation?.commentRootIdsByMessageId.get( + threadHead.id, + )} + videoReviewContext={videoReviewPresentation?.contextsByMessageId.get( threadHead.id, )} /> @@ -776,7 +780,10 @@ export function MessageThreadPanel({ onToggleReaction={onToggleReaction} profiles={profiles} showDepthGuides={shouldShowThreadBranchGuides} - videoReviewContext={videoReviewContextsByMessageId?.get( + videoReviewCommentRootId={videoReviewPresentation?.commentRootIdsByMessageId.get( + entry.message.id, + )} + videoReviewContext={videoReviewPresentation?.contextsByMessageId.get( entry.message.id, )} /> @@ -955,24 +962,26 @@ export function MessageThreadPanel({ ); return ( - {threadHeaderContent} - ) - } - isSinglePanelView={isSinglePanelView} - layout={layout} - onClose={onClose} - testId="message-thread-panel" - transparentChrome={transparentChrome} - widthPx={widthPx} - > - {threadScrollRegion} - + + {threadHeaderContent} + ) + } + isSinglePanelView={isSinglePanelView} + layout={layout} + onClose={onClose} + testId="message-thread-panel" + transparentChrome={transparentChrome} + widthPx={widthPx} + > + {threadScrollRegion} + + ); } diff --git a/desktop/src/shared/ui/VideoPlayer.tsx b/desktop/src/shared/ui/VideoPlayer.tsx index 908bd8e3f0..07e1044d92 100644 --- a/desktop/src/shared/ui/VideoPlayer.tsx +++ b/desktop/src/shared/ui/VideoPlayer.tsx @@ -28,6 +28,13 @@ import { UserAvatar } from "@/shared/ui/UserAvatar"; import { Spinner } from "./spinner"; import { useNaturalVideoAspectRatio } from "./videoAspectRatio"; import { useVideoContextMenu } from "./useVideoContextMenu"; +import { useRegisterVideoReview } from "./VideoReviewNavigation"; +import { VideoReviewPosterPreview } from "./VideoReviewPosterPreview"; +import { parseVideoReviewTimecode } from "./videoReviewTimecode"; +import { + VideoReviewTimecodeButton, + VIDEO_REVIEW_TIMECODE_ACCENT_CLASS, +} from "./VideoReviewTimecodeButton"; import { getInlinePlaybackPosition, getReviewPlaybackPosition, @@ -109,16 +116,10 @@ type TimecodedComment = { text: string; }; -const TIMECODE_RE = - /^\s*\[((?:(?:\d{1,2}:)?\d{1,2}:)?\d{2}(?:\.\d{1,3})?)\]\s*/; const QUICK_REACTIONS = ["😂", "😍", "😮", "🙌", "👍", "👎"]; const DEFAULT_PLAYBACK_SPEED = 1; const INLINE_SPEED_CONTROL_MIN_WIDTH = 220; const PLAYBACK_SPEEDS = [2, 1.75, 1.5, 1.25, 1, 0.75, 0.5, 0.25]; -const TIMECODE_ACCENT_CLASS = - "bg-[hsl(var(--buzz-video-review-accent,var(--primary))/0.15)] text-[hsl(var(--buzz-video-review-accent-foreground,var(--buzz-video-review-accent,var(--primary))))]"; -const TIMECODE_ACCENT_HOVER_CLASS = - "hover:bg-[hsl(var(--buzz-video-review-accent,var(--primary))/0.3)]"; /** * Frosted-glass backing layer for floating media controls. The parent must @@ -188,40 +189,11 @@ function isPlaybackSpeedOption(speed: number): boolean { return PLAYBACK_SPEEDS.some((option) => option === speed); } -function parseTimecode(value: string): number | null { - const parts = value.split(":").map((part) => Number(part)); - if (parts.some((part) => !Number.isFinite(part) || part < 0)) { - return null; - } - - if (parts.length === 2) { - return parts[0] * 60 + parts[1]; - } - - if (parts.length === 3) { - return parts[0] * 3600 + parts[1] * 60 + parts[2]; - } - - return null; -} - function parseTimecodedComment(comment: VideoReviewComment): TimecodedComment { - const match = comment.body.match(TIMECODE_RE); - if (!match) { - return { - comment, - seconds: null, - timecode: null, - text: comment.body.trim(), - }; - } - - return { - comment, - seconds: parseTimecode(match[1]), - timecode: match[1], - text: comment.body.slice(match[0].length).trim(), - }; + const parsed = parseVideoReviewTimecode(comment.body); + return parsed + ? { comment, ...parsed } + : { comment, seconds: null, text: comment.body.trim(), timecode: null }; } function sortTimecodedComments( @@ -928,20 +900,32 @@ export function VideoPlayer({ video.muted = value <= 0; }, []); + const openReviewAt = React.useCallback( + (seconds: number) => { + const video = videoRef.current; + video?.pause(); + const safeSeconds = Number.isFinite(seconds) ? Math.max(seconds, 0) : 0; + const nextSeconds = + duration > 0 ? Math.min(safeSeconds, duration) : safeSeconds; + setPendingSeekSeconds(nextSeconds); + setReviewCurrentTime(nextSeconds); + setReviewOpen(true); + }, + [duration, setReviewCurrentTime, setReviewOpen], + ); + useRegisterVideoReview(reviewContext?.rootEventId, openReviewAt); + const handleOpenReview = React.useCallback( (event?: React.SyntheticEvent) => { event?.stopPropagation(); const video = videoRef.current; - video?.pause(); const startTime = video && Number.isFinite(video.currentTime) ? video.currentTime : currentTime; - setPendingSeekSeconds(startTime); - setReviewCurrentTime(startTime); - setReviewOpen(true); + openReviewAt(startTime); }, - [currentTime, setReviewCurrentTime, setReviewOpen], + [currentTime, openReviewAt], ); const handleReviewOpenChange = React.useCallback( @@ -984,12 +968,18 @@ export function VideoPlayer({ const showInlineSpeedControl = inlineRenderedWidth !== null && inlineRenderedWidth >= INLINE_SPEED_CONTROL_MIN_WIDTH; + const shouldWarmReviewMedia = timecodedComments.length > 0; const inlineSurfaceStyle: React.CSSProperties = { aspectRatio: String(inlineAspectRatio), maxHeight: 256, width: inlineSurfaceWidth, }; - const showControls = started && !hasError; + const hideInlineControls = !started || isPlaying; + const inlineControlsRevealClass = cn( + "transition-opacity duration-150 ease-[cubic-bezier(0.23,1,0.32,1)] motion-reduce:transition-none", + hideInlineControls && + "opacity-0 group-focus-within/inline-controls:opacity-100 group-hover/video:opacity-100", + ); return ( <> @@ -1013,9 +1003,9 @@ export function VideoPlayer({ className="h-full w-full object-cover" playsInline poster={poster} - preload="metadata" + preload={shouldWarmReviewMedia ? "auto" : "metadata"} src={src} - onClick={showControls ? handleTogglePlay : undefined} + onClick={started ? handleTogglePlay : undefined} onDurationChange={(event) => handleMediaDuration(event.currentTarget.duration) } @@ -1072,16 +1062,29 @@ export function VideoPlayer({ }} onWaiting={() => setIsBuffering(true)} /> - {!started && !hasError ? ( + {!hasError && !isBuffering ? ( ) : null} @@ -1109,33 +1112,22 @@ export function VideoPlayer({ ) : null} - {/* Slide (not fade) the pill out: animating opacity on an ancestor - of a backdrop-filter flattens the glass into a plain fill - mid-transition, which reads as a flicker. The video container's - overflow-hidden clips the slid-out pill. */} - {showControls ? ( + {!hasError ? (
-
- - + +
(null); + const [hasVisibleFrame, setHasVisibleFrame] = React.useState(false); const [videoAreaSize, setVideoAreaSize] = React.useState<{ height: number; width: number; @@ -1364,6 +1357,7 @@ function VideoReviewDialog({ React.useEffect(() => { if (!open) { setIsComposerMounted(false); + setHasVisibleFrame(false); return; } // Two frames: one for the dialog to paint, one for the browser to @@ -1715,7 +1709,7 @@ function VideoReviewDialog({ className="h-full w-full min-h-0 object-contain" playsInline poster={poster} - preload="metadata" + preload="auto" src={src} onClick={togglePlay} onDurationChange={(event) => @@ -1740,6 +1734,7 @@ function VideoReviewDialog({ syncCurrentTime(pendingSeekSeconds); } }} + onLoadedData={() => setHasVisibleFrame(true)} onPause={(event) => { syncCurrentTime(event.currentTarget.currentTime); setIsPlaying(false); @@ -1748,7 +1743,15 @@ function VideoReviewDialog({ syncCurrentTime(event.currentTarget.currentTime); setIsPlaying(true); }} - onSeeked={reviewSeek.handleSeeked} + onSeeked={(event) => { + reviewSeek.handleSeeked(); + if ( + event.currentTarget.readyState >= + HTMLMediaElement.HAVE_CURRENT_DATA + ) { + setHasVisibleFrame(true); + } + }} onTimeUpdate={(event) => { syncCurrentTime(event.currentTarget.currentTime); }} @@ -1757,6 +1760,10 @@ function VideoReviewDialog({ setMuted(event.currentTarget.muted); }} /> +
@@ -1903,12 +1910,12 @@ function VideoReviewDialog({ {showCommentsPanel ? (