| author | |
| committer | |
| log | 411da2a7356dd795b720228e9c834ea9bad0415f |
| tree | cf1f61b093df47af023e9a80642e1c3da1556c9b |
| parent | 58f89818e3ada7f680bede396d41ed27eb618203 |
| signature | Signed by PGP key B5690EEEBB952194 |
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>11 files changed, 719 insertions(+), 356 deletions(-)
apps/server/src/pullRequest/gitHubPullRequestJson.test.ts+34| ... | @@ -97,6 +97,40 @@ describe("pull request list decoding", () => { | ... | @@ -97,6 +97,40 @@ describe("pull request list decoding", () => { |
| 97 | ]); | 97 | ]); |
| 98 | }); | 98 | }); |
| 99 | 99 | ||
| 100 | it("takes the verdict from the latest reviews when GitHub summarizes none, as for a bot's approval", () => { | ||
| 101 | const batch = expectSuccess( | ||
| 102 | decodePullRequestListJson( | ||
| 103 | listJson([ | ||
| 104 | { | ||
| 105 | reviewDecision: null, | ||
| 106 | latestReviews: [{ author: { login: "macroscopeapp" }, state: "APPROVED" }], | ||
| 107 | }, | ||
| 108 | { | ||
| 109 | reviewDecision: "REVIEW_REQUIRED", | ||
| 110 | latestReviews: [ | ||
| 111 | { author: { login: "octocat" }, state: "APPROVED" }, | ||
| 112 | { author: { login: "hubot" }, state: "CHANGES_REQUESTED" }, | ||
| 113 | ], | ||
| 114 | }, | ||
| 115 | { | ||
| 116 | reviewDecision: "APPROVED", | ||
| 117 | latestReviews: [{ author: { login: "hubot" }, state: "CHANGES_REQUESTED" }], | ||
| 118 | }, | ||
| 119 | { | ||
| 120 | reviewDecision: null, | ||
| 121 | latestReviews: [{ author: { login: "octocat" }, state: "COMMENTED" }], | ||
| 122 | }, | ||
| 123 | ]), | ||
| 124 | ), | ||
| 125 | ); | ||
| 126 | expect(batch.items.map((entry) => entry.reviewDecision)).toEqual([ | ||
| 127 | "approved", | ||
| 128 | "changes-requested", | ||
| 129 | "approved", | ||
| 130 | null, | ||
| 131 | ]); | ||
| 132 | }); | ||
| 133 | |||
| 100 | it("rolls the head commit's checks up to the one word a row has space for", () => { | 134 | it("rolls the head commit's checks up to the one word a row has space for", () => { |
| 101 | const batch = expectSuccess( | 135 | const batch = expectSuccess( |
| 102 | decodePullRequestListJson( | 136 | decodePullRequestListJson( |
apps/server/src/pullRequest/gitHubPullRequestJson.ts+47-10| ... | @@ -68,6 +68,12 @@ const RawReviewRequestSchema = Schema.Struct({ | ... | @@ -68,6 +68,12 @@ const RawReviewRequestSchema = Schema.Struct({ |
| 68 | name: Schema.optional(Schema.NullOr(Schema.String)), | 68 | name: Schema.optional(Schema.NullOr(Schema.String)), |
| 69 | }); | 69 | }); |
| 70 | 70 | ||
| 71 | /** One reviewer's most recent review: the state is all the verdict needs, the author is for who. */ | ||
| 72 | const RawLatestReviewSchema = Schema.Struct({ | ||
| 73 | author: Schema.optional(Schema.NullOr(RawActorSchema)), | ||
| 74 | state: Schema.optional(Schema.NullOr(Schema.String)), | ||
| 75 | }); | ||
| 76 | |||
| 71 | const RawCheckSchema = Schema.Struct({ | 77 | const RawCheckSchema = Schema.Struct({ |
| 72 | __typename: Schema.optional(Schema.String), | 78 | __typename: Schema.optional(Schema.String), |
| 73 | name: Schema.optional(Schema.NullOr(Schema.String)), | 79 | name: Schema.optional(Schema.NullOr(Schema.String)), |
| ... | @@ -106,6 +112,7 @@ const RawListItemSchema = Schema.Struct({ | ... | @@ -106,6 +112,7 @@ const RawListItemSchema = Schema.Struct({ |
| 106 | updatedAt: Schema.String, | 112 | updatedAt: Schema.String, |
| 107 | mergedAt: Schema.optional(Schema.NullOr(Schema.String)), | 113 | mergedAt: Schema.optional(Schema.NullOr(Schema.String)), |
| 108 | reviewRequests: Schema.optional(Schema.Array(RawReviewRequestSchema)), | 114 | reviewRequests: Schema.optional(Schema.Array(RawReviewRequestSchema)), |
| 115 | latestReviews: Schema.optional(Schema.NullOr(Schema.Array(RawLatestReviewSchema))), | ||
| 109 | labels: Schema.optional(Schema.Array(RawLabelSchema)), | 116 | labels: Schema.optional(Schema.Array(RawLabelSchema)), |
| 110 | /** | 117 | /** |
| 111 | * Every check of the head commit, which is the only rollup `gh pr list --json` can give: there | 118 | * Every check of the head commit, which is the only rollup `gh pr list --json` can give: there |
| ... | @@ -143,6 +150,9 @@ const RawSearchItemSchema = Schema.Struct({ | ... | @@ -143,6 +150,9 @@ const RawSearchItemSchema = Schema.Struct({ |
| 143 | isDraft: Schema.optional(Schema.Boolean), | 150 | isDraft: Schema.optional(Schema.Boolean), |
| 144 | mergeable: Schema.optional(Schema.NullOr(Schema.String)), | 151 | mergeable: Schema.optional(Schema.NullOr(Schema.String)), |
| 145 | reviewDecision: Schema.optional(Schema.NullOr(Schema.String)), | 152 | reviewDecision: Schema.optional(Schema.NullOr(Schema.String)), |
| 153 | latestReviews: Schema.optional( | ||
| 154 | Schema.NullOr(Schema.Struct({ nodes: Schema.Array(Schema.NullOr(RawLatestReviewSchema)) })), | ||
| 155 | ), | ||
| 146 | createdAt: Schema.String, | 156 | createdAt: Schema.String, |
| 147 | updatedAt: Schema.String, | 157 | updatedAt: Schema.String, |
| 148 | mergedAt: Schema.optional(Schema.NullOr(Schema.String)), | 158 | mergedAt: Schema.optional(Schema.NullOr(Schema.String)), |
| ... | @@ -514,13 +524,7 @@ const RawReviewThreadsSchema = Schema.Struct({ | ... | @@ -514,13 +524,7 @@ const RawReviewThreadsSchema = Schema.Struct({ |
| 514 | ), | 524 | ), |
| 515 | ), | 525 | ), |
| 516 | latestReviews: Schema.optional( | 526 | latestReviews: Schema.optional( |
| 517 | Schema.NullOr( | 527 | Schema.NullOr(Schema.Struct({ nodes: Schema.Array(RawLatestReviewSchema) })), |
| 518 | Schema.Struct({ | ||
| 519 | nodes: Schema.Array( | ||
| 520 | Schema.Struct({ author: Schema.optional(Schema.NullOr(RawActorSchema)) }), | ||
| 521 | ), | ||
| 522 | }), | ||
| 523 | ), | ||
| 524 | ), | 528 | ), |
| 525 | reviewDismissals: Schema.optional( | 529 | reviewDismissals: Schema.optional( |
| 526 | Schema.NullOr( | 530 | Schema.NullOr( |
| ... | @@ -698,7 +702,7 @@ export function decodeActorAvatarsJson( | ... | @@ -698,7 +702,7 @@ export function decodeActorAvatarsJson( |
| 698 | } | 702 | } |
| 699 | 703 | ||
| 700 | export const PULL_REQUEST_LIST_JSON_FIELDS = | 704 | export const PULL_REQUEST_LIST_JSON_FIELDS = |
| 701 | "number,title,url,author,headRefName,baseRefName,state,isDraft,mergeable,reviewDecision,additions,deletions,createdAt,updatedAt,mergedAt,reviewRequests,labels,statusCheckRollup"; | 705 | "number,title,url,author,headRefName,baseRefName,state,isDraft,mergeable,reviewDecision,additions,deletions,createdAt,updatedAt,mergedAt,reviewRequests,latestReviews,labels,statusCheckRollup"; |
| 702 | 706 | ||
| 703 | export const PULL_REQUEST_DETAIL_JSON_FIELDS = `${PULL_REQUEST_LIST_JSON_FIELDS},body,changedFiles,closedAt,isCrossRepository,headRepositoryOwner,headRefOid,autoMergeRequest`; | 707 | export const PULL_REQUEST_DETAIL_JSON_FIELDS = `${PULL_REQUEST_LIST_JSON_FIELDS},body,changedFiles,closedAt,isCrossRepository,headRepositoryOwner,headRefOid,autoMergeRequest`; |
| 704 | 708 | ||
| ... | @@ -817,6 +821,7 @@ export function pullRequestSearchGraphQlQuery(rows: number, includeStacks = fals | ... | @@ -817,6 +821,7 @@ export function pullRequestSearchGraphQlQuery(rows: number, includeStacks = fals |
| 817 | isDraft | 821 | isDraft |
| 818 | mergeable | 822 | mergeable |
| 819 | reviewDecision | 823 | reviewDecision |
| 824 | latestReviews(first: 20) { nodes { state author { login } } } | ||
| 820 | createdAt | 825 | createdAt |
| 821 | updatedAt | 826 | updatedAt |
| 822 | mergedAt | 827 | mergedAt |
| ... | @@ -891,7 +896,7 @@ export const REVIEW_THREADS_GRAPHQL_QUERY = `query($owner: String!, $name: Strin | ... | @@ -891,7 +896,7 @@ export const REVIEW_THREADS_GRAPHQL_QUERY = `query($owner: String!, $name: Strin |
| 891 | } | 896 | } |
| 892 | } | 897 | } |
| 893 | latestReviews(first: 50) { | 898 | latestReviews(first: 50) { |
| 894 | nodes { author { __typename login avatarUrl } } | 899 | nodes { state author { __typename login avatarUrl } } |
| 895 | } | 900 | } |
| 896 | reviewDismissals: timelineItems(itemTypes: [REVIEW_DISMISSED_EVENT], first: ${GRAPHQL_PAGE_SIZE}) { | 901 | reviewDismissals: timelineItems(itemTypes: [REVIEW_DISMISSED_EVENT], first: ${GRAPHQL_PAGE_SIZE}) { |
| 897 | pageInfo { hasNextPage endCursor } | 902 | pageInfo { hasNextPage endCursor } |
| ... | @@ -1325,6 +1330,35 @@ function toMergeMethod(value: string | null | undefined): PullRequestMergeMethod | ... | @@ -1325,6 +1330,35 @@ function toMergeMethod(value: string | null | undefined): PullRequestMergeMethod |
| 1325 | } | 1330 | } |
| 1326 | } | 1331 | } |
| 1327 | 1332 | ||
| 1333 | /** | ||
| 1334 | * GitHub's own `reviewDecision` counts only reviews that satisfy the branch rules, so an | ||
| 1335 | * approval from an app (a review bot) or from anyone without the required permission leaves it | ||
| 1336 | * empty. The reviewers still said something, and a row should show it: when GitHub reports no | ||
| 1337 | * verdict, the latest review per reviewer decides, changes requested outranking approval. | ||
| 1338 | */ | ||
| 1339 | function toReviewDecisionWithReviews( | ||
| 1340 | value: string | null | undefined, | ||
| 1341 | // `gh pr list` hands the reviews as an array; the GraphQL reads hand a connection. | ||
| 1342 | latestReviews: | ||
| 1343 | | ReadonlyArray<Schema.Schema.Type<typeof RawLatestReviewSchema>> | ||
| 1344 | | { readonly nodes: ReadonlyArray<Schema.Schema.Type<typeof RawLatestReviewSchema>> } | ||
| 1345 | | null | ||
| 1346 | | undefined, | ||
| 1347 | ): PullRequestReviewDecision | null { | ||
| 1348 | const summarized = toReviewDecision(value); | ||
| 1349 | if (summarized === "approved" || summarized === "changes-requested") return summarized; | ||
| 1350 | const reviews = | ||
| 1351 | latestReviews === null || latestReviews === undefined | ||
| 1352 | ? [] | ||
| 1353 | : "nodes" in latestReviews | ||
| 1354 | ? latestReviews.nodes | ||
| 1355 | : latestReviews; | ||
| 1356 | const states = new Set(reviews.map((review) => review.state?.trim().toUpperCase() ?? "")); | ||
| 1357 | if (states.has("CHANGES_REQUESTED")) return "changes-requested"; | ||
| 1358 | if (states.has("APPROVED")) return "approved"; | ||
| 1359 | return summarized; | ||
| 1360 | } | ||
| 1361 | |||
| 1328 | function toReviewDecision(value: string | null | undefined): PullRequestReviewDecision | null { | 1362 | function toReviewDecision(value: string | null | undefined): PullRequestReviewDecision | null { |
| 1329 | switch (value?.trim().toUpperCase()) { | 1363 | switch (value?.trim().toUpperCase()) { |
| 1330 | case "APPROVED": | 1364 | case "APPROVED": |
| ... | @@ -1558,7 +1592,7 @@ function toListItem(raw: Schema.Schema.Type<typeof RawListItemSchema>): GitHubPu | ... | @@ -1558,7 +1592,7 @@ function toListItem(raw: Schema.Schema.Type<typeof RawListItemSchema>): GitHubPu |
| 1558 | state: toState(raw), | 1592 | state: toState(raw), |
| 1559 | isDraft: raw.isDraft ?? false, | 1593 | isDraft: raw.isDraft ?? false, |
| 1560 | mergeability: toMergeability(raw.mergeable), | 1594 | mergeability: toMergeability(raw.mergeable), |
| 1561 | reviewDecision: toReviewDecision(raw.reviewDecision), | 1595 | reviewDecision: toReviewDecisionWithReviews(raw.reviewDecision, raw.latestReviews), |
| 1562 | additions: raw.additions ?? 0, | 1596 | additions: raw.additions ?? 0, |
| 1563 | deletions: raw.deletions ?? 0, | 1597 | deletions: raw.deletions ?? 0, |
| 1564 | createdAt: raw.createdAt, | 1598 | createdAt: raw.createdAt, |
| ... | @@ -1681,6 +1715,9 @@ export function decodePullRequestSearchJson( | ... | @@ -1681,6 +1715,9 @@ export function decodePullRequestSearchJson( |
| 1681 | items.push({ | 1715 | items.push({ |
| 1682 | ...toListItem({ | 1716 | ...toListItem({ |
| 1683 | ...node, | 1717 | ...node, |
| 1718 | latestReviews: (node.latestReviews?.nodes ?? []).flatMap((review) => | ||
| 1719 | review === null ? [] : [review], | ||
| 1720 | ), | ||
| 1684 | reviewRequests: (node.reviewRequests?.nodes ?? []).flatMap((request) => { | 1721 | reviewRequests: (node.reviewRequests?.nodes ?? []).flatMap((request) => { |
| 1685 | const login = trimmed(request?.requestedReviewer?.login); | 1722 | const login = trimmed(request?.requestedReviewer?.login); |
| 1686 | return login === null ? [] : [{ login }]; | 1723 | return login === null ? [] : [{ login }]; |
apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx+82-46| ... | @@ -50,6 +50,7 @@ import { | ... | @@ -50,6 +50,7 @@ import { |
| 50 | useMemo, | 50 | useMemo, |
| 51 | useRef, | 51 | useRef, |
| 52 | useState, | 52 | useState, |
| 53 | type ReactNode, | ||
| 53 | } from "react"; | 54 | } from "react"; |
| 54 | 55 | ||
| 55 | import { type DraftId, useComposerDraftStore } from "~/composerDraftStore"; | 56 | import { type DraftId, useComposerDraftStore } from "~/composerDraftStore"; |
| ... | @@ -344,6 +345,8 @@ function PullRequestBaseFreshnessWarning({ | ... | @@ -344,6 +345,8 @@ function PullRequestBaseFreshnessWarning({ |
| 344 | pending, | 345 | pending, |
| 345 | onUpdate, | 346 | onUpdate, |
| 346 | iconClassName, | 347 | iconClassName, |
| 348 | className, | ||
| 349 | children, | ||
| 347 | }: { | 350 | }: { |
| 348 | readonly baseBranch: string; | 351 | readonly baseBranch: string; |
| 349 | readonly freshness: { | 352 | readonly freshness: { |
| ... | @@ -353,6 +356,9 @@ function PullRequestBaseFreshnessWarning({ | ... | @@ -353,6 +356,9 @@ function PullRequestBaseFreshnessWarning({ |
| 353 | readonly pending: boolean; | 356 | readonly pending: boolean; |
| 354 | readonly onUpdate: (method: PullRequestUpdateMethod) => void; | 357 | readonly onUpdate: (method: PullRequestUpdateMethod) => void; |
| 355 | readonly iconClassName?: string; | 358 | readonly iconClassName?: string; |
| 359 | readonly className?: string; | ||
| 360 | /** What the warning is about, drawn in the same amber before the mark: the base branch. */ | ||
| 361 | readonly children?: ReactNode; | ||
| 356 | }) { | 362 | }) { |
| 357 | const behind = | 363 | const behind = |
| 358 | freshness.behindBy === null | 364 | freshness.behindBy === null |
| ... | @@ -371,11 +377,15 @@ function PullRequestBaseFreshnessWarning({ | ... | @@ -371,11 +377,15 @@ function PullRequestBaseFreshnessWarning({ |
| 371 | <button | 377 | <button |
| 372 | type="button" | 378 | type="button" |
| 373 | aria-label={summary} | 379 | aria-label={summary} |
| 374 | className="inline-flex shrink-0 cursor-help items-center rounded-sm text-amber-600 outline-none focus-visible:ring-2 focus-visible:ring-ring" | 380 | className={cn( |
| 381 | "inline-flex min-w-0 shrink-0 cursor-help items-center gap-1 rounded-sm text-amber-600 outline-none focus-visible:ring-2 focus-visible:ring-ring dark:text-amber-400/90", | ||
| 382 | className, | ||
| 383 | )} | ||
| 375 | /> | 384 | /> |
| 376 | } | 385 | } |
| 377 | > | 386 | > |
| 378 | <TriangleAlertIcon aria-hidden className={cn("size-3.5", iconClassName)} /> | 387 | {children} |
| 388 | <TriangleAlertIcon aria-hidden className={cn("size-3.5 shrink-0", iconClassName)} /> | ||
| 379 | </PopoverTrigger> | 389 | </PopoverTrigger> |
| 380 | <PopoverPopup | 390 | <PopoverPopup |
| 381 | align="start" | 391 | align="start" |
| ... | @@ -2239,26 +2249,9 @@ export function PullRequestDetailPanel({ | ... | @@ -2239,26 +2249,9 @@ export function PullRequestDetailPanel({ |
| 2239 | </span> | 2249 | </span> |
| 2240 | <span aria-hidden className="h-3 w-px shrink-0 bg-border/70" /> | 2250 | <span aria-hidden className="h-3 w-px shrink-0 bg-border/70" /> |
| 2241 | <span className="flex min-w-0 flex-1 items-center gap-1.5 font-mono text-[11px] text-muted-foreground/65"> | 2251 | <span className="flex min-w-0 flex-1 items-center gap-1.5 font-mono text-[11px] text-muted-foreground/65"> |
| 2242 | <Tooltip> | 2252 | {/* An out-of-date base wears the warning on the branch name itself, so the |
| 2243 | <TooltipTrigger | 2253 | name is amber and pointing at either the name or the mark opens the way |
| 2244 | render={ | 2254 | out. Up to date, the name keeps its plain tooltip. */} |
| 2245 | <span className="inline-flex min-w-0 max-w-[40%] shrink-0 items-center gap-1"> | ||
| 2246 | {isStackedPullRequest ? ( | ||
| 2247 | <PullRequestGlyph.stack | ||
| 2248 | aria-label="Stacked pull request" | ||
| 2249 | className="size-3 shrink-0" | ||
| 2250 | /> | ||
| 2251 | ) : null} | ||
| 2252 | <code className="min-w-0 truncate">{detail.baseBranch}</code> | ||
| 2253 | </span> | ||
| 2254 | } | ||
| 2255 | /> | ||
| 2256 | <TooltipPopup side="top"> | ||
| 2257 | {isStackedPullRequest | ||
| 2258 | ? `Stacked on ${detail.baseBranch}` | ||
| 2259 | : detail.baseBranch} | ||
| 2260 | </TooltipPopup> | ||
| 2261 | </Tooltip> | ||
| 2262 | {freshness ? ( | 2255 | {freshness ? ( |
| 2263 | <PullRequestBaseFreshnessWarning | 2256 | <PullRequestBaseFreshnessWarning |
| 2264 | baseBranch={detail.baseBranch} | 2257 | baseBranch={detail.baseBranch} |
| ... | @@ -2266,8 +2259,38 @@ export function PullRequestDetailPanel({ | ... | @@ -2266,8 +2259,38 @@ export function PullRequestDetailPanel({ |
| 2266 | pending={actionPending} | 2259 | pending={actionPending} |
| 2267 | onUpdate={(method) => void perform("update-branch", undefined, method)} | 2260 | onUpdate={(method) => void perform("update-branch", undefined, method)} |
| 2268 | iconClassName="size-3" | 2261 | iconClassName="size-3" |
| 2269 | /> | 2262 | className="max-w-[40%]" |
| 2270 | ) : null} | 2263 | > |
| 2264 | {isStackedPullRequest ? ( | ||
| 2265 | <PullRequestGlyph.stack | ||
| 2266 | aria-label="Stacked pull request" | ||
| 2267 | className="size-3 shrink-0" | ||
| 2268 | /> | ||
| 2269 | ) : null} | ||
| 2270 | <code className="min-w-0 truncate">{detail.baseBranch}</code> | ||
| 2271 | </PullRequestBaseFreshnessWarning> | ||
| 2272 | ) : ( | ||
| 2273 | <Tooltip> | ||
| 2274 | <TooltipTrigger | ||
| 2275 | render={ | ||
| 2276 | <span className="inline-flex min-w-0 max-w-[40%] shrink-0 items-center gap-1"> | ||
| 2277 | {isStackedPullRequest ? ( | ||
| 2278 | <PullRequestGlyph.stack | ||
| 2279 | aria-label="Stacked pull request" | ||
| 2280 | className="size-3 shrink-0" | ||
| 2281 | /> | ||
| 2282 | ) : null} | ||
| 2283 | <code className="min-w-0 truncate">{detail.baseBranch}</code> | ||
| 2284 | </span> | ||
| 2285 | } | ||
| 2286 | /> | ||
| 2287 | <TooltipPopup side="top"> | ||
| 2288 | {isStackedPullRequest | ||
| 2289 | ? `Stacked on ${detail.baseBranch}` | ||
| 2290 | : detail.baseBranch} | ||
| 2291 | </TooltipPopup> | ||
| 2292 | </Tooltip> | ||
| 2293 | )} | ||
| 2271 | <ArrowLeftIcon | 2294 | <ArrowLeftIcon |
| 2272 | aria-label="receives changes from" | 2295 | aria-label="receives changes from" |
| 2273 | className="size-3 shrink-0 opacity-60" | 2296 | className="size-3 shrink-0 opacity-60" |
| ... | @@ -2412,34 +2435,47 @@ export function PullRequestDetailPanel({ | ... | @@ -2412,34 +2435,47 @@ export function PullRequestDetailPanel({ |
| 2412 | 2435 | ||
| 2413 | <div className="mt-4 flex min-h-5 min-w-0 items-center gap-2 text-xs text-muted-foreground"> | 2436 | <div className="mt-4 flex min-h-5 min-w-0 items-center gap-2 text-xs text-muted-foreground"> |
| 2414 | <span className="flex min-w-0 flex-1 items-center gap-1.5 font-mono text-xs text-muted-foreground/70"> | 2437 | <span className="flex min-w-0 flex-1 items-center gap-1.5 font-mono text-xs text-muted-foreground/70"> |
| 2415 | <Tooltip> | 2438 | {/* An out-of-date base wears the warning on the branch name itself, so the |
| 2416 | <TooltipTrigger | 2439 | name is amber and pointing at either the name or the mark opens the way |
| 2417 | render={ | 2440 | out. Up to date, the name keeps its plain tooltip. */} |
| 2418 | <span className="inline-flex min-w-0 max-w-[40%] shrink-0 items-center gap-1"> | ||
| 2419 | {isStackedPullRequest ? ( | ||
| 2420 | <PullRequestGlyph.stack | ||
| 2421 | aria-label="Stacked pull request" | ||
| 2422 | className="size-3 shrink-0" | ||
| 2423 | /> | ||
| 2424 | ) : null} | ||
| 2425 | <code className="min-w-0 truncate">{detail.baseBranch}</code> | ||
| 2426 | </span> | ||
| 2427 | } | ||
| 2428 | /> | ||
| 2429 | <TooltipPopup side="top"> | ||
| 2430 | {isStackedPullRequest | ||
| 2431 | ? `Stacked on ${detail.baseBranch}` | ||
| 2432 | : detail.baseBranch} | ||
| 2433 | </TooltipPopup> | ||
| 2434 | </Tooltip> | ||
| 2435 | {freshness ? ( | 2441 | {freshness ? ( |
| 2436 | <PullRequestBaseFreshnessWarning | 2442 | <PullRequestBaseFreshnessWarning |
| 2437 | baseBranch={detail.baseBranch} | 2443 | baseBranch={detail.baseBranch} |
| 2438 | freshness={freshness} | 2444 | freshness={freshness} |
| 2439 | pending={actionPending} | 2445 | pending={actionPending} |
| 2440 | onUpdate={(method) => void perform("update-branch", undefined, method)} | 2446 | onUpdate={(method) => void perform("update-branch", undefined, method)} |
| 2441 | /> | 2447 | className="max-w-[40%]" |
| 2442 | ) : null} | 2448 | > |
| 2449 | {isStackedPullRequest ? ( | ||
| 2450 | <PullRequestGlyph.stack | ||
| 2451 | aria-label="Stacked pull request" | ||
| 2452 | className="size-3 shrink-0" | ||
| 2453 | /> | ||
| 2454 | ) : null} | ||
| 2455 | <code className="min-w-0 truncate">{detail.baseBranch}</code> | ||
| 2456 | </PullRequestBaseFreshnessWarning> | ||
| 2457 | ) : ( | ||
| 2458 | <Tooltip> | ||
| 2459 | <TooltipTrigger | ||
| 2460 | render={ | ||
| 2461 | <span className="inline-flex min-w-0 max-w-[40%] shrink-0 items-center gap-1"> | ||
| 2462 | {isStackedPullRequest ? ( | ||
| 2463 | <PullRequestGlyph.stack | ||
| 2464 | aria-label="Stacked pull request" | ||
| 2465 | className="size-3 shrink-0" | ||
| 2466 | /> | ||
| 2467 | ) : null} | ||
| 2468 | <code className="min-w-0 truncate">{detail.baseBranch}</code> | ||
| 2469 | </span> | ||
| 2470 | } | ||
| 2471 | /> | ||
| 2472 | <TooltipPopup side="top"> | ||
| 2473 | {isStackedPullRequest | ||
| 2474 | ? `Stacked on ${detail.baseBranch}` | ||
| 2475 | : detail.baseBranch} | ||
| 2476 | </TooltipPopup> | ||
| 2477 | </Tooltip> | ||
| 2478 | )} | ||
| 2443 | <ArrowLeftIcon | 2479 | <ArrowLeftIcon |
| 2444 | aria-label="receives changes from" | 2480 | aria-label="receives changes from" |
| 2445 | className="size-3.5 shrink-0 opacity-60" | 2481 | className="size-3.5 shrink-0 opacity-60" |
apps/web/src/components/pullRequest/PullRequestGhosts.tsx+14-22| ... | @@ -28,10 +28,10 @@ import { formatRelativeTimeLabel } from "~/timestampFormat"; | ... | @@ -28,10 +28,10 @@ import { formatRelativeTimeLabel } from "~/timestampFormat"; |
| 28 | import { Button, InlineButton } from "../ui/button"; | 28 | import { Button, InlineButton } from "../ui/button"; |
| 29 | import { Toggle, ToggleGroup } from "../ui/toggle-group"; | 29 | import { Toggle, ToggleGroup } from "../ui/toggle-group"; |
| 30 | import { PullRequestCopyableCode } from "./PullRequestCopyableCode"; | 30 | import { PullRequestCopyableCode } from "./PullRequestCopyableCode"; |
| 31 | import { pullRequestLabelColor } from "./pullRequestList.logic"; | ||
| 32 | import { | 31 | import { |
| 33 | PullRequestActorLabel, | 32 | PullRequestActorLabel, |
| 34 | PullRequestDiffStat, | 33 | PullRequestDiffStat, |
| 34 | PullRequestLabelChip, | ||
| 35 | PullRequestMetaLine, | 35 | PullRequestMetaLine, |
| 36 | pullRequestChecksStatePresentation, | 36 | pullRequestChecksStatePresentation, |
| 37 | resolvePullRequestState, | 37 | resolvePullRequestState, |
| ... | @@ -71,12 +71,12 @@ export function PullRequestListGhost({ | ... | @@ -71,12 +71,12 @@ export function PullRequestListGhost({ |
| 71 | {Array.from({ length: rows }, (_, index) => ( | 71 | {Array.from({ length: rows }, (_, index) => ( |
| 72 | <div | 72 | <div |
| 73 | key={index} | 73 | key={index} |
| 74 | className="grid grid-cols-[auto_minmax(0,1fr)_auto] items-center gap-3 rounded-lg px-3 py-2" | 74 | className="grid grid-cols-[auto_minmax(0,1fr)_auto] items-center gap-2 rounded-md px-3 py-2.5" |
| 75 | > | 75 | > |
| 76 | <GhostBar className="size-4 rounded-full" /> | 76 | <GhostBar className="size-4 rounded-full" /> |
| 77 | <div className="min-w-0 space-y-1.5"> | 77 | <div className="min-w-0 space-y-1.5"> |
| 78 | <GhostBar className={cn("h-3.5", TITLE_WIDTHS[index % TITLE_WIDTHS.length])} /> | 78 | <GhostBar className={cn("h-4", TITLE_WIDTHS[index % TITLE_WIDTHS.length])} /> |
| 79 | <GhostBar className={META_WIDTHS[index % META_WIDTHS.length]} /> | 79 | <GhostBar className={cn("h-3.5", META_WIDTHS[index % META_WIDTHS.length])} /> |
| 80 | </div> | 80 | </div> |
| 81 | <div className="flex flex-col items-end gap-1.5"> | 81 | <div className="flex flex-col items-end gap-1.5"> |
| 82 | <GhostBar className="w-12" /> | 82 | <GhostBar className="w-12" /> |
| ... | @@ -356,29 +356,21 @@ export function PullRequestDetailGhost({ | ... | @@ -356,29 +356,21 @@ export function PullRequestDetailGhost({ |
| 356 | <span className="flex min-w-0 flex-wrap items-center gap-1 text-foreground"> | 356 | <span className="flex min-w-0 flex-wrap items-center gap-1 text-foreground"> |
| 357 | {entry?.labels ? ( | 357 | {entry?.labels ? ( |
| 358 | entry.labels.length > 0 ? ( | 358 | entry.labels.length > 0 ? ( |
| 359 | entry.labels.map((label) => { | 359 | entry.labels.map((label) => ( |
| 360 | const color = pullRequestLabelColor(label.color); | 360 | <PullRequestLabelChip |
| 361 | return ( | 361 | key={label.name} |
| 362 | <span | 362 | label={label} |
| 363 | key={label.name} | 363 | size="default" |
| 364 | className="inline-flex max-w-48 items-center gap-1.5 truncate rounded-full bg-muted/40 py-0.5 pl-1.5 pr-2 text-xs" | 364 | className="max-w-48" |
| 365 | > | 365 | /> |
| 366 | <span | 366 | )) |
| 367 | aria-hidden | ||
| 368 | className="size-2 shrink-0 rounded-full bg-muted-foreground" | ||
| 369 | {...(color ? { style: { backgroundColor: color } } : {})} | ||
| 370 | /> | ||
| 371 | <span className="truncate">{label.name}</span> | ||
| 372 | </span> | ||
| 373 | ); | ||
| 374 | }) | ||
| 375 | ) : ( | 367 | ) : ( |
| 376 | <span className="text-muted-foreground">None</span> | 368 | <span className="text-muted-foreground">None</span> |
| 377 | ) | 369 | ) |
| 378 | ) : ( | 370 | ) : ( |
| 379 | <> | 371 | <> |
| 380 | <GhostBar className="h-4 w-24 rounded-full" /> | 372 | <GhostBar className="h-4.5 w-24" /> |
| 381 | <GhostBar className="h-4 w-20 rounded-full" /> | 373 | <GhostBar className="h-4.5 w-20" /> |
| 382 | </> | 374 | </> |
| 383 | )} | 375 | )} |
| 384 | <Button size="icon-xs" variant="ghost" disabled aria-label="Labels loading"> | 376 | <Button size="icon-xs" variant="ghost" disabled aria-label="Labels loading"> |
apps/web/src/components/pullRequest/PullRequestListRow.tsx created+158| ... | @@ -0,0 +1,158 @@ | ||
| 1 | import type { | ||
| 2 | PullRequestActor, | ||
| 3 | PullRequestMergeability, | ||
| 4 | PullRequestState, | ||
| 5 | } from "@t3tools/contracts"; | ||
| 6 | import type { ReactNode } from "react"; | ||
| 7 | |||
| 8 | import { cn } from "~/lib/utils"; | ||
| 9 | import { formatRelativeTimeLabel } from "~/timestampFormat"; | ||
| 10 | |||
| 11 | import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; | ||
| 12 | import { | ||
| 13 | PullRequestActorAvatar, | ||
| 14 | PullRequestConflictGlyph, | ||
| 15 | PullRequestStateGlyph, | ||
| 16 | } from "./pullRequestPresentation"; | ||
| 17 | |||
| 18 | /** | ||
| 19 | * The one row shape both pull request lists share: the full page and a thread's linked panel. | ||
| 20 | * A status column, then two lines — number and title with the diff counts on the right; | ||
| 21 | * author and whatever else the caller shows under them with the time on the right. The column | ||
| 22 | * stacks the lifecycle glyph over the checks glyph, so what a pull request is and how it is | ||
| 23 | * doing read top to bottom at the left edge, and the right edge is only numbers and time. The | ||
| 24 | * page puts repository and labels there, the panel puts the branches there. The caller owns | ||
| 25 | * the wrapper (a link on the panel, a button on the page) and hands in the slots. | ||
| 26 | */ | ||
| 27 | export const PULL_REQUEST_ROW_CLASS = | ||
| 28 | "group/pr-row flex w-full items-center gap-2 rounded-md py-1 pr-1 text-left"; | ||
| 29 | |||
| 30 | export const PULL_REQUEST_ROW_NUMBER_CLASS = | ||
| 31 | "shrink-0 font-mono text-xs tabular-nums text-muted-foreground"; | ||
| 32 | |||
| 33 | /** | ||
| 34 | * The conflict warning rides the corner of the lifecycle glyph, over the arrow's merge circle, | ||
| 35 | * so the leading slot stays one icon wide and titles line up whether or not a row is blocked. | ||
| 36 | * The background fill cuts it out of the glyph beneath. | ||
| 37 | */ | ||
| 38 | export function PullRequestRowGlyph({ | ||
| 39 | state, | ||
| 40 | isDraft, | ||
| 41 | mergeability, | ||
| 42 | baseBranch, | ||
| 43 | below, | ||
| 44 | className, | ||
| 45 | }: { | ||
| 46 | state: PullRequestState; | ||
| 47 | isDraft: boolean; | ||
| 48 | mergeability?: PullRequestMergeability | undefined; | ||
| 49 | baseBranch?: string | undefined; | ||
| 50 | /** Under the lifecycle glyph, level with the second line: the checks glyph. */ | ||
| 51 | below?: ReactNode; | ||
| 52 | className?: string; | ||
| 53 | }) { | ||
| 54 | return ( | ||
| 55 | <span className={cn("flex w-4 shrink-0 flex-col items-center gap-0.5", className)}> | ||
| 56 | <span className="relative inline-flex"> | ||
| 57 | <PullRequestStateGlyph state={state} isDraft={isDraft} /> | ||
| 58 | {/* The wrapper takes the offset, not the icon, so the tooltip trigger inside keeps the | ||
| 59 | badge's size and anchors the popup to it. */} | ||
| 60 | <span className="absolute -right-1 -bottom-1 inline-flex"> | ||
| 61 | <PullRequestConflictGlyph | ||
| 62 | state={state} | ||
| 63 | isDraft={isDraft} | ||
| 64 | {...(mergeability === undefined ? {} : { mergeability })} | ||
| 65 | {...(baseBranch === undefined ? {} : { baseBranch })} | ||
| 66 | className="size-3 fill-background [stroke-width:2.5]" | ||
| 67 | /> | ||
| 68 | </span> | ||
| 69 | </span> | ||
| 70 | {below ? <span className="inline-flex text-[11px]">{below}</span> : null} | ||
| 71 | </span> | ||
| 72 | ); | ||
| 73 | } | ||
| 74 | |||
| 75 | export function PullRequestRowLines({ | ||
| 76 | number, | ||
| 77 | title, | ||
| 78 | status, | ||
| 79 | signals, | ||
| 80 | meta, | ||
| 81 | metaClassName, | ||
| 82 | updatedAt, | ||
| 83 | }: { | ||
| 84 | /** The `#n` reference, already wrapped in whatever tooltip or menu the caller wants on it. */ | ||
| 85 | number: ReactNode; | ||
| 86 | title: ReactNode; | ||
| 87 | /** Right end of the first line: stack, diff counts. */ | ||
| 88 | status?: ReactNode; | ||
| 89 | /** Right after the title text: checks and review verdict glyphs. */ | ||
| 90 | signals?: ReactNode; | ||
| 91 | /** Left of the second line: author, then repository and labels or the branches. */ | ||
| 92 | meta?: ReactNode; | ||
| 93 | metaClassName?: string; | ||
| 94 | updatedAt?: string | null | undefined; | ||
| 95 | }) { | ||
| 96 | return ( | ||
| 97 | <span className="min-w-0 flex-1"> | ||
| 98 | <span className="flex min-w-0 items-center gap-1.5"> | ||
| 99 | {number} | ||
| 100 | <span className="min-w-0 truncate text-sm">{title}</span> | ||
| 101 | {signals ? ( | ||
| 102 | <span className="flex shrink-0 items-center gap-1 text-[11px]">{signals}</span> | ||
| 103 | ) : null} | ||
| 104 | {status ? ( | ||
| 105 | <span className="ml-auto flex shrink-0 items-center gap-1.5 text-[11px]">{status}</span> | ||
| 106 | ) : null} | ||
| 107 | </span> | ||
| 108 | <span | ||
| 109 | className={cn( | ||
| 110 | "flex min-w-0 items-center gap-1.5 overflow-hidden text-[11px] text-muted-foreground", | ||
| 111 | metaClassName, | ||
| 112 | )} | ||
| 113 | > | ||
| 114 | {meta} | ||
| 115 | {updatedAt ? ( | ||
| 116 | <span className="ml-auto shrink-0 whitespace-nowrap tabular-nums"> | ||
| 117 | {formatRelativeTimeLabel(updatedAt)} | ||
| 118 | </span> | ||
| 119 | ) : null} | ||
| 120 | </span> | ||
| 121 | </span> | ||
| 122 | ); | ||
| 123 | } | ||
| 124 | |||
| 125 | /** Avatar and login at the meta line's own size, with the actor's full name on hover. */ | ||
| 126 | export function PullRequestRowAuthor({ | ||
| 127 | actor, | ||
| 128 | className, | ||
| 129 | labelClassName, | ||
| 130 | }: { | ||
| 131 | actor: PullRequestActor | null; | ||
| 132 | className?: string; | ||
| 133 | labelClassName?: string; | ||
| 134 | }) { | ||
| 135 | const login = actor?.login ?? "ghost"; | ||
| 136 | return ( | ||
| 137 | <Tooltip> | ||
| 138 | <TooltipTrigger | ||
| 139 | render={<span className={cn("inline-flex min-w-0 items-center gap-1", className)} />} | ||
| 140 | > | ||
| 141 | <PullRequestActorAvatar actor={actor} className="size-3.5" /> | ||
| 142 | <span className={cn("truncate", labelClassName)}>{login}</span> | ||
| 143 | </TooltipTrigger> | ||
| 144 | <TooltipPopup side="top"> | ||
| 145 | {actor?.name && actor.name !== login ? `${actor.name} (@${login})` : login} | ||
| 146 | </TooltipPopup> | ||
| 147 | </Tooltip> | ||
| 148 | ); | ||
| 149 | } | ||
| 150 | |||
| 151 | /** `head → base`, in the mono the branches are typed in. */ | ||
| 152 | export function PullRequestRowBranches({ head, base }: { head: string; base: string }) { | ||
| 153 | return ( | ||
| 154 | <span className="truncate font-mono"> | ||
| 155 | {head} → {base} | ||
| 156 | </span> | ||
| 157 | ); | ||
| 158 | } | ||
apps/web/src/components/pullRequest/PullRequestRow.tsx+129-137| ... | @@ -4,19 +4,22 @@ import { memo, type RefCallback } from "react"; | ... | @@ -4,19 +4,22 @@ import { memo, type RefCallback } from "react"; |
| 4 | 4 | ||
| 5 | import { cn } from "~/lib/utils"; | 5 | import { cn } from "~/lib/utils"; |
| 6 | import { getSourceControlPresentationForKind } from "~/sourceControlPresentation"; | 6 | import { getSourceControlPresentationForKind } from "~/sourceControlPresentation"; |
| 7 | import { formatRelativeTimeLabel } from "~/timestampFormat"; | ||
| 8 | 7 | ||
| 9 | import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; | 8 | import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; |
| 10 | import { PullRequestChecksPopover } from "./PullRequestChecksPopover"; | 9 | import { PullRequestChecksPopover } from "./PullRequestChecksPopover"; |
| 11 | import { pullRequestLabelColor, type EnvironmentPullRequestEntry } from "./pullRequestList.logic"; | 10 | import type { EnvironmentPullRequestEntry } from "./pullRequestList.logic"; |
| 12 | import { openOnHostLabel, showPullRequestLinkContextMenu } from "./pullRequestLinkContextMenu"; | 11 | import { openOnHostLabel, showPullRequestLinkContextMenu } from "./pullRequestLinkContextMenu"; |
| 13 | import { | 12 | import { |
| 14 | PullRequestActorLabel, | 13 | PULL_REQUEST_ROW_CLASS, |
| 15 | PullRequestConflictGlyph, | 14 | PULL_REQUEST_ROW_NUMBER_CLASS, |
| 15 | PullRequestRowAuthor, | ||
| 16 | PullRequestRowGlyph, | ||
| 17 | PullRequestRowLines, | ||
| 18 | } from "./PullRequestListRow"; | ||
| 19 | import { | ||
| 16 | PullRequestDiffStat, | 20 | PullRequestDiffStat, |
| 17 | PullRequestMetaLine, | 21 | PullRequestLabelChip, |
| 18 | PullRequestApprovalGlyph, | 22 | PullRequestReviewDecisionGlyph, |
| 19 | PullRequestStateGlyph, | ||
| 20 | } from "./pullRequestPresentation"; | 23 | } from "./pullRequestPresentation"; |
| 21 | 24 | ||
| 22 | /** | 25 | /** |
| ... | @@ -37,32 +40,26 @@ function PullRequestRowLabels({ labels }: { labels: EnvironmentPullRequestEntry[ | ... | @@ -37,32 +40,26 @@ function PullRequestRowLabels({ labels }: { labels: EnvironmentPullRequestEntry[ |
| 37 | {LABEL_SLOTS.map((slot, index) => { | 40 | {LABEL_SLOTS.map((slot, index) => { |
| 38 | const label = labels[index]; | 41 | const label = labels[index]; |
| 39 | if (!label) return null; | 42 | if (!label) return null; |
| 40 | const dot = pullRequestLabelColor(label.color); | ||
| 41 | const remaining = labels.length - index - 1; | 43 | const remaining = labels.length - index - 1; |
| 42 | return ( | 44 | return ( |
| 43 | <span | 45 | <PullRequestLabelChip key={label.name} label={label} className={slot.pill}> |
| 44 | key={label.name} | ||
| 45 | className={cn( | ||
| 46 | "inline-flex max-w-40 min-w-0 items-center gap-1 rounded-full border border-border/70 bg-muted/40 py-0 pl-1 pr-1.5 text-[10px] leading-3.5 text-muted-foreground", | ||
| 47 | slot.pill, | ||
| 48 | )} | ||
| 49 | > | ||
| 50 | <span | ||
| 51 | aria-hidden | ||
| 52 | className="size-2 shrink-0 rounded-full bg-muted-foreground" | ||
| 53 | {...(dot ? { style: { backgroundColor: dot } } : {})} | ||
| 54 | /> | ||
| 55 | <span className="truncate">{label.name}</span> | ||
| 56 | {remaining > 0 ? ( | 46 | {remaining > 0 ? ( |
| 57 | <span className={cn("shrink-0", slot.overflow)}>+{remaining}</span> | 47 | <span className={cn("shrink-0", slot.overflow)}>+{remaining}</span> |
| 58 | ) : null} | 48 | ) : null} |
| 59 | </span> | 49 | </PullRequestLabelChip> |
| 60 | ); | 50 | ); |
| 61 | })} | 51 | })} |
| 62 | </span> | 52 | </span> |
| 63 | ); | 53 | ); |
| 64 | } | 54 | } |
| 65 | 55 | ||
| 56 | /** | ||
| 57 | * The page row keeps a little more room around the shared lines than the panel, which sits in | ||
| 58 | * a narrow column. The intrinsic size is the content box a skipped row reserves, which is the | ||
| 59 | * two lines without the padding: a 56px row less 20px of `py-2.5`. | ||
| 60 | */ | ||
| 61 | const PAGE_ROW_CLASS = "px-3 py-2.5 [contain-intrinsic-block-size:36.5px]"; | ||
| 62 | |||
| 66 | export type PullRequestRowTarget = Pick< | 63 | export type PullRequestRowTarget = Pick< |
| 67 | EnvironmentPullRequestEntry, | 64 | EnvironmentPullRequestEntry, |
| 68 | "environmentId" | "projectId" | "host" | "repository" | "number" | 65 | "environmentId" | "projectId" | "host" | "repository" | "number" |
| ... | @@ -105,95 +102,109 @@ function PullRequestRowImpl({ | ... | @@ -105,95 +102,109 @@ function PullRequestRowImpl({ |
| 105 | aria-current={selected ? "true" : undefined} | 102 | aria-current={selected ? "true" : undefined} |
| 106 | onClick={() => onSelect(entry)} | 103 | onClick={() => onSelect(entry)} |
| 107 | className={cn( | 104 | className={cn( |
| 108 | "@container/pr-row grid w-full cursor-pointer grid-cols-[auto_minmax(0,1fr)] items-center gap-3 rounded-lg px-3 py-3 text-left transition-colors focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-ring", | 105 | PULL_REQUEST_ROW_CLASS, |
| 106 | PAGE_ROW_CLASS, | ||
| 107 | "cursor-pointer transition-colors focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-ring", | ||
| 109 | // Offscreen rows are skipped for style, layout and paint: a long list costs what the | 108 | // Offscreen rows are skipped for style, layout and paint: a long list costs what the |
| 110 | // viewport shows, not what the pages have loaded. The intrinsic size keeps the | 109 | // viewport shows, not what the pages have loaded. The intrinsic size keeps the |
| 111 | // scrollbar honest while a row is skipped. | 110 | // scrollbar honest while a row is skipped. |
| 112 | "[contain-intrinsic-block-size:66px] [content-visibility:auto]", | 111 | "[content-visibility:auto]", |
| 113 | selected ? "bg-accent" : "hover:bg-accent/60", | 112 | selected ? "bg-accent" : "hover:bg-accent/60", |
| 114 | )} | 113 | )} |
| 115 | > | 114 | > |
| 116 | {/* The conflict warning rides the corner of the lifecycle glyph, over the arrow's | 115 | <PullRequestRowGlyph |
| 117 | merge circle, so the leading slot stays one icon wide and titles line up whether or | 116 | state={entry.state} |
| 118 | not a row is blocked. The background fill cuts it out of the glyph beneath. */} | 117 | isDraft={entry.isDraft} |
| 119 | <span className="relative inline-flex self-start mt-0.75 shrink-0"> | 118 | mergeability={entry.mergeability} |
| 120 | <PullRequestStateGlyph state={entry.state} isDraft={entry.isDraft} /> | 119 | baseBranch={entry.baseBranch} |
| 121 | {/* The wrapper takes the offset, not the icon, so the tooltip trigger inside keeps the | 120 | // On the title line rather than between the lines, as main aligns it. |
| 122 | badge's size and anchors the popup to it. */} | 121 | className="mt-0.75 self-start" |
| 123 | <span className="absolute -right-1 -bottom-1 inline-flex"> | 122 | /> |
| 124 | <PullRequestConflictGlyph | 123 | <PullRequestRowLines |
| 125 | state={entry.state} | 124 | number={ |
| 126 | isDraft={entry.isDraft} | 125 | // The number carries the link, here as much as on the detail: a right-click on it |
| 127 | mergeability={entry.mergeability} | 126 | // copies the pull request's own address rather than opening the editing menu. |
| 128 | baseBranch={entry.baseBranch} | 127 | <span |
| 129 | className="size-3 fill-background [stroke-width:2.5]" | 128 | className={PULL_REQUEST_ROW_NUMBER_CLASS} |
| 130 | /> | 129 | onContextMenu={(event) => { |
| 131 | </span> | 130 | event.preventDefault(); |
| 132 | </span> | 131 | event.stopPropagation(); |
| 133 | <span className="grid min-w-0 grid-cols-[minmax(0,1fr)_auto] items-center gap-x-3 gap-y-1.5"> | 132 | void showPullRequestLinkContextMenu({ |
| 134 | <span className="col-start-1 row-start-1 block truncate text-sm font-medium text-foreground"> | 133 | url: entry.url, |
| 135 | {entry.title} | 134 | openLabel: openOnHostLabel(entry.provider), |
| 136 | </span> | 135 | position: { x: event.clientX, y: event.clientY }, |
| 137 | <span className="col-start-2 row-start-1 flex items-center justify-self-end gap-2 text-xs"> | 136 | }); |
| 138 | {entry.stack ? ( | 137 | }} |
| 139 | <PullRequestStackPopover | 138 | > |
| 140 | environmentId={entry.environmentId} | 139 | #{entry.number} |
| 141 | reference={{ | 140 | </span> |
| 142 | projectId: entry.projectId, | 141 | } |
| 143 | host: entry.host, | 142 | title={entry.title} |
| 144 | repository: entry.repository, | 143 | signals={ |
| 145 | number: entry.number, | 144 | <> |
| 146 | }} | 145 | {entry.checksState === undefined ? null : ( |
| 147 | membership={entry.stack} | 146 | <PullRequestChecksPopover |
| 148 | onSelect={(target) => | 147 | checksState={entry.checksState} |
| 149 | onSelect({ ...target, host: entry.host, environmentId: entry.environmentId }) | 148 | environmentId={entry.environmentId} |
| 150 | } | 149 | reference={{ |
| 151 | /> | 150 | projectId: entry.projectId, |
| 152 | ) : null} | 151 | repository: entry.repository, |
| 153 | {/* Only a verdict somebody has actually given: "review required" is the absence of | 152 | number: entry.number, |
| 154 | one, and saying so on every unreviewed row would say nothing. */} | 153 | }} |
| 155 | {entry.reviewDecision === "approved" ? ( | 154 | /> |
| 156 | <PullRequestApprovalGlyph /> | 155 | )} |
| 157 | ) : entry.reviewDecision === "changes-requested" ? ( | 156 | {/* Only a verdict the host actually reports: an approval, a request for changes, |
| 158 | <span className="min-w-0 truncate text-amber-600/90 dark:text-amber-400/80"> | 157 | or a review the branch rules still require. No glyph on the common case of a |
| 159 | Changes requested | 158 | pull request nobody has reviewed, so a row only wears a person when the person |
| 160 | </span> | 159 | has said something. */} |
| 161 | ) : null} | 160 | {entry.reviewDecision === undefined ? null : ( |
| 162 | {entry.checksState === undefined ? null : ( | 161 | <PullRequestReviewDecisionGlyph decision={entry.reviewDecision} /> |
| 163 | <PullRequestChecksPopover | 162 | )} |
| 164 | checksState={entry.checksState} | 163 | </> |
| 165 | environmentId={entry.environmentId} | 164 | } |
| 166 | reference={{ | 165 | status={ |
| 167 | projectId: entry.projectId, | 166 | <> |
| 168 | repository: entry.repository, | 167 | {entry.stack ? ( |
| 169 | number: entry.number, | 168 | <PullRequestStackPopover |
| 170 | }} | 169 | environmentId={entry.environmentId} |
| 171 | /> | 170 | reference={{ |
| 172 | )} | 171 | projectId: entry.projectId, |
| 173 | <PullRequestDiffStat | 172 | host: entry.host, |
| 174 | additions={entry.additions} | 173 | repository: entry.repository, |
| 175 | deletions={entry.deletions} | 174 | number: entry.number, |
| 176 | className="shrink-0 whitespace-nowrap text-[11px]" | 175 | }} |
| 177 | /> | 176 | membership={entry.stack} |
| 178 | </span> | 177 | onSelect={(target) => |
| 179 | <PullRequestMetaLine className="@container/pr-row-meta col-start-1 row-start-2 overflow-hidden text-xs text-muted-foreground/70"> | 178 | onSelect({ ...target, host: entry.host, environmentId: entry.environmentId }) |
| 180 | {matchedElsewhere ? ( | ||
| 181 | <Tooltip> | ||
| 182 | <TooltipTrigger | ||
| 183 | render={ | ||
| 184 | <span className="flex min-w-6 items-center gap-1 overflow-hidden rounded-full border border-border/60 px-1 text-[10px]" /> | ||
| 185 | } | 179 | } |
| 186 | > | 180 | /> |
| 187 | <span className="sr-only">matched in the description</span> | 181 | ) : null} |
| 188 | <SearchIcon aria-hidden className="size-3 shrink-0" /> | 182 | <PullRequestDiffStat |
| 189 | <span aria-hidden className="hidden truncate @xs/pr-row-meta:block"> | 183 | additions={entry.additions} |
| 190 | matched in the description | 184 | deletions={entry.deletions} |
| 191 | </span> | 185 | className="font-mono" |
| 192 | </TooltipTrigger> | 186 | /> |
| 193 | <TooltipPopup side="top">Matched in the description</TooltipPopup> | 187 | </> |
| 194 | </Tooltip> | 188 | } |
| 195 | ) : null} | 189 | metaClassName="@container/pr-row-meta" |
| 196 | <span className="flex shrink-0 items-center gap-1"> | 190 | meta={ |
| 191 | <> | ||
| 192 | {matchedElsewhere ? ( | ||
| 193 | <Tooltip> | ||
| 194 | <TooltipTrigger | ||
| 195 | render={ | ||
| 196 | <span className="flex min-w-6 items-center gap-1 overflow-hidden rounded-full border border-border/60 px-1 text-[10px]" /> | ||
| 197 | } | ||
| 198 | > | ||
| 199 | <span className="sr-only">matched in the description</span> | ||
| 200 | <SearchIcon aria-hidden className="size-3 shrink-0" /> | ||
| 201 | <span aria-hidden className="hidden truncate @xs/pr-row-meta:block"> | ||
| 202 | matched in the description | ||
| 203 | </span> | ||
| 204 | </TooltipTrigger> | ||
| 205 | <TooltipPopup side="top">Matched in the description</TooltipPopup> | ||
| 206 | </Tooltip> | ||
| 207 | ) : null} | ||
| 197 | {showProvider ? ( | 208 | {showProvider ? ( |
| 198 | <Tooltip> | 209 | <Tooltip> |
| 199 | <TooltipTrigger render={<span className="inline-flex shrink-0" />}> | 210 | <TooltipTrigger render={<span className="inline-flex shrink-0" />}> |
| ... | @@ -202,39 +213,20 @@ function PullRequestRowImpl({ | ... | @@ -202,39 +213,20 @@ function PullRequestRowImpl({ |
| 202 | <TooltipPopup>{providerName}</TooltipPopup> | 213 | <TooltipPopup>{providerName}</TooltipPopup> |
| 203 | </Tooltip> | 214 | </Tooltip> |
| 204 | ) : null} | 215 | ) : null} |
| 205 | {/* The number carries the link, here as much as on the detail: a right-click on it | 216 | <PullRequestRowAuthor |
| 206 | copies the pull request's own address rather than opening the editing menu. */} | 217 | actor={entry.author} |
| 207 | <span | 218 | className="min-w-3.5 max-w-40" |
| 208 | onContextMenu={(event) => { | 219 | labelClassName="sr-only @xs/pr-row-meta:not-sr-only @xs/pr-row-meta:truncate" |
| 209 | event.preventDefault(); | 220 | /> |
| 210 | event.stopPropagation(); | 221 | {showProjectTitle ? <span className="truncate">{entry.repository}</span> : null} |
| 211 | void showPullRequestLinkContextMenu({ | 222 | {environmentLabel ? ( |
| 212 | url: entry.url, | 223 | <span className="min-w-0 max-w-32 truncate">{environmentLabel}</span> |
| 213 | openLabel: openOnHostLabel(entry.provider), | 224 | ) : null} |
| 214 | position: { x: event.clientX, y: event.clientY }, | 225 | {entry.labels.length > 0 ? <PullRequestRowLabels labels={entry.labels} /> : null} |
| 215 | }); | 226 | </> |
| 216 | }} | 227 | } |
| 217 | > | 228 | updatedAt={entry.updatedAt} |
| 218 | #{entry.number} | 229 | /> |
| 219 | </span> | ||
| 220 | </span> | ||
| 221 | {showProjectTitle ? <span className="truncate">{entry.repository}</span> : null} | ||
| 222 | {environmentLabel ? ( | ||
| 223 | <span className="min-w-0 max-w-32 truncate">{environmentLabel}</span> | ||
| 224 | ) : null} | ||
| 225 | <PullRequestActorLabel | ||
| 226 | actor={entry.author} | ||
| 227 | className="min-w-4 max-w-40" | ||
| 228 | labelClassName="sr-only @xs/pr-row-meta:not-sr-only @xs/pr-row-meta:truncate" | ||
| 229 | /> | ||
| 230 | {entry.labels.length > 0 ? <PullRequestRowLabels labels={entry.labels} /> : null} | ||
| 231 | </PullRequestMetaLine> | ||
| 232 | <span className="col-start-2 row-start-2 flex items-center justify-self-end gap-3 whitespace-nowrap text-[11px] text-muted-foreground/70 tabular-nums"> | ||
| 233 | <span className="hidden @sm/pr-row:inline"> | ||
| 234 | {formatRelativeTimeLabel(entry.updatedAt)} | ||
| 235 | </span> | ||
| 236 | </span> | ||
| 237 | </span> | ||
| 238 | </button> | 230 | </button> |
| 239 | ); | 231 | ); |
| 240 | } | 232 | } |
apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx+10-18| ... | @@ -30,8 +30,9 @@ import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; | ... | @@ -30,8 +30,9 @@ import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; |
| 30 | import { | 30 | import { |
| 31 | PullRequestActorLabel, | 31 | PullRequestActorLabel, |
| 32 | PullRequestCheckStatusIcon, | 32 | PullRequestCheckStatusIcon, |
| 33 | PullRequestReviewOutcomeBadge, | ||
| 34 | pullRequestCheckStatusLabel, | 33 | pullRequestCheckStatusLabel, |
| 34 | PullRequestLabelChip, | ||
| 35 | PullRequestReviewOutcomeBadge, | ||
| 35 | pullRequestReviewOutcomeLabel, | 36 | pullRequestReviewOutcomeLabel, |
| 36 | pullRequestReviewOutcomeRingClassName, | 37 | pullRequestReviewOutcomeRingClassName, |
| 37 | pullRequestReviewOutcomeStaleLabel, | 38 | pullRequestReviewOutcomeStaleLabel, |
| ... | @@ -56,7 +57,6 @@ import { PullRequestCommentBody } from "./PullRequestCommentBody"; | ... | @@ -56,7 +57,6 @@ import { PullRequestCommentBody } from "./PullRequestCommentBody"; |
| 56 | import { PullRequestMarkdownEditor } from "./PullRequestMarkdownEditor"; | 57 | import { PullRequestMarkdownEditor } from "./PullRequestMarkdownEditor"; |
| 57 | import { PullRequestReactionBar } from "./PullRequestReactions"; | 58 | import { PullRequestReactionBar } from "./PullRequestReactions"; |
| 58 | import { PullRequestConversationGhost } from "./PullRequestGhosts"; | 59 | import { PullRequestConversationGhost } from "./PullRequestGhosts"; |
| 59 | import { pullRequestLabelColor } from "./pullRequestList.logic"; | ||
| 60 | import { sectionCollapseAnchorScrollTop } from "./pullRequestSummaryScroll.logic"; | 60 | import { sectionCollapseAnchorScrollTop } from "./pullRequestSummaryScroll.logic"; |
| 61 | 61 | ||
| 62 | /** One reviewer, however a host happens to have cased their login this time. */ | 62 | /** One reviewer, however a host happens to have cased their login this time. */ |
| ... | @@ -809,22 +809,14 @@ export function PullRequestSummaryTab({ | ... | @@ -809,22 +809,14 @@ export function PullRequestSummaryTab({ |
| 809 | {detail.labels.length === 0 ? ( | 809 | {detail.labels.length === 0 ? ( |
| 810 | <span className="text-muted-foreground">None</span> | 810 | <span className="text-muted-foreground">None</span> |
| 811 | ) : ( | 811 | ) : ( |
| 812 | detail.labels.map((label) => { | 812 | detail.labels.map((label) => ( |
| 813 | const dot = pullRequestLabelColor(label.color); | 813 | <PullRequestLabelChip |
| 814 | return ( | 814 | key={label.name} |
| 815 | <span | 815 | label={label} |
| 816 | key={label.name} | 816 | size="default" |
| 817 | className="inline-flex max-w-48 items-center gap-1.5 rounded-full bg-muted/40 py-0.5 pl-1.5 pr-2 text-xs" | 817 | className="max-w-48" |
| 818 | > | 818 | /> |
| 819 | <span | 819 | )) |
| 820 | aria-hidden | ||
| 821 | className="size-2 shrink-0 rounded-full bg-muted-foreground" | ||
| 822 | {...(dot ? { style: { backgroundColor: dot } } : {})} | ||
| 823 | /> | ||
| 824 | <span className="truncate">{label.name}</span> | ||
| 825 | </span> | ||
| 826 | ); | ||
| 827 | }) | ||
| 828 | )} | 820 | )} |
| 829 | {detail.capabilities.labels === true ? ( | 821 | {detail.capabilities.labels === true ? ( |
| 830 | <PullRequestLabelPicker | 822 | <PullRequestLabelPicker |
apps/web/src/components/pullRequest/ThreadPullRequestsPanel.tsx+123-108| ... | @@ -21,11 +21,16 @@ import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; | ... | @@ -21,11 +21,16 @@ import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; |
| 21 | import { openLinkPullRequestDialog } from "./LinkPullRequestDialog"; | 21 | import { openLinkPullRequestDialog } from "./LinkPullRequestDialog"; |
| 22 | import { pullRequestListLines, type PullRequestListLine } from "./pullRequestListLines"; | 22 | import { pullRequestListLines, type PullRequestListLine } from "./pullRequestListLines"; |
| 23 | import { | 23 | import { |
| 24 | PullRequestActorAvatar, | 24 | PULL_REQUEST_ROW_CLASS, |
| 25 | PullRequestConflictGlyph, | 25 | PULL_REQUEST_ROW_NUMBER_CLASS, |
| 26 | PullRequestRowAuthor, | ||
| 27 | PullRequestRowBranches, | ||
| 28 | PullRequestRowGlyph, | ||
| 29 | PullRequestRowLines, | ||
| 30 | } from "./PullRequestListRow"; | ||
| 31 | import { | ||
| 26 | PullRequestDiffStat, | 32 | PullRequestDiffStat, |
| 27 | PullRequestApprovalGlyph, | 33 | PullRequestReviewDecisionGlyph, |
| 28 | PullRequestStateGlyph, | ||
| 29 | pullRequestChecksStatePresentation, | 34 | pullRequestChecksStatePresentation, |
| 30 | } from "./pullRequestPresentation"; | 35 | } from "./pullRequestPresentation"; |
| 31 | import { PullRequestGlyph } from "./pullRequestIcons"; | 36 | import { PullRequestGlyph } from "./pullRequestIcons"; |
| ... | @@ -72,7 +77,7 @@ function LinkRow({ | ... | @@ -72,7 +77,7 @@ function LinkRow({ |
| 72 | const snapshot = link.snapshot; | 77 | const snapshot = link.snapshot; |
| 73 | return ( | 78 | return ( |
| 74 | <div | 79 | <div |
| 75 | className="group/pr-row flex items-center gap-2 rounded-md py-1 pr-1 hover:bg-accent/60" | 80 | className={cn(PULL_REQUEST_ROW_CLASS, "relative hover:bg-accent/60")} |
| 76 | // Each layer steps in under the one it targets. The step is capped: beyond a few layers | 81 | // Each layer steps in under the one it targets. The step is capped: beyond a few layers |
| 77 | // the indent only says "still in the stack", which the connector line already does, and | 82 | // the indent only says "still in the stack", which the connector line already does, and |
| 78 | // a sixteen-layer stack would otherwise stair-step off the right edge. | 83 | // a sixteen-layer stack would otherwise stair-step off the right edge. |
| ... | @@ -85,122 +90,132 @@ function LinkRow({ | ... | @@ -85,122 +90,132 @@ function LinkRow({ |
| 85 | className="size-4 shrink-0 text-muted-foreground" | 90 | className="size-4 shrink-0 text-muted-foreground" |
| 86 | /> | 91 | /> |
| 87 | ) : ( | 92 | ) : ( |
| 88 | <span className="flex shrink-0 items-center gap-1"> | 93 | <PullRequestRowGlyph |
| 89 | <PullRequestStateGlyph state={snapshot.state} isDraft={snapshot.isDraft} /> | 94 | state={snapshot.state} |
| 90 | <PullRequestConflictGlyph | 95 | isDraft={snapshot.isDraft} |
| 91 | state={snapshot.state} | 96 | mergeability={snapshot.mergeability} |
| 92 | isDraft={snapshot.isDraft} | 97 | baseBranch={snapshot.baseBranch} |
| 93 | baseBranch={snapshot.baseBranch} | 98 | /> |
| 94 | {...(snapshot.mergeability ? { mergeability: snapshot.mergeability } : {})} | ||
| 95 | /> | ||
| 96 | </span> | ||
| 97 | )} | 99 | )} |
| 98 | <a | 100 | <a |
| 99 | href={link.url} | 101 | href={link.url} |
| 100 | onClick={(event) => openPrLink(event, link.url, threadRef)} | 102 | onClick={(event) => openPrLink(event, link.url, threadRef)} |
| 101 | className="min-w-0 flex-1" | 103 | className="flex min-w-0 flex-1" |
| 102 | > | 104 | > |
| 103 | <span className="flex min-w-0 items-center gap-1.5"> | 105 | <PullRequestRowLines |
| 104 | <Tooltip> | 106 | number={ |
| 105 | <TooltipTrigger | ||
| 106 | render={ | ||
| 107 | <span className="shrink-0 font-mono text-xs tabular-nums text-muted-foreground" /> | ||
| 108 | } | ||
| 109 | > | ||
| 110 | #{link.number} | ||
| 111 | </TooltipTrigger> | ||
| 112 | <TooltipPopup> | ||
| 113 | {SOURCE_LABELS[link.source]} · {formatRelativeTimeLabel(link.linkedAt)} | ||
| 114 | </TooltipPopup> | ||
| 115 | </Tooltip> | ||
| 116 | <span className="min-w-0 flex-1 truncate text-sm"> | ||
| 117 | {snapshot?.title ?? link.repository} | ||
| 118 | </span> | ||
| 119 | {/* Match the full PR list: review verdict, checks, then diff counts. | ||
| 120 | Each is absent rather than neutral when the | ||
| 121 | host said nothing, so a row without them reads as unknown, not as fine. */} | ||
| 122 | <span className="ml-auto flex shrink-0 items-center gap-1.5 text-[11px]"> | ||
| 123 | {snapshot?.state === "open" && | ||
| 124 | (snapshot.reviewDecision === "approved" || | ||
| 125 | snapshot.reviewDecision === "changes-requested") ? ( | ||
| 126 | snapshot.reviewDecision === "approved" ? ( | ||
| 127 | <PullRequestApprovalGlyph /> | ||
| 128 | ) : ( | ||
| 129 | <span className="text-amber-600/90 dark:text-amber-400/80">Changes requested</span> | ||
| 130 | ) | ||
| 131 | ) : null} | ||
| 132 | {snapshot?.checksState ? <ChecksGlyph state={snapshot.checksState} /> : null} | ||
| 133 | <PullRequestDiffStat | ||
| 134 | additions={snapshot?.additions ?? 0} | ||
| 135 | deletions={snapshot?.deletions ?? 0} | ||
| 136 | className="font-mono" | ||
| 137 | /> | ||
| 138 | </span> | ||
| 139 | </span> | ||
| 140 | <span className="flex min-w-0 items-center gap-1.5 text-[11px] text-muted-foreground"> | ||
| 141 | {stack ? ( | ||
| 142 | <Tooltip> | 107 | <Tooltip> |
| 143 | <TooltipTrigger | 108 | <TooltipTrigger render={<span className={PULL_REQUEST_ROW_NUMBER_CLASS} />}> |
| 144 | render={ | 109 | #{link.number} |
| 145 | <span className="inline-flex shrink-0 items-center gap-0.5 text-foreground/70" /> | ||
| 146 | } | ||
| 147 | > | ||
| 148 | <PullRequestGlyph.stack aria-hidden className="size-3" /> | ||
| 149 | {stack.size} | ||
| 150 | </TooltipTrigger> | 110 | </TooltipTrigger> |
| 151 | <TooltipPopup> | 111 | <TooltipPopup> |
| 152 | {stack.kind === "native" | 112 | {SOURCE_LABELS[link.source]} · {formatRelativeTimeLabel(link.linkedAt)} |
| 153 | ? `GitHub stack of ${stack.size}: merging a layer lands the ones below it.` | ||
| 154 | : `${stack.size} pull requests chained by base branch.`} | ||
| 155 | </TooltipPopup> | 113 | </TooltipPopup> |
| 156 | </Tooltip> | 114 | </Tooltip> |
| 157 | ) : null} | 115 | } |
| 158 | {snapshot?.author ? ( | 116 | title={snapshot?.title ?? link.repository} |
| 159 | <span className="inline-flex shrink-0 items-center gap-1"> | 117 | signals={ |
| 160 | <PullRequestActorAvatar actor={snapshot.author} className="size-3.5" /> | 118 | snapshot?.state === "open" ? ( |
| 161 | <span className="max-w-28 truncate">{snapshot.author.login}</span> | 119 | <> |
| 162 | </span> | 120 | {snapshot.checksState ? <ChecksGlyph state={snapshot.checksState} /> : null} |
| 163 | ) : null} | 121 | {snapshot.reviewDecision ? ( |
| 164 | <span className="truncate font-mono"> | 122 | <PullRequestReviewDecisionGlyph decision={snapshot.reviewDecision} /> |
| 165 | {snapshot !== null | 123 | ) : null} |
| 166 | ? `${snapshot.headBranch} → ${snapshot.baseBranch}` | 124 | </> |
| 167 | : `${link.host}/${link.repository}`} | 125 | ) : null |
| 168 | </span> | 126 | } |
| 169 | {snapshot?.updatedAt ? ( | 127 | // Match the full PR list: diff counts up top, checks under the lifecycle glyph, the |
| 170 | <span className="ml-auto shrink-0">{formatRelativeTimeLabel(snapshot.updatedAt)}</span> | 128 | // verdict by the author. Each is absent rather than neutral when the host said |
| 171 | ) : null} | 129 | // nothing, so a row without them reads as unknown, not as fine. |
| 172 | </span> | 130 | status={ |
| 173 | </a> | 131 | <PullRequestDiffStat |
| 174 | <Menu> | 132 | additions={snapshot?.additions ?? 0} |
| 175 | <MenuTrigger | 133 | deletions={snapshot?.deletions ?? 0} |
| 176 | render={ | 134 | className="font-mono" |
| 177 | <Button | 135 | /> |
| 178 | variant="ghost" | 136 | } |
| 179 | size="icon-xs" | 137 | meta={ |
| 180 | aria-label={`Actions for #${link.number}`} | 138 | <> |
| 181 | className={cn( | 139 | {stack ? ( |
| 182 | "opacity-0 group-hover/pr-row:opacity-100 data-[popup-open]:opacity-100", | 140 | <Tooltip> |
| 141 | <TooltipTrigger | ||
| 142 | render={ | ||
| 143 | <span className="inline-flex shrink-0 items-center gap-0.5 text-foreground/70" /> | ||
| 144 | } | ||
| 145 | > | ||
| 146 | <PullRequestGlyph.stack aria-hidden className="size-3" /> | ||
| 147 | {stack.size} | ||
| 148 | </TooltipTrigger> | ||
| 149 | <TooltipPopup> | ||
| 150 | {stack.kind === "native" | ||
| 151 | ? `GitHub stack of ${stack.size}: merging a layer lands the ones below it.` | ||
| 152 | : `${stack.size} pull requests chained by base branch.`} | ||
| 153 | </TooltipPopup> | ||
| 154 | </Tooltip> | ||
| 155 | ) : null} | ||
| 156 | {snapshot?.author ? ( | ||
| 157 | <PullRequestRowAuthor | ||
| 158 | actor={snapshot.author} | ||
| 159 | className="shrink-0" | ||
| 160 | labelClassName="max-w-28" | ||
| 161 | /> | ||
| 162 | ) : null} | ||
| 163 | {snapshot !== null ? ( | ||
| 164 | <PullRequestRowBranches head={snapshot.headBranch} base={snapshot.baseBranch} /> | ||
| 165 | ) : ( | ||
| 166 | <span className="truncate font-mono"> | ||
| 167 | {link.host}/{link.repository} | ||
| 168 | </span> | ||
| 183 | )} | 169 | )} |
| 184 | > | 170 | </> |
| 185 | <MoreHorizontalIcon className="size-3.5" /> | ||
| 186 | </Button> | ||
| 187 | } | 171 | } |
| 172 | updatedAt={snapshot?.updatedAt} | ||
| 188 | /> | 173 | /> |
| 189 | <MenuPopup align="end" side="bottom"> | 174 | </a> |
| 190 | <MenuItem onClick={() => void writeTextToClipboard(link.url, "link")}> | 175 | {/* Out of the row's flow, so no row reserves a column for a button only the hovered one |
| 191 | <LinkIcon className="size-3.5" /> | 176 | shows. It sits over the right end of the second line on the row's own hover color, |
| 192 | Copy link | 177 | fading in from the left, so it covers the time and leaves the diff counts alone. */} |
| 193 | </MenuItem> | 178 | <span |
| 194 | <MenuItem onClick={(event) => openPrLink(event, link.url, threadRef)}> | 179 | className={cn( |
| 195 | <ArrowUpRightIcon className="size-3.5" /> | 180 | "absolute right-0 bottom-0.5 flex items-center rounded-r-md bg-background pr-1 pl-5", |
| 196 | Open | 181 | "[mask-image:linear-gradient(to_right,transparent,black_1rem)]", |
| 197 | </MenuItem> | 182 | // Hidden means untouchable too: on a touch screen there is no hover, and an invisible |
| 198 | <MenuItem onClick={() => onUnlink(link)}> | 183 | // layer over the right of the row would otherwise swallow the tap meant for the link. |
| 199 | <PullRequestGlyph.unlink className="size-3.5" /> | 184 | "pointer-events-none opacity-0 group-hover/pr-row:pointer-events-auto group-hover/pr-row:opacity-100", |
| 200 | {link.source === "stack" ? "Dismiss from thread" : "Unlink from thread"} | 185 | "has-[[data-popup-open]]:pointer-events-auto has-[[data-popup-open]]:opacity-100", |
| 201 | </MenuItem> | 186 | "has-[:focus-visible]:pointer-events-auto has-[:focus-visible]:opacity-100", |
| 202 | </MenuPopup> | 187 | )} |
| 203 | </Menu> | 188 | > |
| 189 | <span aria-hidden className="absolute inset-0 bg-accent/60" /> | ||
| 190 | <Menu> | ||
| 191 | <MenuTrigger | ||
| 192 | render={ | ||
| 193 | <Button | ||
| 194 | variant="ghost" | ||
| 195 | size="icon-micro" | ||
| 196 | aria-label={`Actions for #${link.number}`} | ||
| 197 | className="relative" | ||
| 198 | > | ||
| 199 | <MoreHorizontalIcon className="size-3.5" /> | ||
| 200 | </Button> | ||
| 201 | } | ||
| 202 | /> | ||
| 203 | <MenuPopup align="end" side="bottom"> | ||
| 204 | <MenuItem onClick={() => void writeTextToClipboard(link.url, "link")}> | ||
| 205 | <LinkIcon className="size-3.5" /> | ||
| 206 | Copy link | ||
| 207 | </MenuItem> | ||
| 208 | <MenuItem onClick={(event) => openPrLink(event, link.url, threadRef)}> | ||
| 209 | <ArrowUpRightIcon className="size-3.5" /> | ||
| 210 | Open | ||
| 211 | </MenuItem> | ||
| 212 | <MenuItem onClick={() => onUnlink(link)}> | ||
| 213 | <PullRequestGlyph.unlink className="size-3.5" /> | ||
| 214 | {link.source === "stack" ? "Dismiss from thread" : "Unlink from thread"} | ||
| 215 | </MenuItem> | ||
| 216 | </MenuPopup> | ||
| 217 | </Menu> | ||
| 218 | </span> | ||
| 204 | </div> | 219 | </div> |
| 205 | ); | 220 | ); |
| 206 | } | 221 | } |
apps/web/src/components/pullRequest/pullRequestChecks.test.tsx+9-2| ... | @@ -52,13 +52,20 @@ describe("pullRequestChecksState", () => { | ... | @@ -52,13 +52,20 @@ describe("pullRequestChecksState", () => { |
| 52 | }); | 52 | }); |
| 53 | }); | 53 | }); |
| 54 | 54 | ||
| 55 | /** Every element of the tree the row returned, so a nested indicator can be looked for. */ | 55 | /** |
| 56 | * Every element of the tree the row returned, so a nested indicator can be looked for. The row | ||
| 57 | * hands its slots to the shared row lines as props rather than children, so every prop that | ||
| 58 | * holds an element is walked too. | ||
| 59 | */ | ||
| 56 | function flatten(node: ReactNode): ReadonlyArray<ReturnType<typeof Object>> { | 60 | function flatten(node: ReactNode): ReadonlyArray<ReturnType<typeof Object>> { |
| 57 | const found: unknown[] = []; | 61 | const found: unknown[] = []; |
| 58 | for (const child of Children.toArray(node)) { | 62 | for (const child of Children.toArray(node)) { |
| 59 | if (!isValidElement(child)) continue; | 63 | if (!isValidElement(child)) continue; |
| 60 | found.push(child); | 64 | found.push(child); |
| 61 | found.push(...flatten((child.props as { readonly children?: ReactNode }).children)); | 65 | for (const value of Object.values(child.props as Record<string, unknown>)) { |
| 66 | if (isValidElement(value)) found.push(...flatten(value)); | ||
| 67 | else if (Array.isArray(value)) found.push(...flatten(value.filter(isValidElement))); | ||
| 68 | } | ||
| 62 | } | 69 | } |
| 63 | return found as ReadonlyArray<ReturnType<typeof Object>>; | 70 | return found as ReadonlyArray<ReturnType<typeof Object>>; |
| 64 | } | 71 | } |
apps/web/src/components/pullRequest/pullRequestPresentation.tsx+83-8| ... | @@ -4,7 +4,9 @@ import type { | ... | @@ -4,7 +4,9 @@ import type { |
| 4 | PullRequestCheck, | 4 | PullRequestCheck, |
| 5 | PullRequestCheckStatus, | 5 | PullRequestCheckStatus, |
| 6 | PullRequestChecksState, | 6 | PullRequestChecksState, |
| 7 | PullRequestLabel, | ||
| 7 | PullRequestMergeability, | 8 | PullRequestMergeability, |
| 9 | PullRequestReviewDecision, | ||
| 8 | PullRequestState, | 10 | PullRequestState, |
| 9 | } from "@t3tools/contracts"; | 11 | } from "@t3tools/contracts"; |
| 10 | import { | 12 | import { |
| ... | @@ -13,14 +15,17 @@ import { | ... | @@ -13,14 +15,17 @@ import { |
| 13 | CircleDotIcon, | 15 | CircleDotIcon, |
| 14 | CircleXIcon, | 16 | CircleXIcon, |
| 15 | UserCheckIcon, | 17 | UserCheckIcon, |
| 18 | UserRoundIcon, | ||
| 19 | UserRoundXIcon, | ||
| 16 | } from "lucide-react"; | 20 | } from "lucide-react"; |
| 17 | import { Children, isValidElement, type ReactNode, useState } from "react"; | 21 | import { Children, type CSSProperties, isValidElement, type ReactNode, useState } from "react"; |
| 18 | 22 | ||
| 19 | import { cn } from "~/lib/utils"; | 23 | import { cn } from "~/lib/utils"; |
| 20 | 24 | ||
| 21 | import { Badge } from "../ui/badge"; | 25 | import { Badge } from "../ui/badge"; |
| 22 | import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; | 26 | import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; |
| 23 | import type { PullRequestReviewOutcome } from "./pullRequestDetail.logic"; | 27 | import type { PullRequestReviewOutcome } from "./pullRequestDetail.logic"; |
| 28 | import { pullRequestLabelColor } from "./pullRequestList.logic"; | ||
| 24 | import { | 29 | import { |
| 25 | PULL_REQUEST_STATE_PRESENTATION, | 30 | PULL_REQUEST_STATE_PRESENTATION, |
| 26 | PullRequestGlyph, | 31 | PullRequestGlyph, |
| ... | @@ -28,17 +33,87 @@ import { | ... | @@ -28,17 +33,87 @@ import { |
| 28 | type PullRequestGlyphIcon, | 33 | type PullRequestGlyphIcon, |
| 29 | } from "./pullRequestIcons"; | 34 | } from "./pullRequestIcons"; |
| 30 | 35 | ||
| 31 | export function PullRequestApprovalGlyph() { | 36 | /** |
| 37 | * A host label as a flat tinted tag in the label's own color: a wash of it behind, the name | ||
| 38 | * in a mix of it and the theme foreground. The mix leans to the foreground because hosts hand | ||
| 39 | * out any color at all: at 30% of the label on light and 45% on dark, white, black and | ||
| 40 | * GitHub's pale yellows all clear 4.5:1 on their wash, selected row included, and | ||
| 41 | * saturated colors sit well above. | ||
| 42 | * A label with no usable color falls back to the muted tag. Children ride after the name, | ||
| 43 | * for an overflow count. The height is pinned so a labeled row is as tall as one without. | ||
| 44 | */ | ||
| 45 | export function PullRequestLabelChip({ | ||
| 46 | label, | ||
| 47 | size = "sm", | ||
| 48 | className, | ||
| 49 | children, | ||
| 50 | }: { | ||
| 51 | label: Pick<PullRequestLabel, "name" | "color">; | ||
| 52 | size?: "sm" | "default"; | ||
| 53 | className?: string; | ||
| 54 | children?: ReactNode; | ||
| 55 | }) { | ||
| 56 | const color = pullRequestLabelColor(label.color); | ||
| 57 | return ( | ||
| 58 | <Badge | ||
| 59 | size={size} | ||
| 60 | variant="secondary" | ||
| 61 | className={cn( | ||
| 62 | "min-w-0 max-w-40 shrink justify-start gap-1 rounded-full px-2", | ||
| 63 | size === "sm" && "h-4 text-[.625rem]", | ||
| 64 | color && | ||
| 65 | "bg-[color-mix(in_srgb,var(--label)_8%,transparent)] text-[color-mix(in_srgb,var(--label)_30%,var(--color-foreground))] dark:bg-[color-mix(in_srgb,var(--label)_12%,transparent)] dark:text-[color-mix(in_srgb,var(--label)_45%,var(--color-foreground))]", | ||
| 66 | className, | ||
| 67 | )} | ||
| 68 | {...(color ? { style: { "--label": color } as CSSProperties } : {})} | ||
| 69 | > | ||
| 70 | <span className="truncate">{label.name}</span> | ||
| 71 | {children} | ||
| 72 | </Badge> | ||
| 73 | ); | ||
| 74 | } | ||
| 75 | |||
| 76 | /** | ||
| 77 | * The review verdict as one glyph beside the checks glyph, so a row answers both "does it | ||
| 78 | * build" and "did someone say yes" in the same spot. "Awaiting review" is only drawn when the | ||
| 79 | * host reports it, which on GitHub means the branch rules require a review nobody has given. | ||
| 80 | */ | ||
| 81 | function reviewDecisionPresentation(decision: PullRequestReviewDecision) { | ||
| 82 | switch (decision) { | ||
| 83 | case "approved": | ||
| 84 | return { | ||
| 85 | Icon: UserCheckIcon, | ||
| 86 | label: "Approved", | ||
| 87 | toneClassName: CHECK_STATUS_PRESENTATION.success.toneClassName, | ||
| 88 | }; | ||
| 89 | case "changes-requested": | ||
| 90 | return { | ||
| 91 | Icon: UserRoundXIcon, | ||
| 92 | label: "Changes requested", | ||
| 93 | toneClassName: "text-amber-600/90 dark:text-amber-400/80", | ||
| 94 | }; | ||
| 95 | case "review-required": | ||
| 96 | return { | ||
| 97 | Icon: UserRoundIcon, | ||
| 98 | label: "Awaiting review", | ||
| 99 | toneClassName: "text-muted-foreground/60", | ||
| 100 | }; | ||
| 101 | } | ||
| 102 | } | ||
| 103 | |||
| 104 | export function PullRequestReviewDecisionGlyph({ | ||
| 105 | decision, | ||
| 106 | }: { | ||
| 107 | decision: PullRequestReviewDecision; | ||
| 108 | }) { | ||
| 109 | const presentation = reviewDecisionPresentation(decision); | ||
| 32 | return ( | 110 | return ( |
| 33 | <Tooltip> | 111 | <Tooltip> |
| 34 | <TooltipTrigger render={<span className="inline-flex shrink-0" />}> | 112 | <TooltipTrigger render={<span className="inline-flex shrink-0" />}> |
| 35 | <UserCheckIcon | 113 | <presentation.Icon aria-hidden className={cn("size-3.5", presentation.toneClassName)} /> |
| 36 | aria-hidden | 114 | <span className="sr-only">{presentation.label}</span> |
| 37 | className={cn("size-3.5", CHECK_STATUS_PRESENTATION.success.toneClassName)} | ||
| 38 | /> | ||
| 39 | <span className="sr-only">Approved</span> | ||
| 40 | </TooltipTrigger> | 115 | </TooltipTrigger> |
| 41 | <TooltipPopup>Approved</TooltipPopup> | 116 | <TooltipPopup>{presentation.label}</TooltipPopup> |
| 42 | </Tooltip> | 117 | </Tooltip> |
| 43 | ); | 118 | ); |
| 44 | } | 119 | } |
apps/web/src/routes/_chat.pull-requests.tsx+30-5| ... | @@ -24,11 +24,13 @@ import { | ... | @@ -24,11 +24,13 @@ import { |
| 24 | LayersIcon, | 24 | LayersIcon, |
| 25 | ListChecksIcon, | 25 | ListChecksIcon, |
| 26 | PenLineIcon, | 26 | PenLineIcon, |
| 27 | UsersIcon, | ||
| 27 | Plug2Icon, | 28 | Plug2Icon, |
| 28 | Maximize2Icon, | 29 | Maximize2Icon, |
| 29 | Minimize2Icon, | 30 | Minimize2Icon, |
| 30 | SearchIcon, | 31 | SearchIcon, |
| 31 | UserLockIcon, | 32 | UserLockIcon, |
| 33 | type LucideIcon, | ||
| 32 | } from "lucide-react"; | 34 | } from "lucide-react"; |
| 33 | import { | 35 | import { |
| 34 | useCallback, | 36 | useCallback, |
| ... | @@ -146,6 +148,7 @@ import { | ... | @@ -146,6 +148,7 @@ import { |
| 146 | } from "../state/pullRequests"; | 148 | } from "../state/pullRequests"; |
| 147 | import { useAtomCommand } from "../state/use-atom-command"; | 149 | import { useAtomCommand } from "../state/use-atom-command"; |
| 148 | import { cn } from "~/lib/utils"; | 150 | import { cn } from "~/lib/utils"; |
| 151 | import { Separator } from "~/components/ui/separator"; | ||
| 149 | import { primaryServerKeybindingsAtom } from "~/state/server"; | 152 | import { primaryServerKeybindingsAtom } from "~/state/server"; |
| 150 | import { getSourceControlPresentationForKind } from "~/sourceControlPresentation"; | 153 | import { getSourceControlPresentationForKind } from "~/sourceControlPresentation"; |
| 151 | import { PullRequestGlyph } from "~/components/pullRequest/pullRequestIcons"; | 154 | import { PullRequestGlyph } from "~/components/pullRequest/pullRequestIcons"; |
| ... | @@ -188,6 +191,32 @@ export interface PullRequestsSearch extends PullRequestListPreferences { | ... | @@ -188,6 +191,32 @@ export interface PullRequestsSearch extends PullRequestListPreferences { |
| 188 | readonly selectedEnvironmentId?: EnvironmentId; | 191 | readonly selectedEnvironmentId?: EnvironmentId; |
| 189 | } | 192 | } |
| 190 | 193 | ||
| 194 | /** | ||
| 195 | * A group reads like the sidebar's shelves: its glyph, its name, how many, then a rule out | ||
| 196 | * to the edge. The glyph is the one the involvement filter uses for the same idea. | ||
| 197 | */ | ||
| 198 | const GROUP_ICONS: Record<string, LucideIcon> = { | ||
| 199 | authored: PenLineIcon, | ||
| 200 | reviewRequested: EyeIcon, | ||
| 201 | others: UsersIcon, | ||
| 202 | }; | ||
| 203 | |||
| 204 | function PullRequestGroupHeader({ | ||
| 205 | group, | ||
| 206 | }: { | ||
| 207 | group: { key: string; label: string; entries: ReadonlyArray<unknown> }; | ||
| 208 | }) { | ||
| 209 | const Icon = GROUP_ICONS[group.key] ?? LayersIcon; | ||
| 210 | return ( | ||
| 211 | <div className="flex items-center gap-2 px-3 pb-1 text-xs font-medium text-muted-foreground/70"> | ||
| 212 | <Icon aria-hidden className="size-3.5 shrink-0" /> | ||
| 213 | <h2 className="shrink-0">{group.label}</h2> | ||
| 214 | <span className="shrink-0 tabular-nums text-muted-foreground/50">{group.entries.length}</span> | ||
| 215 | <Separator className="min-w-2 flex-1 bg-border/60" /> | ||
| 216 | </div> | ||
| 217 | ); | ||
| 218 | } | ||
| 219 | |||
| 191 | // The state filters wear the same glyphs the rows do, so the two read as one vocabulary. | 220 | // The state filters wear the same glyphs the rows do, so the two read as one vocabulary. |
| 192 | const INVOLVEMENT_TABS = [ | 221 | const INVOLVEMENT_TABS = [ |
| 193 | { value: "all", label: "All", Icon: LayersIcon }, | 222 | { value: "all", label: "All", Icon: LayersIcon }, |
| ... | @@ -1659,11 +1688,7 @@ function PullRequestsRouteView() { | ... | @@ -1659,11 +1688,7 @@ function PullRequestsRouteView() { |
| 1659 | <div className="space-y-3"> | 1688 | <div className="space-y-3"> |
| 1660 | {displayGroups.map((group) => ( | 1689 | {displayGroups.map((group) => ( |
| 1661 | <div key={group.key} className="space-y-0.5"> | 1690 | <div key={group.key} className="space-y-0.5"> |
| 1662 | {group.label ? ( | 1691 | {group.label ? <PullRequestGroupHeader group={group} /> : null} |
| 1663 | <h2 className="px-3 pb-0.5 text-xs font-medium text-muted-foreground/70"> | ||
| 1664 | {group.label} | ||
| 1665 | </h2> | ||
| 1666 | ) : null} | ||
| 1667 | {group.entries.map((entry) => { | 1692 | {group.entries.map((entry) => { |
| 1668 | const entryKey = pullRequestEntryKey(entry); | 1693 | const entryKey = pullRequestEntryKey(entry); |
| 1669 | return ( | 1694 | return ( |