mirror of
https://github.com/block/buzz.git
synced 2026-10-27 04:30:52 +00:00
Fix video reviews in thread replies (#3719)
## Summary - Show video review comments when a video is opened from a thread reply. - Reuse review-context construction across timeline and thread views. ## Validation - `pnpm run build:e2e && pnpm exec playwright test tests/e2e/video-attachment.spec.ts --project smoke --grep "video replies in threads open the review comments view"` - `pnpm test` --------- Signed-off-by: kenny lopez <klopez4212@gmail.com>
This commit is contained in:
@@ -21,10 +21,7 @@ import {
|
||||
getDmHuddleMemberPubkeys,
|
||||
hasOtherDmParticipant,
|
||||
} from "@/features/channels/lib/dmHuddleMembers";
|
||||
import {
|
||||
buildVideoReviewCommentsByRootId,
|
||||
buildVideoReviewContextForMessage,
|
||||
} from "@/features/messages/lib/videoReviewContext";
|
||||
import { buildVideoReviewContextsByMessageId } 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";
|
||||
@@ -150,6 +147,7 @@ export const ChannelPane = React.memo(function ChannelPane({
|
||||
profilePanelTab,
|
||||
profilePanelView,
|
||||
targetMessageId,
|
||||
threadAllMessages,
|
||||
threadHeadMessage,
|
||||
threadMessages,
|
||||
threadMessagesPending = false,
|
||||
@@ -472,25 +470,26 @@ export const ChannelPane = React.memo(function ChannelPane({
|
||||
threadHeadMessage,
|
||||
threadMessages,
|
||||
});
|
||||
const videoReviewCommentsByRootId = React.useMemo(
|
||||
() => buildVideoReviewCommentsByRootId(messages),
|
||||
[messages],
|
||||
);
|
||||
const activeVideoReviewCommentSender = activeChannel?.archivedAt
|
||||
? undefined
|
||||
: onSendVideoReviewComment;
|
||||
const threadHeadVideoReviewContext = React.useMemo(() => {
|
||||
if (!threadHeadMessage) {
|
||||
return undefined;
|
||||
const threadVideoReviewContextsByMessageId = React.useMemo(() => {
|
||||
const messagesById = new Map(
|
||||
messages.map((message) => [message.id, message]),
|
||||
);
|
||||
if (threadHeadMessage) {
|
||||
messagesById.set(threadHeadMessage.id, threadHeadMessage);
|
||||
}
|
||||
for (const message of threadAllMessages) {
|
||||
messagesById.set(message.id, message);
|
||||
}
|
||||
|
||||
return buildVideoReviewContextForMessage({
|
||||
return buildVideoReviewContextsByMessageId({
|
||||
channelId: activeChannel?.id ?? null,
|
||||
channelName: activeChannel?.name,
|
||||
channelType: activeChannel?.channelType ?? null,
|
||||
comments: videoReviewCommentsByRootId.get(threadHeadMessage.id) ?? [],
|
||||
isSendingVideoReviewComment: isSending,
|
||||
message: threadHeadMessage,
|
||||
messages: [...messagesById.values()],
|
||||
onSendVideoReviewComment: activeVideoReviewCommentSender,
|
||||
onToggleReaction,
|
||||
profiles,
|
||||
@@ -499,10 +498,11 @@ export const ChannelPane = React.memo(function ChannelPane({
|
||||
activeChannel,
|
||||
activeVideoReviewCommentSender,
|
||||
isSending,
|
||||
messages,
|
||||
onToggleReaction,
|
||||
profiles,
|
||||
threadAllMessages,
|
||||
threadHeadMessage,
|
||||
videoReviewCommentsByRootId,
|
||||
]);
|
||||
|
||||
const isOverlay = useIsThreadPanelOverlay();
|
||||
@@ -876,7 +876,9 @@ export const ChannelPane = React.memo(function ChannelPane({
|
||||
scrollTargetHighlights={!layoutScrollTargetId}
|
||||
scrollTargetId={layoutScrollTargetId ?? threadScrollTargetId}
|
||||
threadHead={threadHeadMessage}
|
||||
threadHeadVideoReviewContext={threadHeadVideoReviewContext}
|
||||
videoReviewContextsByMessageId={
|
||||
threadVideoReviewContextsByMessageId
|
||||
}
|
||||
widthPx={threadPanelWidthPx}
|
||||
threadReplies={threadMessages}
|
||||
threadRepliesPending={threadMessagesPending}
|
||||
|
||||
@@ -151,6 +151,7 @@ export type ChannelPaneProps = {
|
||||
profilePanelTab: ProfilePanelTab;
|
||||
profilePanelView: ProfilePanelView;
|
||||
threadHeadMessage: TimelineMessage | null;
|
||||
threadAllMessages: TimelineMessage[];
|
||||
threadMessages: MainTimelineEntry[];
|
||||
threadMessagesPending?: boolean;
|
||||
threadPanelWidthPx: number;
|
||||
|
||||
@@ -691,6 +691,7 @@ export function ChannelScreen({
|
||||
channelManagementOpen,
|
||||
);
|
||||
const displayedThreadHeadMessage = threadPanelData.threadHead;
|
||||
const displayedThreadAllMessages = threadPanelData.messages;
|
||||
const displayedThreadMessages = threadPanelData.visibleReplies;
|
||||
const displayedThreadReplyTargetMessage = threadPanelData.replyTargetMessage;
|
||||
const displayedThreadFirstUnreadReplyId = displayedThreadHeadMessage
|
||||
@@ -944,6 +945,7 @@ export function ChannelScreen({
|
||||
firstUnreadMessageId={firstUnreadMessageId}
|
||||
unreadCount={unreadCount}
|
||||
targetMessageId={mainTimelineTargetMessageId}
|
||||
threadAllMessages={displayedThreadAllMessages}
|
||||
threadHeadMessage={displayedThreadHeadMessage}
|
||||
threadMessages={displayedThreadMessages}
|
||||
threadMessagesPending={threadRepliesQuery.isPending}
|
||||
|
||||
@@ -11,16 +11,18 @@ export function buildIndependentThreadPanel(
|
||||
...formatArgs: Tail<Parameters<typeof formatTimelineMessages>>
|
||||
) {
|
||||
if (!rootId) {
|
||||
return buildThreadPanelData([], null, replyTargetId, expandedReplyIds);
|
||||
return {
|
||||
...buildThreadPanelData([], null, replyTargetId, expandedReplyIds),
|
||||
messages: [],
|
||||
};
|
||||
}
|
||||
const head = channelEvents.find((event) => event.id === rootId);
|
||||
const events = head ? [head, ...replyEvents] : replyEvents;
|
||||
return buildThreadPanelData(
|
||||
formatTimelineMessages(events, ...formatArgs),
|
||||
rootId,
|
||||
replyTargetId,
|
||||
expandedReplyIds,
|
||||
);
|
||||
const messages = formatTimelineMessages(events, ...formatArgs);
|
||||
return {
|
||||
...buildThreadPanelData(messages, rootId, replyTargetId, expandedReplyIds),
|
||||
messages,
|
||||
};
|
||||
}
|
||||
|
||||
type Tail<T extends readonly unknown[]> = T extends readonly [
|
||||
|
||||
@@ -5,6 +5,7 @@ import {
|
||||
buildVideoReviewCommentsByRootId,
|
||||
buildVideoReviewCommentsForRoot,
|
||||
buildVideoReviewContextForMessage,
|
||||
buildVideoReviewContextsByMessageId,
|
||||
hasVideoAttachment,
|
||||
} from "./videoReviewContext.ts";
|
||||
|
||||
@@ -209,3 +210,30 @@ test("buildVideoReviewContextForMessage posts against the source video", async (
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
test("buildVideoReviewContextsByMessageId includes video replies", () => {
|
||||
const root = message({ id: "root", body: "Review request" });
|
||||
const videoReply = message({
|
||||
id: "video-reply",
|
||||
body: "",
|
||||
parentId: root.id,
|
||||
rootId: root.id,
|
||||
});
|
||||
const comment = message({
|
||||
id: "comment",
|
||||
body: "[00:01] tighten this",
|
||||
parentId: videoReply.id,
|
||||
rootId: root.id,
|
||||
});
|
||||
|
||||
const contexts = buildVideoReviewContextsByMessageId({
|
||||
channelId: "channel",
|
||||
messages: [root, videoReply, comment],
|
||||
});
|
||||
|
||||
assert.deepEqual([...contexts.keys()], [videoReply.id]);
|
||||
assert.deepEqual(
|
||||
contexts.get(videoReply.id)?.comments.map((item) => item.id),
|
||||
[comment.id],
|
||||
);
|
||||
});
|
||||
|
||||
@@ -148,3 +148,48 @@ export function buildVideoReviewContextForMessage({
|
||||
rootEventId: message.id,
|
||||
};
|
||||
}
|
||||
|
||||
export function buildVideoReviewContextsByMessageId({
|
||||
channelId,
|
||||
channelName,
|
||||
channelType,
|
||||
isSendingVideoReviewComment = false,
|
||||
messages,
|
||||
onSendVideoReviewComment,
|
||||
onToggleReaction,
|
||||
profiles,
|
||||
}: {
|
||||
channelId?: string | null;
|
||||
channelName?: string;
|
||||
channelType?: ChannelType | null;
|
||||
isSendingVideoReviewComment?: boolean;
|
||||
messages: TimelineMessage[];
|
||||
onSendVideoReviewComment?: SendVideoReviewComment;
|
||||
onToggleReaction?: ToggleMessageReaction;
|
||||
profiles?: UserProfileLookup;
|
||||
}): ReadonlyMap<string, VideoReviewContext> {
|
||||
const contexts = new Map<string, VideoReviewContext>();
|
||||
if (!messages.some(hasVideoAttachment)) {
|
||||
return contexts;
|
||||
}
|
||||
|
||||
const commentsByRootId = buildVideoReviewCommentsByRootId(messages);
|
||||
for (const message of messages) {
|
||||
const context = buildVideoReviewContextForMessage({
|
||||
channelId,
|
||||
channelName,
|
||||
channelType,
|
||||
comments: commentsByRootId.get(message.id) ?? [],
|
||||
isSendingVideoReviewComment,
|
||||
message,
|
||||
onSendVideoReviewComment,
|
||||
onToggleReaction,
|
||||
profiles,
|
||||
});
|
||||
if (context) {
|
||||
contexts.set(message.id, context);
|
||||
}
|
||||
}
|
||||
|
||||
return contexts;
|
||||
}
|
||||
|
||||
@@ -108,7 +108,7 @@ type MessageThreadPanelProps = ThreadPanelLayoutProps & {
|
||||
threadUnreadCount?: number;
|
||||
threadReplyUnreadCounts?: ReadonlyMap<string, number>;
|
||||
threadTypingPubkeys: string[];
|
||||
threadHeadVideoReviewContext?: VideoReviewContext;
|
||||
videoReviewContextsByMessageId?: ReadonlyMap<string, VideoReviewContext>;
|
||||
activityAccessoryContent?: React.ReactNode;
|
||||
activityAccessoryVisible: boolean;
|
||||
widthPx: number;
|
||||
@@ -221,7 +221,7 @@ export function MessageThreadPanel({
|
||||
scrollTargetId,
|
||||
scrollTargetHighlights = true,
|
||||
threadHead,
|
||||
threadHeadVideoReviewContext,
|
||||
videoReviewContextsByMessageId,
|
||||
threadReplies,
|
||||
threadRepliesPending = false,
|
||||
threadUnreadCount,
|
||||
@@ -600,7 +600,9 @@ export function MessageThreadPanel({
|
||||
}
|
||||
profiles={profiles}
|
||||
showDepthGuides={shouldShowThreadBranchGuides}
|
||||
videoReviewContext={threadHeadVideoReviewContext}
|
||||
videoReviewContext={videoReviewContextsByMessageId?.get(
|
||||
threadHead.id,
|
||||
)}
|
||||
/>
|
||||
</div>
|
||||
</div>
|
||||
@@ -756,6 +758,9 @@ export function MessageThreadPanel({
|
||||
onToggleReaction={onToggleReaction}
|
||||
profiles={profiles}
|
||||
showDepthGuides={shouldShowThreadBranchGuides}
|
||||
videoReviewContext={videoReviewContextsByMessageId?.get(
|
||||
entry.message.id,
|
||||
)}
|
||||
/>
|
||||
{entry.summary ? (
|
||||
<MessageThreadSummaryRow
|
||||
|
||||
@@ -22,11 +22,8 @@ import { THREAD_REPLY_ROW_MARGIN_INLINE_REM } from "@/features/messages/lib/thre
|
||||
import { buildMainTimelineEntries } from "@/features/messages/lib/threadPanel";
|
||||
import type { MainTimelineEntry } from "@/features/messages/lib/threadPanel";
|
||||
import type { ChannelWindowThreadSummary } from "@/features/messages/lib/channelWindowStore";
|
||||
import {
|
||||
buildVideoReviewCommentsByRootId,
|
||||
buildVideoReviewContextForMessage,
|
||||
hasVideoAttachment,
|
||||
} from "@/features/messages/lib/videoReviewContext";
|
||||
import { buildVideoReviewContextsByMessageId } from "@/features/messages/lib/videoReviewContext";
|
||||
import type { buildVideoReviewContextForMessage } from "@/features/messages/lib/videoReviewContext";
|
||||
import type { TimelineMessage } from "@/features/messages/types";
|
||||
import { canManageMessageForCurrentUser } from "@/features/messages/lib/canManageMessage";
|
||||
import type { UserProfileLookup } from "@/features/profile/lib/identity";
|
||||
@@ -170,40 +167,21 @@ export const TimelineMessageList = React.memo(function TimelineMessageList({
|
||||
buildMainTimelineEntries(messages, undefined, threadSummaries, profiles),
|
||||
[mainEntries, messages, profiles, threadSummaries],
|
||||
);
|
||||
const reviewCommentsByRootId = React.useMemo(
|
||||
() =>
|
||||
messages.some(hasVideoAttachment)
|
||||
? buildVideoReviewCommentsByRootId(messages)
|
||||
: new Map<string, TimelineMessage[]>(),
|
||||
[messages],
|
||||
);
|
||||
// Contexts are memoized per message id so MessageRow/Markdown memo
|
||||
// comparisons hold across unrelated timeline re-renders (typing
|
||||
// indicators, presence updates) — a fresh context object per render would
|
||||
// defeat the memo and re-render every video message on every pass.
|
||||
const videoReviewContextById = React.useMemo(() => {
|
||||
const contexts = new Map<
|
||||
string,
|
||||
NonNullable<ReturnType<typeof buildVideoReviewContextForMessage>>
|
||||
>();
|
||||
for (const message of messages) {
|
||||
const comments = reviewCommentsByRootId.get(message.id) ?? [];
|
||||
const context = buildVideoReviewContextForMessage({
|
||||
channelId,
|
||||
channelName,
|
||||
channelType,
|
||||
comments,
|
||||
isSendingVideoReviewComment,
|
||||
message,
|
||||
onSendVideoReviewComment,
|
||||
onToggleReaction,
|
||||
profiles,
|
||||
});
|
||||
if (context) {
|
||||
contexts.set(message.id, context);
|
||||
}
|
||||
}
|
||||
return contexts;
|
||||
return buildVideoReviewContextsByMessageId({
|
||||
channelId,
|
||||
channelName,
|
||||
channelType,
|
||||
isSendingVideoReviewComment,
|
||||
messages,
|
||||
onSendVideoReviewComment,
|
||||
onToggleReaction,
|
||||
profiles,
|
||||
});
|
||||
}, [
|
||||
channelId,
|
||||
channelName,
|
||||
@@ -213,7 +191,6 @@ export const TimelineMessageList = React.memo(function TimelineMessageList({
|
||||
onSendVideoReviewComment,
|
||||
onToggleReaction,
|
||||
profiles,
|
||||
reviewCommentsByRootId,
|
||||
]);
|
||||
|
||||
// The flattened item stream, memoized on the entries and the unread boundary
|
||||
|
||||
@@ -53,25 +53,31 @@ function emitMockMessage(
|
||||
page: Page,
|
||||
channelName: string,
|
||||
content: string,
|
||||
options: { extraTags?: string[][] } = {},
|
||||
options: { extraTags?: string[][]; parentEventId?: string } = {},
|
||||
) {
|
||||
return page.evaluate(
|
||||
({ channelName, content, extraTags }) => {
|
||||
({ channelName, content, extraTags, parentEventId }) => {
|
||||
const emit = (
|
||||
window as Window & {
|
||||
__BUZZ_E2E_EMIT_MOCK_MESSAGE__?: (input: {
|
||||
channelName: string;
|
||||
content: string;
|
||||
extraTags?: string[][];
|
||||
parentEventId?: string;
|
||||
}) => unknown;
|
||||
}
|
||||
).__BUZZ_E2E_EMIT_MOCK_MESSAGE__;
|
||||
if (!emit) {
|
||||
throw new Error("Mock message emitter is unavailable.");
|
||||
}
|
||||
emit({ channelName, content, extraTags });
|
||||
return emit({ channelName, content, extraTags, parentEventId });
|
||||
},
|
||||
{
|
||||
channelName,
|
||||
content,
|
||||
extraTags: options.extraTags,
|
||||
parentEventId: options.parentEventId,
|
||||
},
|
||||
{ channelName, content, extraTags: options.extraTags },
|
||||
);
|
||||
}
|
||||
|
||||
@@ -765,6 +771,55 @@ test("video upload previews use poster frames and inline videos open review mode
|
||||
).toContainText("Color pass looks right");
|
||||
});
|
||||
|
||||
test("video replies in threads open the review comments view", async ({
|
||||
page,
|
||||
}) => {
|
||||
await installVideoReviewHarness(page);
|
||||
|
||||
await page.goto("/");
|
||||
await page.getByTestId("channel-general").click();
|
||||
await expect(page.getByTestId("chat-title")).toHaveText("general");
|
||||
await waitForMockLiveSubscription(page, "general");
|
||||
|
||||
const root = (await emitMockMessage(
|
||||
page,
|
||||
"general",
|
||||
"Can you review this cut?",
|
||||
)) as { id: string };
|
||||
const videoReply = (await emitMockMessage(
|
||||
page,
|
||||
"general",
|
||||
``,
|
||||
{
|
||||
parentEventId: root.id,
|
||||
},
|
||||
)) as { id: string };
|
||||
await emitMockMessage(page, "general", "[00:01] Tighten this transition.", {
|
||||
parentEventId: videoReply.id,
|
||||
});
|
||||
|
||||
const threadSummary = page.locator(`[data-thread-head-id="${root.id}"]`);
|
||||
await expect(threadSummary).toBeVisible();
|
||||
await threadSummary.click();
|
||||
|
||||
const threadPanel = page.getByTestId("message-thread-panel");
|
||||
const threadReplies = threadPanel.getByTestId("message-thread-replies");
|
||||
const reviewButton = threadReplies.getByRole("button", {
|
||||
name: "Open video review",
|
||||
});
|
||||
await expect(reviewButton).toBeVisible();
|
||||
await reviewButton.click();
|
||||
|
||||
const reviewDialog = page.getByTestId("video-review-dialog");
|
||||
await expect(
|
||||
reviewDialog.getByTestId("video-review-comments-panel"),
|
||||
).toBeVisible();
|
||||
await expect(reviewDialog.getByTestId("message-composer")).toBeVisible();
|
||||
await expect(reviewDialog.getByTestId("video-review-comments")).toContainText(
|
||||
"Tighten this transition.",
|
||||
);
|
||||
});
|
||||
|
||||
test("narrow inline videos hide playback speed control", async ({ page }) => {
|
||||
await installVideoReviewHarness(page);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user