From a8b38d7d511d48e389828569fa2d054cd27f1bbf Mon Sep 17 00:00:00 2001 From: Shy Alter Date: Wed, 29 Jul 2026 08:12:36 +0200 Subject: [PATCH 1/6] fix(ui): even out activity pane avatars and text, split PR card open targets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The activity timeline mixed a `sm` avatar on the "created this task" row with `lg` ones on every row below it, and quill's `ThreadItemAuthor`/`ThreadItemBody` default to 14px while the pane's own copy is 13px — so a row's name read as a heading over its own message. - Every timeline node is now a `sm` avatar (or the matching `size-6` status bubble), centered in the 2.5rem gutter; the vertical line moves to 1.75rem to stay under them. - Author names and body copy share one 13px size across the pane. - PR cards in the timeline and the Artifacts tab now open the in-app review split when clicked, with a dedicated trailing button for GitHub. Previously the timeline card only ever left the app and the artifacts row only ever stayed in it. Generated-By: PostHog Code Task-Id: 3da5213f-90f2-4123-bbbd-c648c7b08e06 --- .../canvas/components/ActivityPanel.tsx | 1 + .../canvas/components/ActivityTimeline.tsx | 26 +++--- .../canvas/components/TaskArtifactsList.tsx | 66 +++++++------ .../canvas/components/ThreadPanel.test.tsx | 27 +++++- .../canvas/components/ThreadPanel.tsx | 93 ++++++++++++++----- .../features/code-review/openPrInReview.ts | 10 ++ 6 files changed, 157 insertions(+), 66 deletions(-) create mode 100644 packages/ui/src/features/code-review/openPrInReview.ts diff --git a/packages/ui/src/features/canvas/components/ActivityPanel.tsx b/packages/ui/src/features/canvas/components/ActivityPanel.tsx index 67216759df..4831c82e8b 100644 --- a/packages/ui/src/features/canvas/components/ActivityPanel.tsx +++ b/packages/ui/src/features/canvas/components/ActivityPanel.tsx @@ -189,6 +189,7 @@ function ActivityConversation({ return ( -
-
{node}
-
- - {title} +
{node}
+ + {title} {action && {action}} @@ -73,18 +73,18 @@ function UserMessageRow({ }) { return ( - - + + - + {author ? userDisplayName(author) : "You"} - - + + {content} @@ -177,6 +177,7 @@ export function ActivityTimeline({ ), }); @@ -192,6 +193,7 @@ export function ActivityTimeline({ ), }); @@ -246,9 +248,11 @@ export function ActivityTimeline({ return (
+ {/* Every row centers its node in a 2.5rem gutter inset by the row's + 0.5rem padding, so the line runs through 0.5 + 2.5/2 = 1.75rem. */}
diff --git a/packages/ui/src/features/canvas/components/TaskArtifactsList.tsx b/packages/ui/src/features/canvas/components/TaskArtifactsList.tsx index 4ac3e526f4..ae7c3318cc 100644 --- a/packages/ui/src/features/canvas/components/TaskArtifactsList.tsx +++ b/packages/ui/src/features/canvas/components/TaskArtifactsList.tsx @@ -23,7 +23,7 @@ import type { } from "@posthog/shared/domain-types"; import { iconForTemplate } from "@posthog/ui/features/canvas/components/canvasTemplateIcon"; import { useTaskRuns } from "@posthog/ui/features/canvas/hooks/useTaskRuns"; -import { useReviewNavigationStore } from "@posthog/ui/features/code-review/reviewNavigationStore"; +import { openPrInReview } from "@posthog/ui/features/code-review/openPrInReview"; import { usePrArtifact } from "@posthog/ui/features/git-interaction/usePrArtifact"; import { usePanelLayoutStore } from "@posthog/ui/features/panels/panelLayoutStore"; import { usePrComments } from "@posthog/ui/features/pr-review/usePrComments"; @@ -130,6 +130,8 @@ function ArtifactListRow({ detail, external, onOpen, + onOpenExternal, + externalLabel, onHoverStart, }: { icon: ReactNode; @@ -137,33 +139,48 @@ function ArtifactListRow({ detail?: string | null; external?: boolean; onOpen?: () => void; + /** Renders a trailing button that leaves the app instead of opening the + * artifact in place. Absent when there is nowhere safe to send the user. */ + onOpenExternal?: () => void; + externalLabel?: string; onHoverStart?: () => void; }) { return ( - + {onOpenExternal && ( + )} - {external && ( - - )} - +
); } function PrRow({ url, taskId }: { url: string; taskId: string }) { const { safeUrl, title, stateLabel, Icon, iconColor } = usePrArtifact(url); - const setReviewMode = useReviewNavigationStore((s) => s.setReviewMode); - const setSelectedPrUrl = useReviewNavigationStore((s) => s.setSelectedPrUrl); const [countsWanted, setCountsWanted] = useState(false); const comments = usePrComments(countsWanted ? safeUrl : null); @@ -195,14 +212,9 @@ function PrRow({ url, taskId }: { url: string; taskId: string }) { title={title} detail={detailParts.join(" · ") || null} onHoverStart={() => setCountsWanted(true)} - onOpen={ - safeUrl - ? () => { - setSelectedPrUrl(taskId, safeUrl); - setReviewMode(taskId, "split"); - } - : undefined - } + onOpen={safeUrl ? () => openPrInReview(taskId, safeUrl) : undefined} + onOpenExternal={safeUrl ? () => openExternalUrl(safeUrl) : undefined} + externalLabel={`Open ${title} on GitHub`} /> ); } diff --git a/packages/ui/src/features/canvas/components/ThreadPanel.test.tsx b/packages/ui/src/features/canvas/components/ThreadPanel.test.tsx index eb3d49c5e1..0966052432 100644 --- a/packages/ui/src/features/canvas/components/ThreadPanel.test.tsx +++ b/packages/ui/src/features/canvas/components/ThreadPanel.test.tsx @@ -1,3 +1,4 @@ +import { useReviewNavigationStore } from "@posthog/ui/features/code-review/reviewNavigationStore"; import { fireEvent, render, screen } from "@testing-library/react"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { @@ -80,6 +81,7 @@ describe("ThreadArtifactRow", () => { url: "https://us.posthog.com/code/canvas/channel-1/dash-1", }} createdAt="2026-07-17T00:00:00Z" + taskId="task-1" />, ); @@ -101,6 +103,7 @@ describe("ThreadArtifactRow", () => { , ); @@ -117,6 +120,7 @@ describe("ThreadArtifactRow", () => { , ); @@ -126,21 +130,32 @@ describe("ThreadArtifactRow", () => { expect(navigateToShareTarget).not.toHaveBeenCalled(); }); - it("renders a pull request artifact and opens it externally", () => { + it("opens a pull request in the review pane, and on GitHub from its own button", () => { + const url = "https://github.com/org/repo/pull/123"; + render( , ); expect(screen.getByText("Pull request #123")).toBeInTheDocument(); - fireEvent.click(screen.getByRole("button", { name: /Pull request #123/ })); + // The card's own name leads with the title; the GitHub button's trails it. + fireEvent.click(screen.getByRole("button", { name: /^Pull request #123/ })); + + const review = useReviewNavigationStore.getState(); + expect(review.selectedPrUrls["task-1"]).toBe(url); + expect(review.reviewModes["task-1"]).toBe("split"); + expect(openExternalUrl).not.toHaveBeenCalled(); - expect(openExternalUrl).toHaveBeenCalledWith( - "https://github.com/org/repo/pull/123", + fireEvent.click( + screen.getByRole("button", { name: "Open Pull request #123 on GitHub" }), ); + + expect(openExternalUrl).toHaveBeenCalledWith(url); expect(navigateToShareTarget).not.toHaveBeenCalled(); }); @@ -162,6 +177,7 @@ describe("ThreadArtifactRow", () => { , ); @@ -197,6 +213,7 @@ describe("ThreadArtifactRow", () => { , ); diff --git a/packages/ui/src/features/canvas/components/ThreadPanel.tsx b/packages/ui/src/features/canvas/components/ThreadPanel.tsx index 1070a6ae0b..9239f75488 100644 --- a/packages/ui/src/features/canvas/components/ThreadPanel.tsx +++ b/packages/ui/src/features/canvas/components/ThreadPanel.tsx @@ -17,6 +17,7 @@ import { AvatarFallback, Badge, Button, + cn, DropdownMenu, DropdownMenuContent, DropdownMenuItem, @@ -52,6 +53,7 @@ import { MentionText } from "@posthog/ui/features/canvas/components/MentionText" import { ThreadTimestamp } from "@posthog/ui/features/canvas/components/ThreadTimestamp"; import { useThreadConversation } from "@posthog/ui/features/canvas/hooks/useThreadConversation"; import { userDisplayName } from "@posthog/ui/features/canvas/utils/userDisplay"; +import { openPrInReview } from "@posthog/ui/features/code-review/openPrInReview"; import { usePrArtifact } from "@posthog/ui/features/git-interaction/usePrArtifact"; import { taskDetailQuery } from "@posthog/ui/features/tasks/queries"; import { openExternalUrl } from "@posthog/ui/shell/openExternal"; @@ -61,6 +63,14 @@ import { getPostHogUrl } from "@posthog/ui/utils/urls"; import { useQuery } from "@tanstack/react-query"; import { useEffect, useRef } from "react"; +/** One text size for the whole pane: an author's name reads at the same size as + * the message under it, so no row looks like a heading over its own content. */ +export const THREAD_TEXT_CLASS = "text-[13px]"; + +/** Timeline nodes are all `sm` avatars / `size-6` bubbles, centered in the + * 2.5rem gutter so every row's node sits on the timeline's vertical line. */ +export const THREAD_GUTTER_CLASS = "justify-center"; + export function ThreadMessageRow({ message, isTaskAuthor, @@ -83,15 +93,17 @@ export function ThreadMessageRow({ return ( - - + + - {userDisplayName(message.author)} + + {userDisplayName(message.author)} + - + void; + /** Renders a trailing button that leaves the app instead of opening the + * artifact in place. Absent when there is nowhere safe to send the user. */ + onOpenExternal?: () => void; + externalLabel?: string; }) { const body = ( <> @@ -174,19 +192,40 @@ function ArtifactCardButton({ )} ); - const cardClass = - "flex w-fit max-w-full items-center gap-2 rounded-md border border-border bg-muted px-2 py-1.5 text-[13px]"; - if (!onOpen) { - return {body}; - } + const innerClass = "flex min-w-0 items-center gap-2 px-2 py-1.5"; return ( - + {onOpen ? ( + + ) : ( + {body} + )} + {onOpenExternal && ( + + )} +
); } @@ -222,7 +261,7 @@ function CanvasArtifactCard({ ); } -function PrArtifactCard({ url }: { url: string }) { +function PrArtifactCard({ url, taskId }: { url: string; taskId: string }) { const { safeUrl, title, stateLabel, Icon, iconColor } = usePrArtifact(url); return ( openExternalUrl(safeUrl) : undefined} + onOpen={safeUrl ? () => openPrInReview(taskId, safeUrl) : undefined} + onOpenExternal={safeUrl ? () => openExternalUrl(safeUrl) : undefined} + externalLabel={`Open ${title} on GitHub`} /> ); } @@ -244,31 +285,33 @@ function PrArtifactCard({ url }: { url: string }) { export function ThreadArtifactRow({ artifact, createdAt, + taskId, }: { artifact: ThreadArtifact; createdAt: string; + taskId: string; }) { return ( - - + + - + - + {artifact.kind === "canvas" ? "Canvas" : "Pull request"} - + {artifact.kind === "canvas" ? ( ) : ( - + )} @@ -343,6 +386,7 @@ function ThreadPanelHeader({ export function ThreadTimeline({ timeline, + taskId, isReady, currentUserUuid, currentUserEmail, @@ -352,6 +396,7 @@ export function ThreadTimeline({ onDelete, }: { timeline: ThreadTimelineRow[]; + taskId: string; isReady: boolean; currentUserUuid?: string; currentUserEmail?: string; @@ -400,6 +445,7 @@ export function ThreadTimeline({ key={row.message.id} artifact={row.artifact} createdAt={row.message.created_at} + taskId={taskId} /> ), )} @@ -513,6 +559,7 @@ function ThreadConversation({
Date: Wed, 29 Jul 2026 08:45:05 +0200 Subject: [PATCH 2/6] fix(ui): normalise activity rows to two species and preview one line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pane had three typographic treatments and two row skeletons competing, so nothing lined up. It now has exactly two row species: - Lifecycle events (task created, run finished) are an icon bubble plus a single muted line. "Task created" gets its own icon rather than the creator's avatar, so it reads the same as "Task completed" instead of impersonating an authored row. - Authored rows (messages, PR/canvas artifacts) keep the avatar and author line, with content on its own line below. Also: - Message rows preview the first non-empty line and truncate, rather than clamping four lines of wrapped text. The Comments tab still shows messages in full — that's where you read them. - Content sits 0.375rem below its header, so the PR card is no longer jammed against its title. - Timestamps ride a right-edge column via `ThreadTimestamp`'s className. The panel-level `[data-slot=thread-item-timestamp]` override this replaces never matched: quill's `TooltipTrigger` overwrites the wrapped element's `data-slot` with its own, which is why timestamps had been hugging the names. Generated-By: PostHog Code Task-Id: 3da5213f-90f2-4123-bbbd-c648c7b08e06 --- .../canvas/components/ActivityPanel.tsx | 12 +- .../canvas/components/ActivityTimeline.tsx | 111 ++++++++++-------- .../canvas/components/ThreadPanel.test.tsx | 88 ++++++++++++++ .../canvas/components/ThreadPanel.tsx | 49 +++++++- .../canvas/components/ThreadTimestamp.tsx | 13 +- 5 files changed, 204 insertions(+), 69 deletions(-) diff --git a/packages/ui/src/features/canvas/components/ActivityPanel.tsx b/packages/ui/src/features/canvas/components/ActivityPanel.tsx index 4831c82e8b..fbc423e321 100644 --- a/packages/ui/src/features/canvas/components/ActivityPanel.tsx +++ b/packages/ui/src/features/canvas/components/ActivityPanel.tsx @@ -3,7 +3,7 @@ import { CaretRightIcon, XIcon, } from "@phosphor-icons/react"; -import { Button, cn, Tabs, TabsList, TabsTrigger } from "@posthog/quill"; +import { Button, Tabs, TabsList, TabsTrigger } from "@posthog/quill"; import { ANALYTICS_EVENTS } from "@posthog/shared/analytics-events"; import type { Task } from "@posthog/shared/domain-types"; import { ActivityTimeline } from "@posthog/ui/features/canvas/components/ActivityTimeline"; @@ -35,9 +35,6 @@ const TABS_WITH_COMPOSER: ReadonlySet = new Set([ "comments", ]); -const TIMESTAMP_END_CLASS = - "[&_[data-slot=thread-item-timestamp]]:ml-auto [&_[data-slot=thread-item-timestamp]]:shrink-0 [&_[data-slot=thread-item-timestamp]]:pl-2"; - /** The 32px row this panel leads with: the tabs are the header, so the strip * lines up with the tab bar of the pane on its left (TabbedPanel) and the * review toolbar, which are the same fixed height and border. */ @@ -217,12 +214,7 @@ function ActivityConversation({ }; return ( -
+
["items"][number]; +/** A lifecycle marker (task created, run finished): an icon bubble and a single + * muted line. Deliberately one typographic weight and colour, so events read as + * the timeline's punctuation rather than competing with authored rows. */ function ActivityEventRow({ - node, - title, - action, + icon, + label, timestamp, }: { - node: ReactNode; - title: string; - action?: string; + icon: ReactNode; + label: string; timestamp: string; }) { return (
-
{node}
- - {title} - {action && {action}} +
+ + {icon} + +
+ + {label} - +
); } -function EventNode({ icon }: { icon: ReactNode }) { - return ( - - {icon} - - ); -} - function UserMessageRow({ author, content, @@ -81,12 +93,19 @@ function UserMessageRow({ {author ? userDisplayName(author) : "You"} - + - - - {content} - + + {messagePreview(content)} @@ -122,15 +141,10 @@ export function ActivityTimeline({ ts: createdTs, node: ( + icon={ + } - title={task.created_by ? userDisplayName(task.created_by) : "Someone"} - action="created this task" + label={`${task.created_by ? userDisplayName(task.created_by) : "Someone"} created this task`} timestamp={task.created_at} /> ), @@ -170,6 +184,7 @@ export function ActivityTimeline({ } currentUserEmail={currentUserEmail} canForward={canForward} + preview onSendToAgent={() => onSendToAgent(row.message.id)} onDelete={() => onDelete(row.message.id)} /> @@ -207,26 +222,18 @@ export function ActivityTimeline({ ts: updatedTs + 1, node: ( - ) : ( - - ) - } - /> + icon={ + succeeded ? ( + + ) : ( + + ) } - title={`Task ${runStatus.replace(/_/g, " ")}`} + label={`Task ${runStatus.replace(/_/g, " ")}`} timestamp={task.updated_at} /> ), diff --git a/packages/ui/src/features/canvas/components/ThreadPanel.test.tsx b/packages/ui/src/features/canvas/components/ThreadPanel.test.tsx index 0966052432..c36f52d44d 100644 --- a/packages/ui/src/features/canvas/components/ThreadPanel.test.tsx +++ b/packages/ui/src/features/canvas/components/ThreadPanel.test.tsx @@ -3,6 +3,7 @@ import { fireEvent, render, screen } from "@testing-library/react"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { AgentStatusLine, + messagePreview, ThreadArtifactRow, ThreadMessageRow, } from "./ThreadPanel"; @@ -69,6 +70,93 @@ describe("ThreadMessageRow", () => { screen.getByRole("button", { name: "Message actions" }), ).toBeInTheDocument(); }); + + const multiline = "First line\n\nSecond line with more detail"; + + it("shows the whole message by default, so comments stay readable", () => { + render( + {}} + onDelete={() => {}} + />, + ); + + expect( + screen.getByText(/Second line with more detail/), + ).toBeInTheDocument(); + }); + + it("previews only the first line in a timeline row", () => { + render( + {}} + onDelete={() => {}} + />, + ); + + expect(screen.getByText("First line")).toBeInTheDocument(); + expect(screen.queryByText(/Second line/)).not.toBeInTheDocument(); + }); + + it("puts the timestamp in a column away from the author name", () => { + render( + {}} + onDelete={() => {}} + />, + ); + + // Guards the quill seam: TooltipTrigger overwrites `data-slot`, so alignment + // has to ride on className rather than an ancestor [data-slot] rule. + const time = document.querySelector("time"); + expect(time).not.toBeNull(); + expect(time).toHaveClass("ml-auto"); + }); +}); + +describe("messagePreview", () => { + it.each([ + [ + "takes the first non-empty line", + "\n\n Hello there \nignored", + "Hello there", + ], + ["passes a single line through", "just this", "just this"], + ["returns empty for blank content", "\n \n", ""], + ])("%s", (_, input, expected) => { + expect(messagePreview(input)).toBe(expected); + }); }); describe("ThreadArtifactRow", () => { diff --git a/packages/ui/src/features/canvas/components/ThreadPanel.tsx b/packages/ui/src/features/canvas/components/ThreadPanel.tsx index 9239f75488..b1e4f747b3 100644 --- a/packages/ui/src/features/canvas/components/ThreadPanel.tsx +++ b/packages/ui/src/features/canvas/components/ThreadPanel.tsx @@ -71,12 +71,33 @@ export const THREAD_TEXT_CLASS = "text-[13px]"; * 2.5rem gutter so every row's node sits on the timeline's vertical line. */ export const THREAD_GUTTER_CLASS = "justify-center"; +/** Timestamps sit at the row's right edge, so they form a column instead of + * trailing names of varying length. Applied per row rather than as an ancestor + * `[data-slot]` rule, which `TooltipTrigger` defeats — see `ThreadTimestamp`. */ +export const THREAD_TIMESTAMP_CLASS = "ml-auto shrink-0 pl-2"; + +/** Content sits a little below its author line, so a card or a message never + * looks jammed against the name above it. */ +export const THREAD_BODY_SPACING_CLASS = "mt-1.5"; + +/** The first non-empty line of a message. Timeline rows preview one line and + * truncate, so neither a leading blank line nor a wall of text makes a row + * tall enough to bury the entries around it. */ +export function messagePreview(content: string): string { + for (const line of content.split("\n")) { + const trimmed = line.trim(); + if (trimmed) return trimmed; + } + return ""; +} + export function ThreadMessageRow({ message, isTaskAuthor, isOwnMessage, currentUserEmail, canForward, + preview, onSendToAgent, onDelete, }: { @@ -85,6 +106,8 @@ export function ThreadMessageRow({ isOwnMessage: boolean; currentUserEmail?: string | null; canForward: boolean; + /** Timeline rows show one truncated line; the Comments tab shows it all. */ + preview?: boolean; onSendToAgent: () => void; onDelete: () => void; }) { @@ -101,11 +124,22 @@ export function ThreadMessageRow({ {userDisplayName(message.author)} - + - + @@ -305,9 +339,14 @@ export function ThreadArtifactRow({ {artifact.kind === "canvas" ? "Canvas" : "Pull request"} - + - + {artifact.kind === "canvas" ? ( ) : ( diff --git a/packages/ui/src/features/canvas/components/ThreadTimestamp.tsx b/packages/ui/src/features/canvas/components/ThreadTimestamp.tsx index ef754f19e8..adad947a43 100644 --- a/packages/ui/src/features/canvas/components/ThreadTimestamp.tsx +++ b/packages/ui/src/features/canvas/components/ThreadTimestamp.tsx @@ -24,7 +24,16 @@ function formatTooltip(date: Date): string { return `${month} ${ordinal(date.getDate())} at ${formatClock(date)}`; } -export function ThreadTimestamp({ dateTime }: { dateTime: string }) { +// `className` is the only styling seam that reaches the rendered element: +// `TooltipTrigger` replaces the wrapped element's `data-slot` with its own, so an +// ancestor `[data-slot=thread-item-timestamp]` rule never matches it. +export function ThreadTimestamp({ + dateTime, + className, +}: { + dateTime: string; + className?: string; +}) { const date = new Date(dateTime); if (Number.isNaN(date.getTime())) return null; @@ -33,7 +42,7 @@ export function ThreadTimestamp({ dateTime }: { dateTime: string }) { + {formatClock(date)} } From cb4909737e343c7a34e3daa3165e19040d0deaa0 Mon Sep 17 00:00:00 2001 From: Shy Alter Date: Wed, 29 Jul 2026 08:59:32 +0200 Subject: [PATCH 3/6] feat(ui): jump the transcript from an activity row, and finish the type pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Typography: - Timestamps drop to 11px, a step below the row's 13px copy. - Event rows take the same text colour as every other row. Only the icon distinguishes them now, so the pane reads as one typographic system rather than greying out its own lifecycle markers. Clicking a message row scrolls the transcript to that message. The Activity pane is a sibling of the transcript, so it can't reach the scroller's context or the windowed body's jump callback — a small `threadNavigationStore` carries the request and each transcript serves it with its own jump, mirroring how `reviewNavigationStore` already brokers scroll-to-file between the changes list and the review pane. Both transcripts are wired, since which one renders depends on the `useNewChatThread` setting: `ConversationView` reuses its existing `handleJumpToMessage` (grouped-row index), and `ChatThread` gets a bridge inside `ChatMessageScrollerProvider` that prefers the windowed body's `jumpToMessage` and falls back to the engine's `scrollToMessage`. Both panes derive items from the same `useSessionViewState` events, so the conversation item ids match. Only message rows are clickable — lifecycle events and artifacts have nothing to scroll to. The row carries the button role itself: quill's `ThreadItem` renders an
, which a
- - {label} - + {label} void; }) { + const name = author ? userDisplayName(author) : "You"; + // The row itself is the hit target. `ThreadItem` renders an
, which a + //
- {label} - + {label} +
); } @@ -102,27 +94,18 @@ function UserMessageRow({ className={cn("rounded-none", onSelect && "cursor-pointer")} {...activation} > - + - - {name} - - + {name} + - - {messagePreview(content)} + {/* `whitespace-pre-wrap` makes the clamp land on the first *written* + line rather than the first wrapped one. */} + + {content} @@ -137,6 +120,7 @@ export function ActivityTimeline({ currentUserEmail, isTaskAuthor, canForward, + canOpenInPlace, onSendToAgent, onDelete, }: { @@ -147,6 +131,10 @@ export function ActivityTimeline({ currentUserEmail?: string | null; isTaskAuthor: boolean; canForward: boolean; + /** True when the task's transcript and review pane are mounted beside this + * pane. False in the channel-home sidebar, where there is nothing to drive — + * rows there stay inert and PRs open externally instead of dead-clicking. */ + canOpenInPlace?: boolean; onSendToAgent: (messageId: string) => void; onDelete: (messageId: string) => void; }) { @@ -181,7 +169,11 @@ export function ActivityTimeline({ author={task.created_by} content={item.content} timestamp={new Date(item.timestamp).toISOString()} - onSelect={() => requestScrollToMessage(task.id, item.id)} + onSelect={ + canOpenInPlace + ? () => requestScrollToMessage(task.id, item.id) + : undefined + } /> ), }); @@ -214,7 +206,7 @@ export function ActivityTimeline({ ), }); @@ -230,7 +222,7 @@ export function ActivityTimeline({ ), }); @@ -273,6 +265,7 @@ export function ActivityTimeline({ currentUserEmail, onSendToAgent, onDelete, + canOpenInPlace, requestScrollToMessage, ]); diff --git a/packages/ui/src/features/canvas/components/TaskArtifactsList.test.tsx b/packages/ui/src/features/canvas/components/TaskArtifactsList.test.tsx index 878222dd48..1f016ebb09 100644 --- a/packages/ui/src/features/canvas/components/TaskArtifactsList.test.tsx +++ b/packages/ui/src/features/canvas/components/TaskArtifactsList.test.tsx @@ -5,6 +5,11 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; const mocks = vi.hoisted(() => ({ runs: [] as TaskRun[], openArtifactTab: vi.fn(), + openExternalUrl: vi.fn(), +})); + +vi.mock("@posthog/ui/shell/openExternal", () => ({ + openExternalUrl: (url: string) => mocks.openExternalUrl(url), })); vi.mock("@posthog/ui/features/canvas/hooks/useTaskRuns", () => ({ @@ -65,6 +70,7 @@ describe("TaskArtifactsList", () => { beforeEach(() => { mocks.runs = [run("run-1", { prNumber: 1 }), run("run-2", { prNumber: 2 })]; mocks.openArtifactTab.mockReset(); + mocks.openExternalUrl.mockReset(); useReviewNavigationStore.setState({ reviewModes: {}, selectedPrUrls: {}, @@ -72,7 +78,7 @@ describe("TaskArtifactsList", () => { }); it("opens the PR represented by the selected historical row", () => { - render(); + render(); fireEvent.click(screen.getByText("Pull request #2")); @@ -81,6 +87,20 @@ describe("TaskArtifactsList", () => { "https://github.com/acme/repo/pull/2", ); expect(state.reviewModes[task.id]).toBe("split"); + expect(mocks.openExternalUrl).not.toHaveBeenCalled(); + }); + + it("opens a PR externally with no review pane alongside to open into", () => { + render(); + + fireEvent.click(screen.getByText("Pull request #2")); + + expect(mocks.openExternalUrl).toHaveBeenCalledWith( + "https://github.com/acme/repo/pull/2", + ); + expect(useReviewNavigationStore.getState().reviewModes[task.id]).toBe( + undefined, + ); }); it("lists the files the agent uploaded, with their size", () => { diff --git a/packages/ui/src/features/canvas/components/TaskArtifactsList.tsx b/packages/ui/src/features/canvas/components/TaskArtifactsList.tsx index ae7c3318cc..5b434478b7 100644 --- a/packages/ui/src/features/canvas/components/TaskArtifactsList.tsx +++ b/packages/ui/src/features/canvas/components/TaskArtifactsList.tsx @@ -131,7 +131,6 @@ function ArtifactListRow({ external, onOpen, onOpenExternal, - externalLabel, onHoverStart, }: { icon: ReactNode; @@ -142,7 +141,6 @@ function ArtifactListRow({ /** Renders a trailing button that leaves the app instead of opening the * artifact in place. Absent when there is nowhere safe to send the user. */ onOpenExternal?: () => void; - externalLabel?: string; onHoverStart?: () => void; }) { return ( @@ -169,7 +167,7 @@ function ArtifactListRow({