diff --git a/apps/web/src/components/pullRequest/PullRequestCopyableCode.tsx b/apps/web/src/components/pullRequest/PullRequestCopyableCode.tsx new file mode 100644 index 000000000000..4c9d14b53734 --- /dev/null +++ b/apps/web/src/components/pullRequest/PullRequestCopyableCode.tsx @@ -0,0 +1,66 @@ +import { useCopyToClipboard } from "~/hooks/useCopyToClipboard"; +import { cn } from "~/lib/utils"; + +import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; + +export function PullRequestCopyableCode({ + value, + target, + copyLabel, + copiedLabel, + className, + tooltipSide = "top", + onError, +}: { + readonly value: string; + readonly target: string; + readonly copyLabel: string; + readonly copiedLabel: string; + readonly className?: string; + readonly tooltipSide?: "top" | "bottom"; + readonly onError?: (error: Error) => void; +}) { + const { copyToClipboard, isCopied } = useCopyToClipboard({ + target, + timeout: 1600, + ...(onError ? { onError } : {}), + }); + return ( + + copyToClipboard(value)} + /> + } + > + + {value} + + + + + {`${isCopied ? "Copied" : copyLabel}: ${value}`} + + + ); +} diff --git a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx index c605409b1801..98d172f53eb5 100644 --- a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx +++ b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx @@ -121,6 +121,7 @@ import { Popover, PopoverPopup, PopoverTrigger } from "../ui/popover"; import { toastManager } from "../ui/toast"; import { Tooltip, TooltipPopup, TooltipProvider, TooltipTrigger } from "../ui/tooltip"; import { PullRequestDetailGhost, PullRequestTimelineGhost } from "./PullRequestGhosts"; +import { PullRequestCopyableCode } from "./PullRequestCopyableCode"; import { PullRequestActivityUnavailableState } from "./PullRequestActivityUnavailableState"; import { DiffPanelLoadingState } from "../DiffPanelShell"; import { PullRequestsUnavailableState } from "./PullRequestsUnavailableState"; @@ -140,6 +141,7 @@ import { handoffPrompt, handoffReviewComments, latestPullRequestReviewOutcomes, + loadingPullRequestCheckoutCommand, isStackedPullRequestBase, pullRequestActionMenuHasGroup, pullRequestActionNeedsHostRefresh, @@ -326,68 +328,6 @@ const openNumberContextMenu = ( }); }; -function PullRequestCopyableCode({ - value, - target, - copyLabel, - copiedLabel, - className, - tooltipSide = "top", - onError, -}: { - readonly value: string; - readonly target: string; - readonly copyLabel: string; - readonly copiedLabel: string; - readonly className?: string; - readonly tooltipSide?: "top" | "bottom"; - readonly onError?: (error: Error) => void; -}) { - const { copyToClipboard, isCopied } = useCopyToClipboard({ - target, - timeout: 1600, - ...(onError ? { onError } : {}), - }); - return ( - - copyToClipboard(value)} - /> - } - > - - {value} - - - - - {`${isCopied ? "Copied" : copyLabel}: ${value}`} - - - ); -} - /** * The stale-branch warning, said beside the branch it is about rather than as a bar of its own. * The banner this replaces held a row of chrome open across the top of every pull request that @@ -804,15 +744,22 @@ export function PullRequestDetailPanel({ repositoryUrl !== null ? new URL(`/${encodeURIComponent(detail.author.login)}`, repositoryUrl).toString() : null; - const checkoutCommand = detail + const checkoutCommand = handoffSummary ? pullRequestCheckoutCommand( - detail.provider, - detail.number, - detail.headBranch, - detail.headRepositoryNameWithOwner, - repositoryUrl, + handoffSummary.provider, + handoffSummary.number, + handoffSummary.headBranch, + detail?.headRepositoryNameWithOwner, + changeRequestRepositoryUrl(handoffSummary.url), ) - : null; + : loadingPullRequestCheckoutCommand(reference, repositoryIdentity); + const onCheckoutCommandError = useCallback((error: Error) => { + toastManager.add({ + type: "error", + title: "Could not copy checkout command", + description: error.message, + }); + }, []); const branchRefsQuery = useEnvironmentQuery( detail === null ? null @@ -1475,8 +1422,9 @@ export function PullRequestDetailPanel({ // Out of date with the base, and still cleanly mergeable — the one pairing an update button // exists for. Null everywhere else, including hosts that cannot compare at all. const freshness = detail === null ? null : resolveBaseFreshness(detail); - // A host that cannot produce a patch has no Code tab to open. The tabs themselves stay hidden - // until the detail arrives, so the loading ghost is the panel's only unfinished UI. + // A host that cannot produce a patch has no Code tab to open. While detail is loading the ghost + // uses this optimistic tab set to reserve the same chrome; a host without a patch removes Code + // when its capabilities arrive. const visibleTabs = TABS.filter( (item) => item.value !== "code" || detail === null || detail.capabilities.diff, ); @@ -1664,6 +1612,13 @@ export function PullRequestDetailPanel({ @@ -2371,7 +2326,7 @@ export function PullRequestDetailPanel({ {detail ? (
{titleDraft === null ? ( -
+
)} -
+
- toastManager.add({ - type: "error", - title: "Could not copy checkout command", - description: error.message, - }) - } + onError={onCheckoutCommandError} /> ) : null}
-
+
- + {detail.changedFiles.toLocaleString()}{" "} {detail.changedFiles === 1 ? "file" : "files"} diff --git a/apps/web/src/components/pullRequest/PullRequestGhosts.tsx b/apps/web/src/components/pullRequest/PullRequestGhosts.tsx index c82129df13b1..90f88da93668 100644 --- a/apps/web/src/components/pullRequest/PullRequestGhosts.tsx +++ b/apps/web/src/components/pullRequest/PullRequestGhosts.tsx @@ -8,16 +8,31 @@ * number of bars costs one opacity animation. */ import type { PullRequestListEntry, PullRequestSummary } from "@t3tools/contracts"; -import { ArrowLeftIcon } from "lucide-react"; +import { + ArrowLeftIcon, + ChevronRightIcon, + EllipsisIcon, + ExternalLinkIcon, + FileDiffIcon, + PanelRightIcon, + TagIcon, + UserPlusIcon, + UsersIcon, +} from "lucide-react"; import type { ReactNode } from "react"; +import { readLocalApi } from "~/localApi"; import { cn } from "~/lib/utils"; import { formatRelativeTimeLabel } from "~/timestampFormat"; +import { Button, InlineButton } from "../ui/button"; +import { Toggle, ToggleGroup } from "../ui/toggle-group"; +import { PullRequestCopyableCode } from "./PullRequestCopyableCode"; import { pullRequestLabelColor } from "./pullRequestList.logic"; import { PullRequestActorLabel, PullRequestDiffStat, + PullRequestMetaLine, pullRequestChecksStatePresentation, resolvePullRequestState, } from "./pullRequestPresentation"; @@ -29,6 +44,11 @@ function GhostBar({ className }: { className?: string | undefined }) { /** Widths cycle rather than randomize, so the ghost renders the same on every pass. */ const TITLE_WIDTHS = ["w-3/5", "w-2/5", "w-1/2", "w-2/3", "w-2/5", "w-3/5", "w-1/2"]; const META_WIDTHS = ["w-2/5", "w-1/3", "w-2/5", "w-1/4", "w-1/3", "w-2/5", "w-1/3"]; +const DEFAULT_DETAIL_TABS = [ + { value: "summary", label: "Summary" }, + { value: "timeline", label: "Timeline" }, + { value: "code", label: "Code" }, +] as const; /** Rows in the list's own grid — glyph, title over meta, time over diffstat. */ export function PullRequestListGhost({ @@ -77,10 +97,25 @@ export function PullRequestDetailGhost({ seed: entry, summary, actions, + checkoutCommand, + tabs = DEFAULT_DETAIL_TABS, + activeTab, + number, + onBack, + onClose, + onCheckoutError, }: { seed?: PullRequestListEntry | null; summary?: PullRequestSummary | null; actions?: ReactNode; + checkoutCommand?: string | null; + tabs?: ReadonlyArray<{ value: string; label: string }>; + /** The panel's current tab, so the highlight does not jump when the detail arrives. */ + activeTab?: string; + number?: number; + onBack?: (() => void) | undefined; + onClose?: (() => void) | undefined; + onCheckoutError?: ((error: Error) => void) | undefined; }) { const seed = summary ? { @@ -100,6 +135,10 @@ export function PullRequestDetailGhost({ seed?.checksState === "failing" || seed?.checksState === "pending" ? pullRequestChecksStatePresentation(seed.checksState) : null; + const checkout = checkoutCommand ?? null; + const changedFiles = summary?.changedFiles ?? null; + const selectedTab = + tabs.find((item) => item.value === activeTab)?.value ?? tabs[0]?.value ?? "summary"; return (
-
-
-
- {seed && statePresentation ? ( +
+
+
+ {onBack ? ( + + ) : null} + {seed ? ( <> - - {seed.repository} - - {seed.repository} + void readLocalApi()?.shell.openExternal(seed.url)} + className={cn( + "font-medium underline-offset-2 hover:underline", + statePresentation?.toneClassName, + )} + aria-label={`Open pull request #${seed.number} on host`} > #{seed.number} - + + ) : ( <> - + #{number ?? "…"} )}
-
- {actions ?? } - -
+
+
+ {actions ?? } + + {onClose ? ( + + ) : null}
-
- {seed ? ( -

{seed.title}

- ) : ( - - )} -
- {seed ? ( - <> - - - updated {formatRelativeTimeLabel(seed.updatedAt)} - - - ) : ( - <> - - - - )} -
-
- {seed ? ( - - {seed.baseBranch} - - {seed.headBranch} - - ) : ( - <> - - - - - )} -
- +
+
+
{seed ? ( - +
+

{seed.title}

+
) : ( - +
+ +
)} +
+ {seed ? ( + + + updated {formatRelativeTimeLabel(seed.updatedAt)} + + ) : ( + + + + + + + + )} + {checkout ? ( + + ) : null} +
+ +
+ + {seed ? ( + + {seed.baseBranch} + + ) : ( + + + + )} + + {seed ? ( + + ) : ( + + )} + + + + + {changedFiles === null ? ( + + ) : ( + `${changedFiles.toLocaleString()} ${changedFiles === 1 ? "file" : "files"}` + )} + + {seed ? ( + + ) : ( + + )} + +
-
-
- - - -
+
+
-
-
-
- - -
-
- - - -
-
-
-
- - -
-
- {seed?.labels ? ( - seed.labels.slice(0, 3).map((label) => { - const color = pullRequestLabelColor(label.color); - return ( - - - {label.name} - - ); - }) - ) : ( - <> - - - - )} +
+
+
+ + + Reviewers + + + + +
-
-
-
- - +
+ + + Labels + + + {entry?.labels ? ( + entry.labels.length > 0 ? ( + entry.labels.map((label) => { + const color = pullRequestLabelColor(label.color); + return ( + + + {label.name} + + ); + }) + ) : ( + None + ) + ) : ( + <> + + + + )} + +
-
-
-
- - +
+
+
+ Description + +
-
- - - - +
+ +
+ + + + +
diff --git a/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx b/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx index b2096c87ded9..4470e6c307c0 100644 --- a/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx +++ b/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx @@ -275,7 +275,7 @@ function MetaRow({ children: ReactNode; }) { return ( -
+
{icon} {label} diff --git a/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts b/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts index 5e75851f08ad..ac0c9d01d17e 100644 --- a/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts +++ b/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts @@ -7,7 +7,9 @@ import { type PullRequestComment, type PullRequestDetail, type PullRequestDetailView, + type PullRequestRef, type PullRequestReviewThread, + type RepositoryIdentity, type ThreadPullRequestLink, } from "@t3tools/contracts"; import { describe, expect, it } from "vite-plus/test"; @@ -27,6 +29,7 @@ import { stripPullRequestHandoffReferences, isPullRequestVerdictStale, isStackedPullRequestBase, + loadingPullRequestCheckoutCommand, pullRequestPanelContext, latestPullRequestReviewOutcomes, newestPullRequestCommitAt, @@ -94,6 +97,49 @@ describe("pull request checkout commands", () => { "git fetch 'https://forgejo.local/maria/repo'\\''$(echo nope)' refs/pull/42/head && git checkout -B pulls/42 FETCH_HEAD", ); }); + + const reference = (host?: string): PullRequestRef => ({ + projectId: ProjectId.make("project-1"), + ...(host === undefined ? {} : { host }), + repository: "acme/web", + number: 42, + }); + const identity = (provider: string, canonicalKey: string): RepositoryIdentity => ({ + canonicalKey, + locator: { + source: "git-remote", + remoteName: "origin", + remoteUrl: "git@github.com:acme/web.git", + }, + provider, + }); + + it("uses a public host when no repository identity is available", () => { + expect(loadingPullRequestCheckoutCommand(reference("github.com"), undefined)).toBe( + "gh pr checkout 42", + ); + expect(loadingPullRequestCheckoutCommand(reference("gitlab.com"), null)).toBe( + "glab mr checkout 42", + ); + }); + + it("uses a matching enterprise identity and rejects an explicit host mismatch", () => { + const enterprise = identity("github", "github.example.test/acme/web"); + expect(loadingPullRequestCheckoutCommand(reference("github.example.test"), enterprise)).toBe( + "gh pr checkout 42", + ); + expect(loadingPullRequestCheckoutCommand(reference("github.com"), enterprise)).toBeNull(); + }); + + it("does not infer a number-only command without a trusted provider", () => { + expect(loadingPullRequestCheckoutCommand(reference(), undefined)).toBeNull(); + expect( + loadingPullRequestCheckoutCommand( + reference("github.com"), + identity("gitlab", "gitlab.com/acme/web"), + ), + ).toBeNull(); + }); }); const TIMELINE_SOURCE: Pick< diff --git a/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts b/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts index 733e314b770f..2ec4863072dc 100644 --- a/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts +++ b/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts @@ -140,6 +140,22 @@ export function pullRequestCheckoutCommand( } } +/** Build a checkout command from identity metadata while the detail request is still pending. */ +export function loadingPullRequestCheckoutCommand( + reference: PullRequestRef, + identity: RepositoryIdentity | null | undefined, +): string | null { + const host = reference.host?.trim().toLowerCase(); + const provider = + identity?.provider ?? + (host === "github.com" ? "github" : host === "gitlab.com" ? "gitlab" : null); + if (provider !== "github" && provider !== "gitlab" && provider !== "azure-devops") return null; + if (identity?.provider !== undefined && host && pullRequestHostOf(identity, provider) !== host) { + return null; + } + return pullRequestCheckoutCommand(provider, reference.number, ""); +} + /** Activity changes only when the same host resource reports a newer revision. */ export function shouldRefreshPullRequestActivity( previous: { readonly key: string; readonly updatedAt: string } | null,