Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/web/src/components/ChatMarkdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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"
>
<tone.Icon aria-hidden className={cn("size-3 shrink-0", tone.className)} />
<span>#{number}</span>
Expand Down
54 changes: 50 additions & 4 deletions apps/web/src/components/ChatView.browser.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
Expand Down Expand Up @@ -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<HTMLAnchorElement>("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<HTMLElement>('[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<HTMLButtonElement>('button[aria-label^="Hide pull request"]')!.click();
await waitForElement(
() =>
document.querySelector('[data-composer-pull-request-row="true"]') === null
? document.querySelector<HTMLElement>('[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();
}
Expand Down
36 changes: 28 additions & 8 deletions apps/web/src/components/chat/ChatWebLink.tsx
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -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;
Expand All @@ -36,14 +50,18 @@ export const ChatWebLink = memo(function ChatWebLink({
children,
className,
title,
onClick,
onContextMenu,
...anchorProps
}: ChatWebLinkProps) {
const pullRequestLink = useThreadPullRequestLink();
const opensPullRequestTab =
pullRequestLink !== null && isLinkToPullRequest(href, pullRequestLink.url);

const handleClick = useCallback(
(event: ReactMouseEvent<HTMLAnchorElement>) => {
if (!isPlainPrimaryClick(event)) {
onClick?.(event);
if (event.defaultPrevented || !isPlainPrimaryClick(event)) {
return;
}
if (opensPullRequestTab) {
Expand All @@ -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<HTMLAnchorElement>) => {
onContextMenu?.(event);
const api = readLocalApi();
if (!api) return;
if (!api || event.defaultPrevented) return;

event.preventDefault();
event.stopPropagation();
Expand Down Expand Up @@ -105,16 +124,17 @@ export const ChatWebLink = memo(function ChatWebLink({
copyTextWithToast(href, "Link address");
}
},
[href, opensPullRequestTab, threadRef],
[href, onContextMenu, opensPullRequestTab, threadRef],
);

return (
<a
target="_blank"
rel="noopener noreferrer"
{...anchorProps}
href={href}
className={className}
title={title}
target="_blank"
rel="noopener noreferrer"
onClick={handleClick}
onContextMenu={handleContextMenu}
>
Expand Down
75 changes: 61 additions & 14 deletions apps/web/src/components/chat/ComposerPullRequestRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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<
Expand All @@ -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);
Expand Down Expand Up @@ -132,6 +140,14 @@ export function ComposerPullRequestRow({
chip={row.chip}
checksUrl={pullRequestChecksUrl(row.url)}
/>
<button
type="button"
aria-label={`Hide pull request #${row.number} from the composer`}
className="inline-flex size-5 shrink-0 cursor-pointer items-center justify-center rounded-md text-muted-foreground/70 transition-colors hover:text-foreground focus-ring"
onClick={pullRequest.onDismiss}
>
<XIcon className="size-3.5" />
</button>
</div>
);
}
Expand Down Expand Up @@ -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<PullRequestDetail>(detailQueryKey);
if (previous && variables.action.endsWith("auto-merge")) {
queryClient.setQueryData<PullRequestDetail>(detailQueryKey, {
...previous,
autoMergeEnabled: variables.action === "enable-auto-merge",
});
}
return { previous };
},
onError: (_error, _variables, context) => {
if (context?.previous) {
queryClient.setQueryData(detailQueryKey, context.previous);
}
},
});

return (
<div className="w-full py-2 text-xs">
Expand Down Expand Up @@ -270,12 +310,11 @@ function ComposerPullRequestChecksPopover({
})
)}
<div className="my-1.5 border-border border-t" />
{canToggleAutoMerge ? (
{autoMergeControl.kind === "toggle" ? (
<label className="flex cursor-pointer items-center gap-2 px-3 py-1 transition-colors hover:bg-accent">
<Checkbox
className="size-3.5"
checked={autoMergeEnabled}
disabled={action.isPending}
checked={autoMergeControl.checked}
onCheckedChange={(checked) => {
const next: PullRequestAction = checked ? "enable-auto-merge" : "disable-auto-merge";
action.mutate({ action: next });
Expand All @@ -284,6 +323,14 @@ function ComposerPullRequestChecksPopover({
Merge when checks pass
</label>
) : 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.
<p className="flex items-center gap-2 px-3 py-1 text-muted-foreground">
<ChipDot className={CHIP_TONE_CLASS.queued.dot + " " + CHIP_TONE_CLASS.queued.chip} />
In the merge queue
</p>
) : null}
<label className="flex cursor-pointer items-center gap-2 px-3 py-1 transition-colors hover:bg-accent">
<Checkbox
className="size-3.5"
Expand Down
Loading
Loading