diff --git a/apps/web/src/components/CommandPalette.tsx b/apps/web/src/components/CommandPalette.tsx index 8af4419fca31..066b169d8371 100644 --- a/apps/web/src/components/CommandPalette.tsx +++ b/apps/web/src/components/CommandPalette.tsx @@ -46,7 +46,6 @@ import { FileSearchIcon, FolderIcon, FolderPlusIcon, - GitPullRequestArrowIcon, LinkIcon, MessageSquareIcon, PaletteIcon, @@ -181,6 +180,7 @@ import { buildSidebarProjectSnapshots, } from "../sidebarProjectGrouping"; import type { Project } from "../types"; +import { PullRequestGlyph } from "~/components/pullRequest/pullRequestIcons"; const EMPTY_BROWSE_ENTRIES: FilesystemBrowseResult["entries"] = []; @@ -1706,7 +1706,7 @@ function OpenCommandPaletteDialog(props: { value: "action:link-pull-request", searchTerms: ["link", "pull request", "pr", "attach", "stack"], title: "Link pull request to thread", - icon: , + icon: , run: async () => { openLinkPullRequestDialog(threadRef); }, @@ -1718,7 +1718,7 @@ function OpenCommandPaletteDialog(props: { searchTerms: ["pull requests", "linked", "stack", "prs"], title: "Show linked pull requests", disabled: visibleThreadPullRequests(activeThread.pullRequests).length === 0, - icon: , + icon: , run: async () => { useRightPanelStore.getState().open(threadRef, "pull-requests"); }, diff --git a/apps/web/src/components/LegacySidebar.tsx b/apps/web/src/components/LegacySidebar.tsx index 81fd6047f13e..82f8102d38e2 100644 --- a/apps/web/src/components/LegacySidebar.tsx +++ b/apps/web/src/components/LegacySidebar.tsx @@ -1,5 +1,4 @@ import { useSupportsMultiplePullRequests } from "~/hooks/useSupportsMultiplePullRequests"; -import { GitPullRequestIcon } from "lucide-react"; import { resolveThreadCurrentPullRequestLink } from "@t3tools/shared/threadPullRequests"; import { Spinner } from "~/components/ui/spinner"; import { @@ -214,6 +213,7 @@ import { type SidebarProjectGroupMember, type SidebarProjectSnapshot, } from "../sidebarProjectGrouping"; +import { PullRequestGlyph } from "~/components/pullRequest/pullRequestIcons"; const SIDEBAR_SORT_LABELS: Record = { updated_at: "Last user message", created_at: "Created at", @@ -757,7 +757,7 @@ const SidebarThreadRow = memo(function SidebarThreadRow(props: SidebarThreadRowP className="text-muted-foreground" aria-label={`PR #${currentLinkedPr.number}, status pending`} > - + ) : null} {threadStatus && } diff --git a/apps/web/src/components/RightPanelTabs.tsx b/apps/web/src/components/RightPanelTabs.tsx index e0fb70b8080d..f234efff093e 100644 --- a/apps/web/src/components/RightPanelTabs.tsx +++ b/apps/web/src/components/RightPanelTabs.tsx @@ -21,8 +21,6 @@ import { ChevronRight, FileDiff, Files, - GitPullRequest, - GitPullRequestArrow, Globe2, Plus, TerminalSquare, @@ -73,6 +71,7 @@ import { FaviconImage } from "./preview/PreviewFaviconIcon"; import { previewBridge } from "./preview/previewBridge"; import { PierreEntryIcon } from "./chat/PierreEntryIcon"; import { resolvePullRequestState } from "./pullRequest/pullRequestPresentation"; +import { PullRequestGlyph } from "~/components/pullRequest/pullRequestIcons"; interface RightPanelTabsProps { mode: PreviewPanelMode; @@ -373,7 +372,7 @@ function RightPanelEmptyState(props: { }, { label: "Pull request", - icon: GitPullRequest, + icon: PullRequestGlyph.pullRequest, shortcut: "P", available: props.pullRequestAvailable, disabledReason: SURFACE_UNAVAILABLE_HINTS.pullRequest, @@ -382,7 +381,7 @@ function RightPanelEmptyState(props: { }, { label: "Linked pull requests", - icon: GitPullRequestArrow, + icon: PullRequestGlyph.link, shortcut: "L", available: props.pullRequestsAvailable, disabledReason: SURFACE_UNAVAILABLE_HINTS.pullRequests, @@ -712,7 +711,7 @@ function SurfaceIcon({ /> ); case "pull-requests": - return ; + return ; case "agents": return ; case "device": @@ -801,8 +800,9 @@ function PullRequestSurfaceIcon({ }, }), ).data; - // Only state and draft reach the tab. A list seed cannot know mergeability, so feeding the - // full detail would flip an open tab to the conflict glyph the moment its read lands. + // The compact tab intentionally shows lifecycle and draft state only. Conflict warnings have + // their own presentation on surfaces that have mergeability, while this tab stays stable as + // detail data arrives. const status = linkedSnapshot !== null ? linkedSnapshot @@ -810,7 +810,7 @@ function PullRequestSurfaceIcon({ ? (seed ?? null) : { state: detail.state, isDraft: detail.isDraft }; if (status === null) { - return ; + return ; } const presentation = resolvePullRequestState({ state: status.state, isDraft: status.isDraft }); return ; @@ -895,7 +895,7 @@ export function RightPanelTabs(props: RightPanelTabsProps) { }, { label: "Pull request", - icon: GitPullRequest, + icon: PullRequestGlyph.pullRequest, shortcut: "P", available: props.pullRequestAvailable, disabledReason: SURFACE_DISABLED_REASONS.pullRequest, @@ -903,7 +903,7 @@ export function RightPanelTabs(props: RightPanelTabsProps) { }, { label: "Linked pull requests", - icon: GitPullRequestArrow, + icon: PullRequestGlyph.link, shortcut: "L", available: props.pullRequestsAvailable, disabledReason: SURFACE_DISABLED_REASONS.pullRequests, diff --git a/apps/web/src/components/ThreadStatusIndicators.test.ts b/apps/web/src/components/ThreadStatusIndicators.test.ts index 0afa45d27d0a..60b17b19aa6b 100644 --- a/apps/web/src/components/ThreadStatusIndicators.test.ts +++ b/apps/web/src/components/ThreadStatusIndicators.test.ts @@ -1,25 +1,20 @@ import { ProjectId, type PullRequestSummary, type VcsStatusResult } from "@t3tools/contracts"; import { describe, expect, it } from "@effect/vitest"; -import { - GitMergeIcon, - GitPullRequestClosedIcon, - GitPullRequestDraftIcon, - GitPullRequestIcon, -} from "lucide-react"; import { ChangeRequestStatusIcon, prStatusIndicator, - settledPrHoverColorClass, + resolveThreadPullRequestBadgePresentation, } from "./ThreadStatusIndicators"; import { newestPullRequestSummary } from "../state/pullRequests"; +import { PullRequestGlyph } from "~/components/pullRequest/pullRequestIcons"; describe("ChangeRequestStatusIcon", () => { it.each([ - ["open", "open", false, GitPullRequestIcon], - ["draft", "open", true, GitPullRequestDraftIcon], - ["closed", "closed", false, GitPullRequestClosedIcon], - ["merged", "merged", false, GitMergeIcon], + ["open", "open", false, PullRequestGlyph.pullRequest], + ["draft", "open", true, PullRequestGlyph.draft], + ["closed", "closed", false, PullRequestGlyph.closed], + ["merged", "merged", false, PullRequestGlyph.merged], ] as const)("uses the %s pull request glyph", (_label, state, isDraft, expectedIcon) => { expect(ChangeRequestStatusIcon({ state, isDraft }).type).toBe(expectedIcon); }); @@ -119,18 +114,131 @@ describe("prStatusIndicator", () => { }); }); -describe("settledPrHoverColorClass", () => { - it.each([ - ["open", "text-emerald-600"], - ["merged", "text-violet-600"], - ["closed", "text-red-600"], - ] as const)("restores the %s pull request color on row hover", (state, colorClass) => { - expect(settledPrHoverColorClass(state)).toContain(`group-hover/sidebar-row:${colorClass}`); +describe("resolveThreadPullRequestBadgePresentation", () => { + const url = "https://github.com/pingdotgg/t3code/pull/42"; + + it("returns the pending pull-request badge when no snapshot is available", () => { + expect( + resolveThreadPullRequestBadgePresentation({ + badge: null, + number: 42, + url, + status: null, + }), + ).toEqual({ + Icon: PullRequestGlyph.pullRequest, + toneClassName: "text-muted-foreground", + label: "PR #42, status pending", + text: 42, + }); }); - it("keeps draft pull requests gray on row hover", () => { - expect(settledPrHoverColorClass("open", true)).toContain( - "group-hover/sidebar-row:text-zinc-500", - ); + it.each([ + [ + "open", + { state: "open", isDraft: false }, + PullRequestGlyph.pullRequest, + "text-emerald-600 dark:text-emerald-300/90", + "PR #42 - Open: PR branch", + ], + [ + "draft", + { state: "open", isDraft: true }, + PullRequestGlyph.draft, + "text-zinc-500 dark:text-zinc-400/80", + "PR #42 - Draft: PR branch", + ], + [ + "closed", + { state: "closed", isDraft: false }, + PullRequestGlyph.closed, + "text-red-600 dark:text-red-300/90", + "PR #42 - Closed: PR branch", + ], + [ + "merged", + { state: "merged", isDraft: false }, + PullRequestGlyph.merged, + "text-violet-600 dark:text-violet-300/90", + "PR #42 - Merged: PR branch", + ], + ] as const)( + "keeps the %s state for one linked pull request", + (_state, prOverrides, expectedIcon, expectedToneClassName, expectedLabel) => { + const fixture = status().pr; + if (!fixture) throw new Error("Expected pull request fixture"); + const prStatus = prStatusIndicator({ ...fixture, ...prOverrides }, undefined); + if (!prStatus) throw new Error("Expected pull request status"); + + expect( + resolveThreadPullRequestBadgePresentation({ + badge: { kind: "pull-request", others: 0, state: "open" }, + number: fixture.number, + url: fixture.url, + status: prStatus, + }), + ).toEqual({ + Icon: expectedIcon, + toneClassName: expectedToneClassName, + label: expectedLabel, + text: fixture.number, + }); + }, + ); + + it.each([ + ["open", "text-emerald-600 dark:text-emerald-300/90"], + ["draft", "text-zinc-500 dark:text-zinc-400/80"], + ["merged", "text-violet-600 dark:text-violet-300/90"], + ] as const)( + "uses a layers badge with the %s stack tone without a link identity", + (state, expectedToneClassName) => { + expect( + resolveThreadPullRequestBadgePresentation({ + badge: { kind: "stack", layers: 3, state }, + status: null, + }), + ).toEqual({ + Icon: PullRequestGlyph.stack, + toneClassName: expectedToneClassName, + label: `Stack of 3 pull requests, ${state}`, + text: 3, + }); + }, + ); + + it.each([ + ["open", PullRequestGlyph.pullRequest, "text-emerald-600 dark:text-emerald-300/90"], + ["draft", PullRequestGlyph.draft, "text-zinc-500 dark:text-zinc-400/80"], + ["merged", PullRequestGlyph.merged, "text-violet-600 dark:text-violet-300/90"], + ] as const)( + "draws the count of unrelated linked pull requests with their %s aggregate state", + (state, expectedIcon, expectedToneClassName) => { + const fixture = status().pr; + if (!fixture) throw new Error("Expected pull request fixture"); + const closedStatus = prStatusIndicator( + { ...fixture, state: "closed", isDraft: false }, + undefined, + ); + if (!closedStatus) throw new Error("Expected pull request status"); + + expect( + resolveThreadPullRequestBadgePresentation({ + badge: { kind: "pull-request", others: 2, state }, + number: fixture.number, + url: fixture.url, + status: closedStatus, + }), + ).toEqual({ + Icon: expectedIcon, + toneClassName: expectedToneClassName, + label: `PR #42 - Closed: PR branch, and 2 more linked; overall ${state}`, + text: "+3", + }); + }, + ); + + it("omits the control when neither a stack nor a linked identity can be shown", () => { + expect(resolveThreadPullRequestBadgePresentation({ badge: null, status: null })).toBeNull(); }); }); diff --git a/apps/web/src/components/ThreadStatusIndicators.tsx b/apps/web/src/components/ThreadStatusIndicators.tsx index 7201779f13f1..016a2c2e431d 100644 --- a/apps/web/src/components/ThreadStatusIndicators.tsx +++ b/apps/web/src/components/ThreadStatusIndicators.tsx @@ -14,7 +14,7 @@ import { visibleThreadPullRequests, type ThreadPullRequestBadge, } from "@t3tools/shared/threadPullRequests"; -import { FolderGit2Icon, GitPullRequestArrowIcon, LayersIcon, TerminalIcon } from "lucide-react"; +import { FolderGit2Icon, TerminalIcon } from "lucide-react"; import { useMemo, type MouseEvent } from "react"; import { buttonVariants, InlineButton } from "./ui/button"; import { cn } from "../lib/utils"; @@ -31,11 +31,17 @@ import type { SidebarThreadSummary } from "../types"; import { formatWorktreePathForDisplay } from "../worktreeCleanup"; import { Tooltip, TooltipPopup, TooltipTrigger } from "./ui/tooltip"; import { pullRequestListLines } from "./pullRequest/pullRequestListLines"; +import { + PULL_REQUEST_STATE_PRESENTATION, + PullRequestGlyph, + type PullRequestGlyphIcon, +} from "./pullRequest/pullRequestIcons"; import { resolvePullRequestState } from "./pullRequest/pullRequestPresentation"; export interface PrStatusIndicator { label: string; colorClass: string; + Icon: PullRequestGlyphIcon; tooltip: string; tooltipLead: string; tooltipTitle: string; @@ -125,16 +131,55 @@ export { type ThreadPullRequestBadge, } from "@t3tools/shared/threadPullRequests"; -/** The glyph a row's badge wears: the layers icon for a stack, the pull-request one otherwise. */ -function ThreadPullRequestBadgeIcon({ - icon, - className, +export interface ThreadPullRequestBadgePresentation { + readonly Icon: PullRequestGlyphIcon; + readonly toneClassName: string; + readonly label: string; + readonly text: string | number; +} + +/** Resolve the complete badge appearance before rendering it in the sidebar or composer. */ +export function resolveThreadPullRequestBadgePresentation({ + badge, + number, + url, + status, }: { - icon: "stack" | "pull-request"; - className?: string | undefined; -}) { - const Icon = icon === "stack" ? LayersIcon : GitPullRequestArrowIcon; - return ; + readonly badge: ThreadPullRequestBadge | null; + readonly number?: number | undefined; + readonly url?: string | undefined; + readonly status: PrStatusIndicator | null; +}): ThreadPullRequestBadgePresentation | null { + // The badge already folds every visible link into one state, draft included, so both the + // stack and the linked count index the shared table directly rather than the single-PR resolver. + if (badge?.kind === "stack") { + const aggregate = PULL_REQUEST_STATE_PRESENTATION[badge.state]; + return { + Icon: PullRequestGlyph.stack, + toneClassName: aggregate.toneClassName, + label: `Stack of ${badge.layers} pull requests, ${aggregate.label.toLowerCase()}`, + text: badge.layers, + }; + } + if (number === undefined || url === undefined) return null; + + const tooltip = status?.tooltip ?? `PR #${number}, status pending`; + if (badge?.kind === "pull-request" && badge.others > 0) { + // Unrelated links fold into one state, so a count of merged PRs reads as merged. + const aggregate = PULL_REQUEST_STATE_PRESENTATION[badge.state]; + return { + Icon: aggregate.Icon, + toneClassName: aggregate.toneClassName, + label: `${tooltip}, and ${badge.others} more linked; overall ${aggregate.label.toLowerCase()}`, + text: `+${badge.others + 1}`, + }; + } + return { + Icon: status?.Icon ?? PullRequestGlyph.pullRequest, + toneClassName: status?.colorClass ?? "text-muted-foreground", + label: tooltip, + text: number, + }; } /** The complete linked-PR control shared by the sidebar and composer footer. */ @@ -155,16 +200,9 @@ export function ThreadPullRequestBadgeControl({ onOpenStack: () => void; onOpenPullRequest: (event: MouseEvent) => void; }) { + const presentation = resolveThreadPullRequestBadgePresentation({ badge, number, url, status }); + if (presentation === null) return null; const isStack = badge?.kind === "stack"; - const linkedCount = badge?.kind === "pull-request" && badge.others > 0 ? badge.others + 1 : null; - if (!isStack && (number === undefined || url === undefined)) return null; - const label = isStack - ? `Stack of ${badge.layers} pull requests, ${badge.state}` - : `${status?.tooltip ?? `PR #${number}, status pending`}${ - badge?.kind === "pull-request" && badge.others > 0 - ? `, and ${badge.others} more linked; overall ${badge.state}` - : "" - }`; const className = cn( variant === "ghost" ? buttonVariants({ variant: "ghost", size: "xs" }) @@ -172,14 +210,12 @@ export function ThreadPullRequestBadgeControl({ "text-xs tabular-nums", variant === "ghost" && "font-normal text-xs! active:scale-100 [--control-icon-color:currentColor]", - badge !== null && (isStack || linkedCount !== null) - ? PR_STATE_COLOR_CLASS[badge.state] - : (status?.colorClass ?? "text-muted-foreground"), + presentation.toneClassName, ); const content = ( <> - - {isStack ? badge.layers : linkedCount !== null ? `+${linkedCount}` : number} + + {presentation.text} ); return ( @@ -189,7 +225,7 @@ export function ThreadPullRequestBadgeControl({ isStack ? ( event.stopPropagation()} onClick={(event) => { event.preventDefault(); @@ -203,7 +239,7 @@ export function ThreadPullRequestBadgeControl({ target="_blank" rel="noopener noreferrer" className={className} - aria-label={label} + aria-label={presentation.label} onPointerDown={(event) => event.stopPropagation()} onClick={onOpenPullRequest} /> @@ -212,7 +248,7 @@ export function ThreadPullRequestBadgeControl({ > {content} - {label} + {presentation.label} ); } @@ -254,7 +290,7 @@ export function ThreadPullRequestsMiniList({ className={cn("size-3 shrink-0", presentation.toneClassName)} /> ) : ( - @@ -275,83 +311,24 @@ export function ThreadPullRequestsMiniList({ ); } -/** The ink each pull-request state wears in the sidebar, shared by the number and stack badges. */ -const PR_STATE_COLOR_CLASS: Record = { - open: "text-emerald-600 dark:text-emerald-300/90", - merged: "text-violet-600 dark:text-violet-300/90", - closed: "text-red-600 dark:text-red-300/90", - draft: "text-zinc-500 dark:text-zinc-400/80", -}; - -export function settledPrHoverColorClass( - state: NonNullable["state"], - isDraft = false, -): string { - switch (state) { - case "open": - if (isDraft) { - return "group-hover/sidebar-row:text-zinc-500 dark:group-hover/sidebar-row:text-zinc-400/80"; - } - return "group-hover/sidebar-row:text-emerald-600 dark:group-hover/sidebar-row:text-emerald-300/90"; - case "merged": - return "group-hover/sidebar-row:text-violet-600 dark:group-hover/sidebar-row:text-violet-300/90"; - case "closed": - return "group-hover/sidebar-row:text-red-600 dark:group-hover/sidebar-row:text-red-300/90"; - } -} - export function prStatusIndicator( pr: ThreadPr, provider: VcsStatusResult["sourceControlProvider"] | null | undefined, ): PrStatusIndicator | null { - function formatPrState(pr: NonNullable): string { - if (pr.state === "open" && pr.isDraft === true) return "Draft"; - return pr.state.charAt(0).toUpperCase() + pr.state.slice(1); - } - - function formatPrStatusLead(pr: NonNullable, changeRequestShortName: string): string { - return `${changeRequestShortName} #${pr.number} - ${formatPrState(pr)}`; - } if (!pr) return null; const presentation = resolveChangeRequestPresentation(provider); + const state = resolvePullRequestState({ state: pr.state, isDraft: pr.isDraft === true }); - const tooltipLead = formatPrStatusLead(pr, presentation.shortName); - const tooltip = `${tooltipLead}: ${pr.title}`; - - if (pr.state === "open") { - const isDraft = pr.isDraft === true; - return { - label: `${presentation.shortName} ${isDraft ? "draft" : "open"}`, - colorClass: isDraft - ? "text-zinc-500 dark:text-zinc-400/80" - : "text-emerald-600 dark:text-emerald-300/90", - tooltip, - tooltipLead, - tooltipTitle: pr.title, - url: pr.url, - }; - } - if (pr.state === "closed") { - return { - label: `${presentation.shortName} closed`, - colorClass: "text-red-600 dark:text-red-300/90", - tooltip, - tooltipLead, - tooltipTitle: pr.title, - url: pr.url, - }; - } - if (pr.state === "merged") { - return { - label: `${presentation.shortName} merged`, - colorClass: "text-violet-600 dark:text-violet-300/90", - tooltip, - tooltipLead, - tooltipTitle: pr.title, - url: pr.url, - }; - } - return null; + const tooltipLead = `${presentation.shortName} #${pr.number} - ${state.label}`; + return { + label: `${presentation.shortName} ${state.label.toLowerCase()}`, + colorClass: state.toneClassName, + Icon: state.Icon, + tooltip: `${tooltipLead}: ${pr.title}`, + tooltipLead, + tooltipTitle: pr.title, + url: pr.url, + }; } export function ChangeRequestStatusIcon({ @@ -529,7 +506,7 @@ export function ThreadRowLeadingStatus({ thread }: { thread: SidebarThreadSummar ) : null} {pendingLink ? ( - diff --git a/apps/web/src/components/chat/MessagesTimeline.tsx b/apps/web/src/components/chat/MessagesTimeline.tsx index 568a86721e4c..b20e26debc44 100644 --- a/apps/web/src/components/chat/MessagesTimeline.tsx +++ b/apps/web/src/components/chat/MessagesTimeline.tsx @@ -108,7 +108,6 @@ import { CircleAlertIcon, DownloadIcon, EyeIcon, - GitPullRequestIcon, GlobeIcon, HammerIcon, MessageCircleIcon, @@ -240,6 +239,7 @@ import { formatReviewCommentFence, type ReviewCommentContext, } from "../../reviewCommentContext"; +import { PullRequestGlyph } from "~/components/pullRequest/pullRequestIcons"; // --------------------------------------------------------------------------- // Context — shared state consumed by every row component via Context. @@ -3008,7 +3008,7 @@ const userMessageContextPresentationRegistry = createContextPresentationRegistry ; + return ; case "bot": return ; case "brain": diff --git a/apps/web/src/components/composerContextPresentation.tsx b/apps/web/src/components/composerContextPresentation.tsx index f583a0ee923f..6a945be5d679 100644 --- a/apps/web/src/components/composerContextPresentation.tsx +++ b/apps/web/src/components/composerContextPresentation.tsx @@ -3,7 +3,7 @@ import { ReadOnlySourcePreview } from "./files/AttachmentFilePreview"; import type { PreviewAnnotationPayload } from "@t3tools/contracts"; import { formatAttachmentSize } from "@t3tools/client-runtime/state/attachments"; import { videoMimeType } from "@t3tools/shared/video"; -import { GitPullRequestIcon, MessageCircleIcon, MousePointerClickIcon } from "lucide-react"; +import { MessageCircleIcon, MousePointerClickIcon } from "lucide-react"; import { createContext, type MouseEvent, type ReactElement, type ReactNode, use } from "react"; import type { ComposerFileAttachment, ComposerImageAttachment } from "~/composerDraftStore"; @@ -14,6 +14,7 @@ import { type AttachmentUploadState, } from "~/lib/attachmentUploadState"; import { cn } from "~/lib/utils"; +import { PullRequestGlyph } from "~/components/pullRequest/pullRequestIcons"; import { fileContextReference, imageContextReference, @@ -392,7 +393,7 @@ const composerContextPresentationRegistry = createContextPresentationRegistry< props.onOpen(event, props.metadata.url)} > - + {props.label} } diff --git a/apps/web/src/components/pullRequest/PullRequestCommentComposer.tsx b/apps/web/src/components/pullRequest/PullRequestCommentComposer.tsx index 7d749f0f3fea..22b31d1b4fb1 100644 --- a/apps/web/src/components/pullRequest/PullRequestCommentComposer.tsx +++ b/apps/web/src/components/pullRequest/PullRequestCommentComposer.tsx @@ -1,11 +1,5 @@ import type { EnvironmentId, PullRequestDetailView, PullRequestRef } from "@t3tools/contracts"; -import { - GitPullRequestClosedIcon, - MessageSquareIcon, - RotateCcwIcon, - SendIcon, - XIcon, -} from "lucide-react"; +import { MessageSquareIcon, SendIcon, XIcon } from "lucide-react"; import { useRef, useState } from "react"; import { useAtomCommand } from "~/state/use-atom-command"; @@ -15,6 +9,7 @@ import { Button } from "../ui/button"; import { Popover, PopoverClose, PopoverPopup, PopoverTitle, PopoverTrigger } from "../ui/popover"; import { Textarea } from "../ui/textarea"; import { toastManager } from "../ui/toast"; +import { PullRequestGlyph } from "./pullRequestIcons"; export function PullRequestCommentComposer({ environmentId, @@ -133,9 +128,9 @@ export function PullRequestCommentComposer({ onClick={() => void submit(followUpAction)} > {followUpAction === "close" ? ( - + ) : ( - + )} {submitting === followUpAction ? followUpAction === "close" diff --git a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx index dbd2eb255c4e..6e605d2d6720 100644 --- a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx +++ b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx @@ -27,12 +27,7 @@ import { FolderGit2Icon, GitBranchIcon, GitCommitHorizontalIcon, - GitMergeIcon, - GitPullRequestClosedIcon, - GitPullRequestDraftIcon, - GitPullRequestIcon, HammerIcon, - LayersIcon, MessageCircleQuestionIcon, MessageSquareIcon, LinkIcon, @@ -165,6 +160,7 @@ import { resolvePullRequestState, summarizePullRequestChecks, } from "./pullRequestPresentation"; +import { PullRequestGlyph } from "./pullRequestIcons"; type DetailTab = "summary" | "timeline" | "code"; @@ -444,7 +440,7 @@ function PullRequestBaseFreshnessWarning({ disabled={pending} onClick={() => onUpdate(method)} > - + {method === "rebase" ? "Update with rebase" : "Update branch"} ))} @@ -1762,7 +1758,7 @@ export function PullRequestDetailPanel({ role="img" aria-label={armedAutoMergeLabel} > - + {armedAutoMergeLabel} } @@ -1787,7 +1783,7 @@ export function PullRequestDetailPanel({ handoff === "conflicts" ? "Preparing..." : "Resolve conflicts" } > - + {handoff === "conflicts" ? "Preparing..." : "Resolve conflicts"} @@ -1811,7 +1807,7 @@ export function PullRequestDetailPanel({ onClick={() => void perform("ready")} aria-label="Ready for review" > - + Ready for review @@ -1837,7 +1833,7 @@ export function PullRequestDetailPanel({ : pendingAutoMergeLabel } > - + {pendingAction === "enable-auto-merge" ? "Enabling..." @@ -1861,7 +1857,7 @@ export function PullRequestDetailPanel({ role="img" aria-label={armedAutoMergeLabel} > - + {armedAutoMergeLabel} } @@ -1885,7 +1881,7 @@ export function PullRequestDetailPanel({ pendingAction === "merge" ? "Merging..." : selectedMergeMethodLabel } > - + {pendingAction === "merge" ? "Merging..." : selectedMergeMethodLabel} @@ -1990,9 +1986,9 @@ export function PullRequestDetailPanel({ onClick={() => void perform(detail.isDraft ? "ready" : "draft")} > {detail.isDraft ? ( - + ) : ( - + )} {detail.isDraft ? "Ready for review" : "Convert to draft"} @@ -2002,7 +1998,7 @@ export function PullRequestDetailPanel({ disabled={actionPending} onClick={() => setConfirmation({ open: true, action: "merge" })} > - + Merge now ) : null} @@ -2014,7 +2010,7 @@ export function PullRequestDetailPanel({ disabled={actionPending} onClick={() => void perform("disable-auto-merge")} > - + Disable auto-merge ) : showsAutoMerge ? ( @@ -2024,7 +2020,7 @@ export function PullRequestDetailPanel({ setConfirmation({ open: true, action: "enable-auto-merge" }) } > - + Enable auto-merge ) : null} @@ -2059,7 +2055,7 @@ export function PullRequestDetailPanel({ {/* The radio item lays its children out as one block, so the icon and the label need their own row to share a line. */} - + {PULL_REQUEST_MERGE_METHOD_LABELS[method]} @@ -2092,7 +2088,7 @@ export function PullRequestDetailPanel({ disabled={actionPending} onClick={() => setConfirmation({ open: true, action: "close" })} > - + Close pull request @@ -2100,7 +2096,7 @@ export function PullRequestDetailPanel({ <> void perform("reopen")}> - + Reopen pull request @@ -2169,7 +2165,7 @@ export function PullRequestDetailPanel({ render={ {isStackedPullRequest ? ( - @@ -2353,7 +2349,7 @@ export function PullRequestDetailPanel({ render={ {isStackedPullRequest ? ( - diff --git a/apps/web/src/components/pullRequest/PullRequestListFilters.tsx b/apps/web/src/components/pullRequest/PullRequestListFilters.tsx index 153beb8a0dd5..9b4769109c23 100644 --- a/apps/web/src/components/pullRequest/PullRequestListFilters.tsx +++ b/apps/web/src/components/pullRequest/PullRequestListFilters.tsx @@ -15,7 +15,6 @@ import { CircleXIcon, EyeOffIcon, FolderGit2Icon, - GitPullRequestDraftIcon, LayersIcon, ListFilterIcon, SearchIcon, @@ -51,6 +50,7 @@ import { type PullRequestLabelFacet, } from "./pullRequestList.logic"; import { PullRequestActorAvatar } from "./pullRequestPresentation"; +import { PullRequestGlyph } from "./pullRequestIcons"; export interface PullRequestFilterOption { readonly value: Value; @@ -143,7 +143,7 @@ export const pullRequestProjectKey = (project: { const DRAFT_OPTIONS = [ { value: UNFILTERED_VALUE, label: "All", Icon: LayersIcon }, - { value: "only", label: "Drafts only", Icon: GitPullRequestDraftIcon }, + { value: "only", label: "Drafts only", Icon: PullRequestGlyph.draft }, { value: "hide", label: "Hide drafts", Icon: EyeOffIcon }, ] as const satisfies ReadonlyArray>; diff --git a/apps/web/src/components/pullRequest/PullRequestRow.tsx b/apps/web/src/components/pullRequest/PullRequestRow.tsx index b9b000829945..14b031314739 100644 --- a/apps/web/src/components/pullRequest/PullRequestRow.tsx +++ b/apps/web/src/components/pullRequest/PullRequestRow.tsx @@ -12,6 +12,7 @@ import { pullRequestLabelColor, type EnvironmentPullRequestEntry } from "./pullR import { openOnHostLabel, showPullRequestLinkContextMenu } from "./pullRequestLinkContextMenu"; import { PullRequestActorLabel, + PullRequestConflictGlyph, PullRequestDiffStat, PullRequestMetaLine, PullRequestApprovalGlyph, @@ -112,12 +113,23 @@ function PullRequestRowImpl({ selected ? "bg-accent" : "hover:bg-accent/60", )} > - + {/* The conflict warning rides the corner of the lifecycle glyph, over the arrow's + merge circle, so the leading slot stays one icon wide and titles line up whether or + not a row is blocked. The background fill cuts it out of the glyph beneath. */} + + + {/* The wrapper takes the offset, not the icon, so the tooltip trigger inside keeps the + badge's size and anchors the popup to it. */} + + + + {entry.title} diff --git a/apps/web/src/components/pullRequest/PullRequestStackMenu.tsx b/apps/web/src/components/pullRequest/PullRequestStackMenu.tsx index a9298e982f67..28fc9cdc67f0 100644 --- a/apps/web/src/components/pullRequest/PullRequestStackMenu.tsx +++ b/apps/web/src/components/pullRequest/PullRequestStackMenu.tsx @@ -6,7 +6,7 @@ import type { PullRequestMergeMethod, } from "@t3tools/contracts"; import { squashAtomCommandFailure } from "@t3tools/client-runtime/state/runtime"; -import { GitMergeIcon, LayersIcon, RefreshCwIcon, TriangleAlertIcon } from "lucide-react"; +import { RefreshCwIcon, TriangleAlertIcon } from "lucide-react"; import { useState } from "react"; import { useAtomCommand } from "~/state/use-atom-command"; import { pullRequestEnvironment } from "~/state/pullRequests"; @@ -25,6 +25,7 @@ import { toastManager } from "../ui/toast"; import { PullRequestStackLayers } from "./PullRequestStackLayers"; import { PullRequestStackHeader } from "./PullRequestStackHeader"; import { PullRequestStackLayerContent } from "./PullRequestStackLayerContent"; +import { PullRequestGlyph } from "./pullRequestIcons"; export function PullRequestStackMenu({ stack, @@ -133,7 +134,8 @@ export function PullRequestStackMenu({ /> } > - {position}/{stack.layers.length} + {position}/ + {stack.layers.length} {onRetry ? : null} } @@ -166,7 +168,7 @@ export function PullRequestStackMenu({ {canMerge ? ( setConfirmation("merge")}> - + Merge stack ({mergeLayers.length}) ) : null} @@ -199,7 +201,7 @@ export function PullRequestStackMenu({ disabled={mergeDisabled} onClick={() => setConfirmation("merge")} > - + Merge stack diff --git a/apps/web/src/components/pullRequest/PullRequestStackPopover.tsx b/apps/web/src/components/pullRequest/PullRequestStackPopover.tsx index 451131cb11fe..9f2267e4572c 100644 --- a/apps/web/src/components/pullRequest/PullRequestStackPopover.tsx +++ b/apps/web/src/components/pullRequest/PullRequestStackPopover.tsx @@ -1,11 +1,11 @@ import { Tooltip, TooltipTrigger, TooltipPopup } from "../ui/tooltip"; import type { EnvironmentId, PullRequestRef, PullRequestStackMembership } from "@t3tools/contracts"; -import { LayersIcon } from "lucide-react"; import { useState } from "react"; import { usePullRequestStack } from "~/state/usePullRequestStack"; import { Menu, MenuTrigger, MenuPopup, MenuGroup, MenuGroupLabel, MenuItem } from "../ui/menu"; import { PullRequestStackLayers } from "./PullRequestStackLayers"; import { PullRequestStackHeader } from "./PullRequestStackHeader"; +import { PullRequestGlyph } from "./pullRequestIcons"; /** Mounted only while the menu is open, so list rows do not each fetch a stack. */ function StackBody({ @@ -74,7 +74,7 @@ export function PullRequestStackPopover({ onClick={(event) => event.stopPropagation()} onKeyDown={(event) => event.stopPropagation()} > - + {membership.position}/{membership.size} } diff --git a/apps/web/src/components/pullRequest/PullRequestThreadLinks.tsx b/apps/web/src/components/pullRequest/PullRequestThreadLinks.tsx index 57d08218f80a..ed692a81370e 100644 --- a/apps/web/src/components/pullRequest/PullRequestThreadLinks.tsx +++ b/apps/web/src/components/pullRequest/PullRequestThreadLinks.tsx @@ -1,7 +1,7 @@ import { Tooltip, TooltipTrigger, TooltipPopup } from "../ui/tooltip"; import { scopeThreadRef } from "@t3tools/client-runtime/environment"; import type { EnvironmentId, PullRequestRef, ScopedThreadRef, ThreadId } from "@t3tools/contracts"; -import { CheckIcon, LinkIcon, MessageSquareIcon, UnlinkIcon } from "lucide-react"; +import { CheckIcon, MessageSquareIcon } from "lucide-react"; import { useState } from "react"; import { threadPullRequestLinkMode } from "@t3tools/client-runtime/thread-pull-request-compatibility"; import { usePullRequestLinking } from "~/hooks/usePullRequestLinking"; @@ -18,6 +18,7 @@ import { Command, CommandInput, CommandItem, CommandList } from "../ui/command"; import { Dialog, DialogPopup, DialogTitle } from "../ui/dialog"; import { MenuItem } from "../ui/menu"; import { toastManager } from "../ui/toast"; +import { PullRequestGlyph } from "./pullRequestIcons"; interface PullRequestThreadLinksProps { environmentId: EnvironmentId; @@ -144,9 +145,9 @@ function EnabledPullRequestThreadLinks({ }} > {linkedHere ? ( - + ) : ( - + )} {linkedHere ? "Unlink from this thread" diff --git a/apps/web/src/components/pullRequest/PullRequestTimelineTab.tsx b/apps/web/src/components/pullRequest/PullRequestTimelineTab.tsx index d4112e5d9de4..21037d8bf4c0 100644 --- a/apps/web/src/components/pullRequest/PullRequestTimelineTab.tsx +++ b/apps/web/src/components/pullRequest/PullRequestTimelineTab.tsx @@ -11,9 +11,6 @@ import { ExternalLinkIcon, FileCode2Icon, GitCommitHorizontalIcon, - GitMergeIcon, - GitPullRequestClosedIcon, - GitPullRequestIcon, MessageSquareIcon, PencilIcon, } from "lucide-react"; @@ -51,6 +48,7 @@ import { pullRequestReviewOutcomeStaleLabel, pullRequestReviewOutcomeToneClassName, } from "./pullRequestPresentation"; +import { PullRequestGlyph } from "./pullRequestIcons"; /** What every comment on the timeline needs to react; only the subject differs between them. */ interface ReactionSurface { @@ -419,16 +417,16 @@ function LifecycleEvent({ event }: { event: PullRequestTimelineEvent }) { const presentation = event.kind === "opened" ? { - icon: , + icon: , label: "Pull request opened", } : event.kind === "merged" ? { - icon: , + icon: , label: "Pull request merged", } : { - icon: , + icon: , label: "Pull request closed", }; @@ -629,7 +627,7 @@ export function PullRequestTimelineTab({ {events.length === 0 ? (
- +

No activity yet.

) : null} diff --git a/apps/web/src/components/pullRequest/PullRequestsUnavailableState.tsx b/apps/web/src/components/pullRequest/PullRequestsUnavailableState.tsx index f17ec882c5a0..f2f509944c46 100644 --- a/apps/web/src/components/pullRequest/PullRequestsUnavailableState.tsx +++ b/apps/web/src/components/pullRequest/PullRequestsUnavailableState.tsx @@ -1,5 +1,5 @@ import { RefreshIcon } from "~/components/ui/refresh-icon"; -import { ExternalLinkIcon, GitPullRequestIcon } from "lucide-react"; +import { ExternalLinkIcon } from "lucide-react"; import { Button } from "../ui/button"; import { @@ -10,6 +10,7 @@ import { EmptyMedia, EmptyTitle, } from "../ui/empty"; +import { PullRequestGlyph } from "./pullRequestIcons"; export function PullRequestsUnavailableState({ title = "Could not load pull requests", @@ -27,7 +28,7 @@ export function PullRequestsUnavailableState({ return ( - + {title} diff --git a/apps/web/src/components/pullRequest/ThreadPullRequestsPanel.tsx b/apps/web/src/components/pullRequest/ThreadPullRequestsPanel.tsx index 35a449a3da79..05bb598e11db 100644 --- a/apps/web/src/components/pullRequest/ThreadPullRequestsPanel.tsx +++ b/apps/web/src/components/pullRequest/ThreadPullRequestsPanel.tsx @@ -3,13 +3,7 @@ import { resolveThreadPullRequestChains, visibleThreadPullRequests, } from "@t3tools/shared/threadPullRequests"; -import { - GitPullRequestArrow, - LayersIcon, - LinkIcon, - MoreHorizontalIcon, - PlusIcon, -} from "lucide-react"; +import { ArrowUpRightIcon, LinkIcon, MoreHorizontalIcon, PlusIcon } from "lucide-react"; import { useCallback, useMemo } from "react"; import { writeTextToClipboard } from "~/hooks/useCopyToClipboard"; @@ -28,11 +22,13 @@ import { openLinkPullRequestDialog } from "./LinkPullRequestDialog"; import { pullRequestListLines, type PullRequestListLine } from "./pullRequestListLines"; import { PullRequestActorAvatar, + PullRequestConflictGlyph, PullRequestDiffStat, PullRequestApprovalGlyph, PullRequestStateGlyph, pullRequestChecksStatePresentation, } from "./pullRequestPresentation"; +import { PullRequestGlyph } from "./pullRequestIcons"; const SOURCE_LABELS: Record = { manual: "Linked by you", @@ -84,12 +80,20 @@ function LinkRow({ > {depth > 0 ? : null} {snapshot === null ? ( - ) : ( - + + + + )} Changes requested ) ) : null} - {snapshot?.state === "open" && snapshot.mergeability === "conflicting" ? ( - Conflicts - ) : null} {snapshot?.checksState ? : null} } > - {stack.kind === "native" ? ( - - ) : ( - - )} + {stack.size} @@ -190,9 +187,16 @@ function LinkRow({ } /> - void writeTextToClipboard(link.url, "link")}>Copy link - openPrLink(event, link.url, threadRef)}>Open + void writeTextToClipboard(link.url, "link")}> + + Copy link + + openPrLink(event, link.url, threadRef)}> + + Open + onUnlink(link)}> + {link.source === "stack" ? "Dismiss from thread" : "Unlink from thread"} @@ -250,7 +254,7 @@ function EnabledThreadPullRequestsPanel({ threadRef }: { threadRef: ScopedThread if (links.length === 0) { return (
- +

No linked pull requests

Pull requests the agent opens from this thread land here. Link one yourself from a URL or diff --git a/apps/web/src/components/pullRequest/pullRequestIcons.tsx b/apps/web/src/components/pullRequest/pullRequestIcons.tsx new file mode 100644 index 000000000000..2f1a60a8c1eb --- /dev/null +++ b/apps/web/src/components/pullRequest/pullRequestIcons.tsx @@ -0,0 +1,54 @@ +import { + GitMergeIcon, + GitPullRequestArrowIcon, + GitPullRequestClosedIcon, + GitPullRequestDraftIcon, + LayersIcon, + Link2Icon, + Unlink2Icon, + TriangleAlertIcon, +} from "lucide-react"; +import type { PullRequestState } from "@t3tools/contracts"; + +export const PullRequestGlyph = { + pullRequest: GitPullRequestArrowIcon, + reopen: GitPullRequestArrowIcon, + draft: GitPullRequestDraftIcon, + closed: GitPullRequestClosedIcon, + merged: GitMergeIcon, + conflicting: TriangleAlertIcon, + stack: LayersIcon, + link: Link2Icon, + unlink: Unlink2Icon, +} as const; + +export type PullRequestGlyphIcon = (typeof PullRequestGlyph)[keyof typeof PullRequestGlyph]; + +export interface PullRequestStatePresentation { + readonly label: string; + readonly toneClassName: string; + readonly Icon: PullRequestGlyphIcon; +} + +export const PULL_REQUEST_STATE_PRESENTATION = { + open: { + label: "Open", + toneClassName: "text-emerald-600 dark:text-emerald-300/90", + Icon: PullRequestGlyph.pullRequest, + }, + draft: { + label: "Draft", + toneClassName: "text-zinc-500 dark:text-zinc-400/80", + Icon: PullRequestGlyph.draft, + }, + closed: { + label: "Closed", + toneClassName: "text-red-600 dark:text-red-300/90", + Icon: PullRequestGlyph.closed, + }, + merged: { + label: "Merged", + toneClassName: "text-violet-600 dark:text-violet-300/90", + Icon: PullRequestGlyph.merged, + }, +} as const satisfies Record; diff --git a/apps/web/src/components/pullRequest/pullRequestPresentation.test.ts b/apps/web/src/components/pullRequest/pullRequestPresentation.test.ts new file mode 100644 index 000000000000..be2efa94c207 --- /dev/null +++ b/apps/web/src/components/pullRequest/pullRequestPresentation.test.ts @@ -0,0 +1,139 @@ +import { describe, expect, it } from "vite-plus/test"; + +import { resolvePullRequestConflict, resolvePullRequestState } from "./pullRequestPresentation"; +import { PullRequestGlyph } from "./pullRequestIcons"; + +describe("resolvePullRequestState", () => { + it.each([ + [ + "open", + { state: "open", isDraft: false }, + PullRequestGlyph.pullRequest, + "Open", + "text-emerald-600 dark:text-emerald-300/90", + ], + [ + "draft", + { state: "open", isDraft: true }, + PullRequestGlyph.draft, + "Draft", + "text-zinc-500 dark:text-zinc-400/80", + ], + [ + "closed", + { state: "closed", isDraft: false }, + PullRequestGlyph.closed, + "Closed", + "text-red-600 dark:text-red-300/90", + ], + [ + "merged", + { state: "merged", isDraft: false }, + PullRequestGlyph.merged, + "Merged", + "text-violet-600 dark:text-violet-300/90", + ], + ] as const)( + "resolves the %s lifecycle presentation", + (_name, input, Icon, label, toneClassName) => { + expect(resolvePullRequestState(input)).toEqual({ + Icon, + label, + toneClassName, + }); + }, + ); + + it("keeps a merged pull request merged when stale draft metadata is also present", () => { + expect(resolvePullRequestState({ state: "merged", isDraft: true })).toMatchObject({ + Icon: PullRequestGlyph.merged, + label: "Merged", + }); + }); + + it("keeps a closed pull request closed when stale draft metadata is also present", () => { + expect(resolvePullRequestState({ state: "closed", isDraft: true })).toMatchObject({ + Icon: PullRequestGlyph.closed, + label: "Closed", + }); + }); + + it("keeps lifecycle and conflict presentation independent for an open conflicting pull request", () => { + const input = { + state: "open" as const, + isDraft: false, + mergeability: "conflicting" as const, + baseBranch: "main", + }; + + expect(resolvePullRequestState(input)).toMatchObject({ + Icon: PullRequestGlyph.pullRequest, + label: "Open", + }); + expect(resolvePullRequestConflict(input)).toMatchObject({ + Icon: PullRequestGlyph.conflicting, + label: "Conflicts with main", + toneClassName: "text-destructive", + }); + }); +}); + +describe("resolvePullRequestConflict", () => { + it.each([ + ["closed", { state: "closed", isDraft: false }], + ["merged", { state: "merged", isDraft: false }], + ["closed draft", { state: "closed", isDraft: true }], + ["merged draft", { state: "merged", isDraft: true }], + ["open draft", { state: "open", isDraft: true }], + ] as const)("does not report a conflict for %s", (_name, input) => { + expect( + resolvePullRequestConflict({ + ...input, + mergeability: "conflicting", + baseBranch: "main", + }), + ).toBeNull(); + }); + + it.each([ + ["omitted", undefined], + ["unknown", "unknown"], + ["mergeable", "mergeable"], + ] as const)("does not report an open conflict when mergeability is %s", (_name, mergeability) => { + const input = { + state: "open" as const, + isDraft: false, + ...(mergeability === undefined ? {} : { mergeability }), + }; + expect(resolvePullRequestConflict(input)).toBeNull(); + }); + + it("reports a known conflict with its base branch", () => { + expect( + resolvePullRequestConflict({ + state: "open", + isDraft: false, + mergeability: "conflicting", + baseBranch: "main", + }), + ).toEqual({ + Icon: PullRequestGlyph.conflicting, + label: "Conflicts with main", + toneClassName: "text-destructive", + }); + }); + + it("reports a known conflict without inventing a base branch", () => { + expect( + resolvePullRequestConflict({ + state: "open", + isDraft: false, + mergeability: "conflicting", + }), + ).toEqual({ + Icon: PullRequestGlyph.conflicting, + label: "Has conflicts", + toneClassName: "text-destructive", + }); + }); +}); diff --git a/apps/web/src/components/pullRequest/pullRequestPresentation.tsx b/apps/web/src/components/pullRequest/pullRequestPresentation.tsx index 890efb67cb9b..4792cca95ea7 100644 --- a/apps/web/src/components/pullRequest/pullRequestPresentation.tsx +++ b/apps/web/src/components/pullRequest/pullRequestPresentation.tsx @@ -12,11 +12,6 @@ import { CircleDashedIcon, CircleDotIcon, CircleXIcon, - GitMergeIcon, - GitPullRequestClosedIcon, - GitPullRequestDraftIcon, - GitPullRequestIcon, - TriangleAlertIcon, UserCheckIcon, } from "lucide-react"; import { Children, isValidElement, type ReactNode } from "react"; @@ -26,12 +21,12 @@ import { cn } from "~/lib/utils"; import { Badge } from "../ui/badge"; import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; import type { PullRequestReviewOutcome } from "./pullRequestDetail.logic"; - -interface StatePresentation { - readonly label: string; - readonly toneClassName: string; - readonly Icon: typeof GitPullRequestIcon; -} +import { + PULL_REQUEST_STATE_PRESENTATION, + PullRequestGlyph, + type PullRequestStatePresentation, + type PullRequestGlyphIcon, +} from "./pullRequestIcons"; export function PullRequestApprovalGlyph() { return ( @@ -49,57 +44,69 @@ export function PullRequestApprovalGlyph() { } /** - * How a pull request's state reads on this page. Open, closed, merged, and draft use the same - * ink as the thread badge in `ThreadStatusIndicators`, so one pull request cannot look like two - * different things in two places. + * How a pull request's state reads anywhere it appears: the thread badge, the right-panel tab, + * the list, and the detail header all resolve through here so one pull request cannot look like + * two different things in two places. * - * Draft outranks conflicts: a draft is not heading for a merge yet, so conflicts only surface - * once it is real work. + * Closed and merged take precedence over a stale draft flag. */ export function resolvePullRequestState(input: { readonly state: PullRequestState; readonly isDraft: boolean; +}): PullRequestStatePresentation { + const key = input.state === "open" && input.isDraft ? "draft" : input.state; + return PULL_REQUEST_STATE_PRESENTATION[key]; +} + +export interface PullRequestConflictPresentation { + readonly label: string; + readonly toneClassName: string; + readonly Icon: PullRequestGlyphIcon; +} + +export function resolvePullRequestConflict(input: { + readonly state: PullRequestState; + readonly isDraft: boolean; readonly mergeability?: PullRequestMergeability; readonly baseBranch?: string; -}): StatePresentation { - if (input.state === "merged") { - return { - label: "Merged", - toneClassName: "text-violet-600 dark:text-violet-300/90", - Icon: GitMergeIcon, - }; - } - if (input.state === "closed") { - return { - label: "Closed", - toneClassName: "text-red-600 dark:text-red-300/90", - Icon: GitPullRequestClosedIcon, - }; - } - if (input.isDraft) { - return { - label: "Draft", - toneClassName: "text-zinc-500 dark:text-zinc-400/80", - Icon: GitPullRequestDraftIcon, - }; - } - if (input.mergeability === "conflicting") { - return { - // "Has conflicts" leaves out the one thing a reader wants when the warning triangle catches - // their eye, so name the branch it collides with wherever the caller knows it. - label: input.baseBranch ? `Conflicts with ${input.baseBranch}` : "Has conflicts", - toneClassName: "text-destructive", - Icon: TriangleAlertIcon, - }; +}): PullRequestConflictPresentation | null { + if (input.state !== "open" || input.isDraft || input.mergeability !== "conflicting") { + return null; } return { - label: "Open", - toneClassName: "text-emerald-600 dark:text-emerald-300/90", - Icon: GitPullRequestIcon, + label: input.baseBranch ? `Conflicts with ${input.baseBranch}` : "Has conflicts", + toneClassName: "text-destructive", + Icon: PullRequestGlyph.conflicting, }; } export function PullRequestStateGlyph({ + state, + isDraft, + className, +}: { + state: PullRequestState; + isDraft: boolean; + className?: string; +}) { + const presentation = resolvePullRequestState({ state, isDraft }); + return ( + + {/* The list row is itself a button, so the trigger stays a span: an interactive one would + nest a control inside that button and steal the row's click target. */} + }> + + + {presentation.label} + + ); +} + +export function PullRequestConflictGlyph({ state, isDraft, mergeability, @@ -112,16 +119,15 @@ export function PullRequestStateGlyph({ baseBranch?: string; className?: string; }) { - const presentation = resolvePullRequestState({ + const presentation = resolvePullRequestConflict({ state, isDraft, - ...(mergeability ? { mergeability } : {}), - ...(baseBranch ? { baseBranch } : {}), + ...(mergeability === undefined ? {} : { mergeability }), + ...(baseBranch === undefined ? {} : { baseBranch }), }); + if (presentation === null) return null; return ( - {/* The list row is itself a button, so the trigger stays a span: an interactive one would - nest a control inside that button and steal the row's click target. */} }> - + diff --git a/apps/web/src/components/sidebar/SidebarChrome.tsx b/apps/web/src/components/sidebar/SidebarChrome.tsx index afbbf7671dfc..97c6a43d3ada 100644 --- a/apps/web/src/components/sidebar/SidebarChrome.tsx +++ b/apps/web/src/components/sidebar/SidebarChrome.tsx @@ -1,9 +1,4 @@ -import { - ArrowLeftIcon, - ChartNoAxesColumnIcon, - GitPullRequestIcon, - SettingsIcon, -} from "lucide-react"; +import { ArrowLeftIcon, ChartNoAxesColumnIcon, SettingsIcon } from "lucide-react"; import type { ReactNode } from "react"; import { memo, useCallback } from "react"; import { Link, useCanGoBack, useLocation, useNavigate } from "@tanstack/react-router"; @@ -33,6 +28,7 @@ import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; import { readPullRequestListPreferences } from "../pullRequest/pullRequestListPreferences"; import { SidebarProviderUpdatePill } from "./SidebarProviderUpdatePill"; import { SidebarUpdateArchitectureWarning, SidebarUpdatePill } from "./SidebarUpdatePill"; +import { PullRequestGlyph } from "~/components/pullRequest/pullRequestIcons"; export const SidebarChromeHeader = memo(function SidebarChromeHeader({ isElectron, @@ -204,7 +200,7 @@ export const SidebarUtilityMenu = memo(function SidebarUtilityMenu() { /> {pullRequestsSupported ? ( } + icon={} label="Pull Requests" onClick={handlePullRequestsClick} /> diff --git a/apps/web/src/routes/_chat.pull-requests.tsx b/apps/web/src/routes/_chat.pull-requests.tsx index 973c406f6900..1dca2dcd9317 100644 --- a/apps/web/src/routes/_chat.pull-requests.tsx +++ b/apps/web/src/routes/_chat.pull-requests.tsx @@ -21,9 +21,6 @@ import { ChevronDownIcon, ClockIcon, EyeIcon, - GitMergeIcon, - GitPullRequestClosedIcon, - GitPullRequestIcon, LayersIcon, ListChecksIcon, PenLineIcon, @@ -150,6 +147,7 @@ import { useAtomCommand } from "../state/use-atom-command"; import { cn } from "~/lib/utils"; import { primaryServerKeybindingsAtom } from "~/state/server"; import { getSourceControlPresentationForKind } from "~/sourceControlPresentation"; +import { PullRequestGlyph } from "~/components/pullRequest/pullRequestIcons"; export interface PullRequestsSearch extends PullRequestListPreferences { /** @@ -186,9 +184,9 @@ const INVOLVEMENT_TABS = [ const STATE_TABS = [ { value: "all", label: "All", Icon: LayersIcon }, - { value: "open", label: "Open", Icon: GitPullRequestIcon }, - { value: "closed", label: "Closed", Icon: GitPullRequestClosedIcon }, - { value: "merged", label: "Merged", Icon: GitMergeIcon }, + { value: "open", label: "Open", Icon: PullRequestGlyph.pullRequest }, + { value: "closed", label: "Closed", Icon: PullRequestGlyph.closed }, + { value: "merged", label: "Merged", Icon: PullRequestGlyph.merged }, ] as const satisfies ReadonlyArray>; const SORT_OPTIONS = [ diff --git a/apps/web/src/sourceControlPresentation.ts b/apps/web/src/sourceControlPresentation.ts index 0f627bca5a17..7a8d2cab8fe5 100644 --- a/apps/web/src/sourceControlPresentation.ts +++ b/apps/web/src/sourceControlPresentation.ts @@ -1,4 +1,3 @@ -import { GitPullRequestIcon } from "lucide-react"; import type { ElementType } from "react"; import type { SourceControlProviderInfo, SourceControlProviderKind } from "@t3tools/contracts"; export { @@ -16,10 +15,11 @@ import { import { AzureDevOpsIcon, BitbucketIcon, + ForgejoIcon, GitHubIcon, GitLabIcon, - ForgejoIcon, } from "./components/Icons"; +import { PullRequestGlyph } from "~/components/pullRequest/pullRequestIcons"; export interface SourceControlPresentation { readonly providerName: string; @@ -66,7 +66,7 @@ export function getSourceControlPresentation( return { providerName: provider?.name || presentation.providerName, terminology: getChangeRequestTerminology(provider), - Icon: GitPullRequestIcon, + Icon: PullRequestGlyph.pullRequest, }; } } diff --git a/vite.config.ts b/vite.config.ts index b9f2c9cc2c4b..46a54377aad4 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -2,6 +2,43 @@ import "vite-plus/test/config"; import { defineConfig } from "vite-plus"; import * as NodeURL from "node:url"; +/** Import restrictions every file keeps, including the one module exempt from the glyph rule. */ +const RESTRICTED_IMPORT_PATHS = [ + { + name: "@t3tools/client-runtime", + message: + "Import from an explicit @t3tools/client-runtime/* subpath. The package has no root export.", + }, + { + name: "@pierre/diffs/react", + importNames: ["CodeView"], + message: "Use StyledDiffCodeView so web diff surfaces share styling and virtualized geometry.", + }, +]; + +/** Lucide's pull-request glyphs, which only `pullRequestIcons.tsx` may name. */ +const RESTRICTED_PULL_REQUEST_GLYPH_IMPORTS = { + name: "lucide-react", + importNames: [ + "GitMerge", + "GitMergeIcon", + "GitPullRequest", + "GitPullRequestIcon", + "GitPullRequestArrow", + "GitPullRequestArrowIcon", + "GitPullRequestClosed", + "GitPullRequestClosedIcon", + "GitPullRequestDraft", + "GitPullRequestDraftIcon", + "GitPullRequestCreate", + "GitPullRequestCreateIcon", + "GitPullRequestCreateArrow", + "GitPullRequestCreateArrowIcon", + ], + message: + "Pick a glyph by meaning from PullRequestGlyph in apps/web/src/components/pullRequest/pullRequestIcons.tsx so every surface draws the same pull request the same way.", +}; + export default defineConfig({ resolve: { alias: { @@ -103,21 +140,7 @@ export default defineConfig({ "typescript/unbound-method": "off", "eslint/no-restricted-imports": [ "error", - { - paths: [ - { - name: "@t3tools/client-runtime", - message: - "Import from an explicit @t3tools/client-runtime/* subpath. The package has no root export.", - }, - { - name: "@pierre/diffs/react", - importNames: ["CodeView"], - message: - "Use StyledDiffCodeView so web diff surfaces share styling and virtualized geometry.", - }, - ], - }, + { paths: [...RESTRICTED_IMPORT_PATHS, RESTRICTED_PULL_REQUEST_GLYPH_IMPORTS] }, ], "t3code/no-global-process-runtime": "error", "t3code/no-inline-schema-compile": "warn", @@ -131,6 +154,12 @@ export default defineConfig({ files: ["packages/shared/src/hostProcess.ts"], rules: { "t3code/no-global-process-runtime": "off" }, }, + { + // The one module allowed to name lucide's pull-request glyphs; everything else picks + // from its vocabulary. The other import restrictions still apply here. + files: ["apps/web/src/components/pullRequest/pullRequestIcons.tsx"], + rules: { "eslint/no-restricted-imports": ["error", { paths: RESTRICTED_IMPORT_PATHS }] }, + }, { files: ["apps/mobile/src/**"], rules: { "t3code/no-mobile-uniwind-theme-escape-hatches": "error" },