diff --git a/apps/web/src/components/ChatMarkdown.tsx b/apps/web/src/components/ChatMarkdown.tsx index f74e5f90..7295480b 100644 --- a/apps/web/src/components/ChatMarkdown.tsx +++ b/apps/web/src/components/ChatMarkdown.tsx @@ -625,7 +625,7 @@ function MarkdownPullRequestChip({ href={href} threadRef={threadRef} title={title} - className="inline-flex items-center gap-1 rounded-sm bg-muted px-1.5 font-mono text-[12px] no-underline transition-colors hover:bg-accent" + className="chat-markdown-pull-request-chip inline-flex items-center gap-1 rounded-sm bg-muted px-1.5 align-middle font-mono text-[12px] transition-colors hover:bg-accent" > #{number} diff --git a/apps/web/src/components/ChatView.browser.tsx b/apps/web/src/components/ChatView.browser.tsx index 635727a0..4cec8a7b 100644 --- a/apps/web/src/components/ChatView.browser.tsx +++ b/apps/web/src/components/ChatView.browser.tsx @@ -2983,12 +2983,30 @@ describe("ChatView timeline estimator parity (full app)", () => { }); it("docks the thread's pull request above the notices in one frame", async () => { + const built = createSnapshotForTargetUser({ + targetMessageId: "msg-user-pull-request-dock" as MessageId, + targetText: "pull request dock", + }); + // The transcript opens at its end, so the address that becomes a chip goes + // in the last message, where it is on screen. + const snapshot: OrchestrationReadModel = { + ...built, + threads: built.threads.map((thread, threadIndex) => + threadIndex === 0 + ? { + ...thread, + messages: thread.messages.map((message, index, all) => + index === all.length - 1 + ? { ...message, text: `Opened ${PULL_REQUEST_URL} for review.` } + : message, + ), + } + : thread, + ), + }; const mounted = await mountChatView({ viewport: DEFAULT_VIEWPORT, - snapshot: createSnapshotForTargetUser({ - targetMessageId: "msg-user-pull-request-dock" as MessageId, - targetText: "pull request dock", - }), + snapshot, configureFixture: withPullRequestFixture, resolveRpc: resolvePullRequestRpc, }); @@ -3034,6 +3052,34 @@ describe("ChatView timeline estimator parity (full app)", () => { dock!.getBoundingClientRect().bottom - composerSurface!.getBoundingClientRect().top, ), ).toBeLessThan(2); + + // The address in the message is a chip, and the chip shares the row's + // hover card: the trigger has to reach the anchor through the link + // component, which is the part that silently broke once. + const chip = await waitForElement( + () => document.querySelector("a.chat-markdown-pull-request-chip"), + "Unable to find the pull request chip in the transcript.", + ); + expect(chip.textContent).toBe(`#${PULL_REQUEST_NUMBER}`); + await page.elementLocator(chip).hover(); + const card = await waitForElement( + () => document.querySelector('[data-testid="pull-request-hover-card"]'), + "Hovering the transcript chip did not open the pull request card.", + ); + expect(card.textContent).toContain(`${PULL_REQUEST_REPOSITORY} #${PULL_REQUEST_NUMBER}`); + // Move off so the card does not sit over the row's close control. + await page.elementLocator(composerSurface!).hover(); + + // Closing the row takes it off the composer; the notice stays docked. + row.querySelector('button[aria-label^="Hide pull request"]')!.click(); + await waitForElement( + () => + document.querySelector('[data-composer-pull-request-row="true"]') === null + ? document.querySelector('[data-composer-notice-dock="true"]') + : null, + "The pull request row did not leave the composer.", + ); + expect(document.querySelector("[data-composer-notice-severity]")).toBeTruthy(); } finally { await mounted.cleanup(); } diff --git a/apps/web/src/components/chat/ChatWebLink.tsx b/apps/web/src/components/chat/ChatWebLink.tsx index 961ebce8..41b126f4 100644 --- a/apps/web/src/components/chat/ChatWebLink.tsx +++ b/apps/web/src/components/chat/ChatWebLink.tsx @@ -1,5 +1,11 @@ import type { ScopedThreadRef } from "@threadlines/contracts"; -import { memo, useCallback, type MouseEvent as ReactMouseEvent, type ReactNode } from "react"; +import { + memo, + useCallback, + type ComponentProps, + type MouseEvent as ReactMouseEvent, + type ReactNode, +} from "react"; import { isElectron } from "../../env"; import { readLocalApi } from "../../localApi"; @@ -9,7 +15,15 @@ import { stackedThreadToast, toastManager } from "../ui/toast"; import { copyTextWithToast } from "./copyTextWithToast"; import { useThreadPullRequestLink } from "./ThreadPullRequestLinkContext"; -export interface ChatWebLinkProps { +/** + * The anchor's own props ride along, `ref` included, so a hover-card trigger + * or tooltip can wrap the link and reach the element: a wrapper that dropped + * them would leave the card with nothing to listen to. + */ +export interface ChatWebLinkProps extends Omit< + ComponentProps<"a">, + "href" | "children" | "className" | "title" +> { href: string; /** Null when the transcript is rendered without a thread to open pages in. */ threadRef: ScopedThreadRef | null; @@ -36,6 +50,9 @@ export const ChatWebLink = memo(function ChatWebLink({ children, className, title, + onClick, + onContextMenu, + ...anchorProps }: ChatWebLinkProps) { const pullRequestLink = useThreadPullRequestLink(); const opensPullRequestTab = @@ -43,7 +60,8 @@ export const ChatWebLink = memo(function ChatWebLink({ const handleClick = useCallback( (event: ReactMouseEvent) => { - if (!isPlainPrimaryClick(event)) { + onClick?.(event); + if (event.defaultPrevented || !isPlainPrimaryClick(event)) { return; } if (opensPullRequestTab) { @@ -61,13 +79,14 @@ export const ChatWebLink = memo(function ChatWebLink({ event.preventDefault(); event.stopPropagation(); }, - [href, opensPullRequestTab, pullRequestLink, threadRef], + [href, onClick, opensPullRequestTab, pullRequestLink, threadRef], ); const handleContextMenu = useCallback( async (event: ReactMouseEvent) => { + onContextMenu?.(event); const api = readLocalApi(); - if (!api) return; + if (!api || event.defaultPrevented) return; event.preventDefault(); event.stopPropagation(); @@ -105,16 +124,17 @@ export const ChatWebLink = memo(function ChatWebLink({ copyTextWithToast(href, "Link address"); } }, - [href, opensPullRequestTab, threadRef], + [href, onContextMenu, opensPullRequestTab, threadRef], ); return ( diff --git a/apps/web/src/components/chat/ComposerPullRequestRow.tsx b/apps/web/src/components/chat/ComposerPullRequestRow.tsx index 745939ed..4f4699f9 100644 --- a/apps/web/src/components/chat/ComposerPullRequestRow.tsx +++ b/apps/web/src/components/chat/ComposerPullRequestRow.tsx @@ -18,13 +18,16 @@ import type { PullRequestRef, } from "@threadlines/contracts"; import { useMutation, useQueryClient } from "@tanstack/react-query"; -import { ChevronDownIcon, ExternalLinkIcon } from "lucide-react"; +import { ChevronDownIcon, ExternalLinkIcon, XIcon } from "lucide-react"; import { useState } from "react"; import { isElectron } from "../../env"; import { useSettings, updateSettings } from "../../hooks/useSettings"; import { readLocalApi } from "../../localApi"; -import { pullRequestActionMutationOptions } from "../../lib/pullRequestsReactQuery"; +import { + pullRequestActionMutationOptions, + pullRequestQueryKeys, +} from "../../lib/pullRequestsReactQuery"; import { cn } from "../../lib/utils"; import { PullRequestHoverCard, @@ -36,7 +39,7 @@ import { Checkbox } from "../ui/checkbox"; import { Popover, PopoverPopup, PopoverTrigger } from "../ui/popover"; import { DiffStatLabel } from "./DiffStatLabel"; import { - canToggleComposerAutoMerge, + composerAutoMergeControl, composerPullRequestCheckBuckets, composerPullRequestRow, pullRequestChecksUrl, @@ -49,10 +52,14 @@ export interface ComposerPullRequest { readonly reference: PullRequestRef; /** What the sidebar badge and the tab already resolved. */ readonly pullRequest: ThreadPullRequest; + /** The thread's project, which is the pull request's too. */ + readonly projectTitle: string | null; /** The shared read behind the Pull request tab; absent until it lands. */ readonly detail: PullRequestDetail | undefined; /** Opens the Pull request tab, the same place the sidebar badge goes. */ readonly onOpen: () => void; + /** Closes the row for this pull request in this thread. */ + readonly onDismiss: () => void; } const CHIP_TONE_CLASS: Readonly< @@ -78,6 +85,7 @@ export function ComposerPullRequestRow({ }) { const row = composerPullRequestRow({ pullRequest: pullRequest.pullRequest, + projectTitle: pullRequest.projectTitle, detail: pullRequest.detail, }); const tone = pullRequestBadgeTone(row.state, row.isDraft, row.autoMergeEnabled); @@ -132,6 +140,14 @@ export function ComposerPullRequestRow({ chip={row.chip} checksUrl={pullRequestChecksUrl(row.url)} /> + ); } @@ -212,15 +228,39 @@ function ComposerPullRequestChecksPopover({ const detail = pullRequest.detail; const buckets = composerPullRequestCheckBuckets(detail?.checks ?? []); const wrapUpOnSettled = useSettings((settings) => settings.wrapUpThreadsOnPullRequestSettled); - const autoMergeEnabled = detail?.autoMergeEnabled === true; - const canToggleAutoMerge = canToggleComposerAutoMerge(detail); - const action = useMutation( - pullRequestActionMutationOptions({ - environmentId: pullRequest.environmentId, - reference: pullRequest.reference, - queryClient, - }), + const autoMergeControl = composerAutoMergeControl(detail); + const detailQueryKey = pullRequestQueryKeys.detail( + pullRequest.environmentId, + pullRequest.reference.projectId, + pullRequest.reference.number, ); + const actionOptions = pullRequestActionMutationOptions({ + environmentId: pullRequest.environmentId, + reference: pullRequest.reference, + queryClient, + }); + const action = useMutation({ + ...actionOptions, + // The switch flips the moment it is clicked. The host takes seconds to arm + // the merge and seconds more to be re-read, and a switch that waits for + // both reads as one that did not take the click. If the host refuses, the + // detail it was read from comes back. + onMutate: (variables) => { + const previous = queryClient.getQueryData(detailQueryKey); + if (previous && variables.action.endsWith("auto-merge")) { + queryClient.setQueryData(detailQueryKey, { + ...previous, + autoMergeEnabled: variables.action === "enable-auto-merge", + }); + } + return { previous }; + }, + onError: (_error, _variables, context) => { + if (context?.previous) { + queryClient.setQueryData(detailQueryKey, context.previous); + } + }, + }); return (
@@ -270,12 +310,11 @@ function ComposerPullRequestChecksPopover({ }) )}
- {canToggleAutoMerge ? ( + {autoMergeControl.kind === "toggle" ? ( ) : null} + {autoMergeControl.kind === "queued" ? ( + // The host has taken it: there is no instruction left to switch off, + // and the queue lands it on its own. +

+ + In the merge queue +

+ ) : null}