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
42 changes: 40 additions & 2 deletions apps/web/src/components/ChatMarkdown.browser.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -572,9 +572,11 @@ describe("ChatMarkdown", () => {
window.addEventListener("click", observeClick);

try {
// Both links are chips now, so their text is the number the chip prints
// followed by the words the author wrote.
const click = (name: string) => {
const anchor = Array.from(document.querySelectorAll("a")).find(
(candidate) => candidate.textContent === name,
const anchor = Array.from(document.querySelectorAll("a")).find((candidate) =>
candidate.textContent?.includes(name),
);
if (!anchor) {
throw new Error(`No link named ${name}`);
Expand All @@ -595,6 +597,42 @@ describe("ChatMarkdown", () => {
}
});

it("renders a pull request address as a numbered chip that opens its tab", async () => {
const { url: pullRequestUrl, open } = THREAD_PULL_REQUEST_LINK;
open.mockClear();
const screen = await render(
<ThreadPullRequestLinkContext.Provider value={THREAD_PULL_REQUEST_LINK}>
<ChatMarkdown
text={`Opened ${pullRequestUrl}, see [PR 223](${pullRequestUrl}) for [the migration fix](${pullRequestUrl}).`}
cwd="/repo/project"
environmentId={CHAT_MARKDOWN_ENVIRONMENT_ID}
threadId={CHAT_MARKDOWN_THREAD_ID}
/>
</ThreadPullRequestLinkContext.Provider>,
);

try {
const chips = Array.from(document.querySelectorAll("a")).filter((anchor) =>
anchor.getAttribute("href")?.startsWith(pullRequestUrl),
);
expect(chips).toHaveLength(3);
// The bare address and text that only restates the number both give way
// to the chip; anything the author actually wrote is kept beside it.
expect(chips.map((chip) => chip.textContent)).toEqual([
"#223",
"#223",
"#223the migration fix",
]);
// A chip, not a URL: the state glyph plus the number.
expect(chips[0]?.querySelector("svg")).toBeTruthy();

chips[0]?.dispatchEvent(new MouseEvent("click", { bubbles: true, cancelable: true }));
expect(open).toHaveBeenCalledTimes(1);
} finally {
await screen.unmount();
}
});

it("keeps normal web links unchanged", async () => {
const screen = await render(
<ChatMarkdown text="[OpenAI](https://openai.com/docs)" cwd="/repo/project" />,
Expand Down
86 changes: 86 additions & 0 deletions apps/web/src/components/ChatMarkdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,12 @@ import {
rewriteMarkdownFileUriHref,
} from "../markdown-links";
import { readLocalApi } from "../localApi";
import { GitPullRequestIcon } from "lucide-react";

import { ChatWebLink } from "./chat/ChatWebLink";
import { PullRequestHoverCard, usePullRequestChip } from "./pull-requests/PullRequestHoverCard";
import { pullRequestBadgeTone } from "./pull-requests/pullRequests.logic";
import { parsePullRequestUrl } from "../pullRequestReference";
import { copyTextWithToast } from "./chat/copyTextWithToast";
import { isBrowserPanelHref } from "./browser/openInBrowserPanel";
import {
Expand Down Expand Up @@ -564,6 +569,72 @@ const renderBareImagePath: NonNullable<InlineMarkdownContext["renderBareImagePat
<MarkdownBareImagePath key={input.key} rawPath={input.path} />
);

/** The visible words of a link, flattened out of whatever markdown made them. */
function markdownChildrenText(children: ReactNode): string {
let text = "";
Children.forEach(children, (child) => {
if (typeof child === "string" || typeof child === "number") {
text += String(child);
return;
}
if (isValidElement<{ children?: ReactNode }>(child)) {
text += markdownChildrenText(child.props.children);
}
});
return text;
}

/** A link whose words only repeat the number the chip already prints. */
const REDUNDANT_PULL_REQUEST_LINK_TEXT = /^(?:pr\s*)?#?\d+$/i;

/**
* A pull request address in a transcript, as a chip rather than a URL: the
* state glyph and `#number`, which is how the sidebar, the composer's row and
* the pull requests page all name one. The words the author wrote are kept
* after the number unless they only repeat it.
*
* The click behaviour is unchanged -- {@link ChatWebLink} still decides whether
* this opens the thread's own Pull request tab or the browser panel.
*/
function MarkdownPullRequestChip({
href,
repository,
number,
label,
threadRef,
title,
}: {
readonly href: string;
readonly repository: string;
readonly number: number;
/** Empty when the link's own text said nothing the number does not. */
readonly label: string;
readonly threadRef: ScopedThreadRef | null;
readonly title?: string | undefined;
}) {
const chip = usePullRequestChip(repository, number);
const tone = chip.state
? pullRequestBadgeTone(chip.state.state, chip.state.isDraft)
: // Nothing here has listed this repository, so the glyph says "a pull
// request" without claiming to know how it is going.
{ Icon: GitPullRequestIcon, className: "text-muted-foreground", label: "Pull request" };

return (
<PullRequestHoverCard payload={chip.payload}>
<ChatWebLink
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"
>
<tone.Icon aria-hidden className={cn("size-3 shrink-0", tone.className)} />
<span>#{number}</span>
{label ? <span className="font-sans">{label}</span> : null}
</ChatWebLink>
</PullRequestHoverCard>
);
}

function MarkdownAnchor({ node: _node, href, children, ...props }: MarkdownRendererProps<"a">) {
const {
cwd,
Expand All @@ -580,6 +651,21 @@ function MarkdownAnchor({ node: _node, href, children, ...props }: MarkdownRende
resolveMarkdownFileLinkMeta(normalizedHref, cwd))
: null;
if (!fileLinkMeta) {
const pullRequest = href ? parsePullRequestUrl(href) : null;
if (href && pullRequest) {
const text = markdownChildrenText(children).trim();
const label = text === href.trim() || REDUNDANT_PULL_REQUEST_LINK_TEXT.test(text) ? "" : text;
return (
<MarkdownPullRequestChip
href={href}
repository={pullRequest.repository}
number={pullRequest.number}
label={label}
threadRef={threadRef}
title={props.title}
/>
);
}
if (href && isBrowserPanelHref(href)) {
return (
<ChatWebLink
Expand Down
200 changes: 200 additions & 0 deletions apps/web/src/components/ChatView.browser.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,149 @@ function buildFixture(snapshot: OrchestrationReadModel): TestFixture {
};
}

const PULL_REQUEST_NUMBER = 234;
const PULL_REQUEST_REPOSITORY = "Threadlines/threadlines";
// The fixture thread is checked out on `main`, which is what links it to the
// listing row the way the sidebar badge and the Pull request tab do.
const PULL_REQUEST_HEAD_BRANCH = "main";
const PULL_REQUEST_URL = `https://github.com/${PULL_REQUEST_REPOSITORY}/pull/${PULL_REQUEST_NUMBER}`;

/**
* A workspace whose project sits on GitHub, whose server can list pull
* requests, and whose version skew raises a notice, so the dock has both kinds
* of row to hold at once.
*/
function withPullRequestFixture(nextFixture: TestFixture): void {
const capabilities = { repositoryIdentity: true, pullRequests: true } as const;
nextFixture.serverConfig = {
...nextFixture.serverConfig,
environment: {
...nextFixture.serverConfig.environment,
serverVersion: "9.9.9",
capabilities,
},
};
nextFixture.welcome = {
...nextFixture.welcome,
environment: { ...nextFixture.welcome.environment, capabilities },
};
nextFixture.snapshot = {
...nextFixture.snapshot,
projects: nextFixture.snapshot.projects.map((project) => ({
...project,
repositoryIdentity: {
canonicalKey: `github.com/${PULL_REQUEST_REPOSITORY}`.toLowerCase(),
locator: {
source: "git-remote" as const,
remoteName: "origin",
remoteUrl: `https://github.com/${PULL_REQUEST_REPOSITORY}.git`,
},
displayName: PULL_REQUEST_REPOSITORY,
provider: "github",
owner: "Threadlines",
name: "threadlines",
},
})),
};
}

/** The listing row and the detail behind the composer's docked pull request. */
function resolvePullRequestRpc(body: NormalizedWsRpcRequestBody): unknown | undefined {
if (body._tag === WS_METHODS.pullRequestsList) {
const state = (body as { state?: string }).state;
return {
viewer: "badcuban",
errors: [],
entries:
state === "open"
? [
{
provider: "github",
projectId: PROJECT_ID,
projectTitle: "threadlines",
repository: PULL_REQUEST_REPOSITORY,
number: PULL_REQUEST_NUMBER,
title: "fix(server): migration 050 no longer stalls startup",
url: PULL_REQUEST_URL,
author: { login: "badcuban", isBot: false, avatarUrl: null },
headBranch: PULL_REQUEST_HEAD_BRANCH,
baseBranch: "main",
state: "open",
isDraft: false,
additions: 26,
deletions: 25,
createdAt: NOW_ISO,
updatedAt: NOW_ISO,
viewerIsAuthor: true,
viewerReviewRequested: false,
labels: [],
origin: "workspace",
},
]
: [],
};
}
if (body._tag === WS_METHODS.pullRequestsDetail) {
return {
provider: "github",
projectId: PROJECT_ID,
projectTitle: "threadlines",
workspaceRoot: "/repo/project",
repository: PULL_REQUEST_REPOSITORY,
number: PULL_REQUEST_NUMBER,
title: "fix(server): migration 050 no longer stalls startup",
body: "",
url: PULL_REQUEST_URL,
author: { login: "badcuban", isBot: false, avatarUrl: null },
state: "open",
isDraft: false,
mergeability: "mergeable",
additions: 26,
deletions: 25,
changedFiles: 2,
headBranch: PULL_REQUEST_HEAD_BRANCH,
baseBranch: "main",
createdAt: NOW_ISO,
updatedAt: NOW_ISO,
mergedAt: null,
closedAt: null,
viewerIsAuthor: true,
reviewers: [],
labels: [],
checks: [
{ name: "build", status: "pending", description: null, url: null },
{ name: "test", status: "pending", description: null, url: null },
{ name: "lint", status: "success", description: null, url: null },
],
checksState: "pending",
viewer: { canWrite: true, canReview: false, canManage: true },
mergeMethods: ["squash"],
capabilities: {
diff: true,
comment: true,
actions: ["merge", "close", "enable-auto-merge", "disable-auto-merge"],
mergeMethods: ["squash"],
updateMethods: ["merge"],
reactions: true,
review: {
inlineComment: true,
reply: true,
resolve: true,
verdicts: ["approve", "comment"],
},
reviewers: { request: true, listCandidates: true },
edit: { pullRequest: true, comment: true },
},
baseComparison: "up-to-date",
behindBy: 0,
autoMergeEnabled: false,
isStacked: false,
defaultBranch: "main",
};
}
return undefined;
}

function addThreadToSnapshot(
snapshot: OrchestrationReadModel,
threadId: ThreadId,
Expand Down Expand Up @@ -2775,6 +2918,63 @@ describe("ChatView timeline estimator parity (full app)", () => {
}
});

it("docks the thread's pull request above the notices in one frame", async () => {
const mounted = await mountChatView({
viewport: DEFAULT_VIEWPORT,
snapshot: createSnapshotForTargetUser({
targetMessageId: "msg-user-pull-request-dock" as MessageId,
targetText: "pull request dock",
}),
configureFixture: withPullRequestFixture,
resolveRpc: resolvePullRequestRpc,
});

try {
// The branch, the project and the size only exist on the detail, so
// waiting for the branch is waiting for the shared read to reach the row.
const row = await waitForElement(() => {
const candidate = document.querySelector<HTMLElement>(
'[data-composer-pull-request-row="true"]',
);
return candidate?.textContent?.includes(PULL_REQUEST_HEAD_BRANCH) ? candidate : null;
}, "Unable to find the composer pull request row with its branch.");
expect(row.textContent).toContain(`#${PULL_REQUEST_NUMBER}`);
expect(row.textContent).toContain("+26");
// Two checks are still running, which is what the chip's word covers and
// its dot colours.
expect(row.textContent).toContain("CI");

// The state glyph is the open one the sidebar badge and the pull
// requests page use, in the same tone.
const stateIcon = row.querySelector("svg");
expect(stateIcon?.getAttribute("class")).toContain("text-emerald-600");

// One frame, not two: the notice the version skew raises sits inside the
// same dock, under the pull request row, and the dock's bottom edge is
// the composer's top edge.
const dock = document.querySelector<HTMLElement>('[data-composer-notice-dock="true"]');
expect(dock).toBeTruthy();
expect(dock!.contains(row)).toBe(true);
const notice = dock!.querySelector("[data-composer-notice-severity]");
expect(notice).toBeTruthy();
expect(row.getBoundingClientRect().bottom).toBeLessThanOrEqual(
notice!.getBoundingClientRect().top + 1,
);

const composerSurface = document.querySelector<HTMLElement>(
"[data-chat-composer-mobile-collapsed]",
);
expect(composerSurface).toBeTruthy();
expect(
Math.abs(
dock!.getBoundingClientRect().bottom - composerSurface!.getBoundingClientRect().top,
),
).toBeLessThan(2);
} finally {
await mounted.cleanup();
}
});

it("re-expands the bootstrap project using its logical key", async () => {
useUiStateStore.setState({
projectExpandedById: {
Expand Down
Loading
Loading