diff --git a/ui/src/components/ExternalObjectPill.test.tsx b/ui/src/components/ExternalObjectPill.test.tsx index 4d5201f7738..5899b116552 100644 --- a/ui/src/components/ExternalObjectPill.test.tsx +++ b/ui/src/components/ExternalObjectPill.test.tsx @@ -100,7 +100,7 @@ describe("ExternalObjectPill", () => { displayKey: "Github Pull Request", iconKey: "github", statusCategory: "succeeded", - statusIconKey: "git-merge", + statusIconKey: null, liveness: "fresh", statusLabel: "Merged", displayTitle: "acme/web#241: Add rich object presentation metadata", @@ -109,7 +109,8 @@ describe("ExternalObjectPill", () => { />, ); expect(html).toContain("Merged"); - expect(html).toContain("acme/web#241"); + expect(html).toContain("PR 241 - Merged"); + expect(html).not.toContain("acme/web#241"); expect(html).toContain("text-violet-600"); expect(html).not.toContain("Github Pull Request"); expect(html).toContain('aria-label="GitHub pull request — Merged: acme/web#241: Add rich object presentation metadata"'); diff --git a/ui/src/components/ExternalObjectPill.tsx b/ui/src/components/ExternalObjectPill.tsx index 7e760e686df..99617be12cf 100644 --- a/ui/src/components/ExternalObjectPill.tsx +++ b/ui/src/components/ExternalObjectPill.tsx @@ -38,25 +38,43 @@ function githubObjectLabel(url: string | null | undefined): string | null { if (parsed.hostname !== "github.com") return null; const [, owner, repo, kind, number] = parsed.pathname.split("/"); if (!owner || !repo || !number) return null; - if (kind !== "pull" && kind !== "issues") return null; - return `${owner}/${repo}#${number}`; + if (kind === "pull") return `PR ${number}`; + if (kind === "issues") return `Issue ${number}`; + return null; } catch { return null; } } -function externalObjectValueLabel(object: ExternalObjectPillData, fallback: string): string { +function externalObjectValueLabel( + object: ExternalObjectPillData, + fallback: string, + statusLabel: string, +): string { const githubLabel = object.providerKey === "github" ? githubObjectLabel(object.url) : null; - return githubLabel ?? object.displayTitle?.trim() ?? fallback; + const base = githubLabel ?? object.displayTitle?.trim() ?? fallback; + return statusLabel ? `${base} - ${statusLabel}` : base; } -function externalObjectPillTone(object: ExternalObjectPillData): string { - if (object.statusIconKey === "git-merge") { +function isMergedExternalObject(object: ExternalObjectPillData, statusLabel: string): boolean { + return object.statusIconKey === "git-merge" || statusLabel.toLowerCase() === "merged"; +} + +function externalObjectPillTone(object: ExternalObjectPillData, statusLabel: string): string { + if (isMergedExternalObject(object, statusLabel)) { return "text-violet-600 border-violet-600 dark:text-violet-400 dark:border-violet-400"; } return externalObjectStatusIcon[object.statusCategory] ?? externalObjectStatusIconDefault; } +function externalObjectStatusIconKey( + object: ExternalObjectPillData, + statusLabel: string, +): string | null | undefined { + if (isMergedExternalObject(object, statusLabel)) return object.statusIconKey ?? "git-merge"; + return object.statusIconKey; +} + interface ExternalObjectPillProps { object: ExternalObjectPillData; /** Optional external mention count (renders as `×N` superscript when > 1). */ @@ -71,6 +89,11 @@ interface ExternalObjectPillProps { * non-interactive contexts like the property panel). Defaults to false. */ inert?: boolean; + /** + * Hide the provider icon when the surrounding UI already names the provider. + * The status icon still renders as the single state glyph inside the pill. + */ + showProviderIcon?: boolean; } /** @@ -84,14 +107,16 @@ export function ExternalObjectPill({ className, children, inert, + showProviderIcon = true, }: ExternalObjectPillProps) { - const tone = externalObjectPillTone(object); const overlay = externalObjectLivenessOverlay[object.liveness] ?? ""; const providerLabel = externalObjectProviderLabel(object.providerKey); const typeLabel = externalObjectTypeLabel(object.objectType); const displayKey = object.displayKey?.trim() || `${providerLabel} ${typeLabel}`; - const valueLabel = externalObjectValueLabel(object, displayKey); const statusLabel = object.statusLabel ?? externalObjectCategoryLabel(object.statusCategory); + const tone = externalObjectPillTone(object, statusLabel); + const valueLabel = externalObjectValueLabel(object, displayKey, statusLabel); + const statusIconKey = externalObjectStatusIconKey(object, statusLabel); const livenessLabel = externalObjectLivenessLabel(object.liveness); const ProviderIcon = externalObjectIconForKey(object.iconKey); const ariaLabel = `${providerLabel} ${typeLabel} — ${statusLabel}${ @@ -115,16 +140,13 @@ export function ExternalObjectPill({ : object.displayTitle ?? displayKey; const labelText = children ?? ( <> - - - {statusLabel} - + {valueLabel} ); @@ -133,17 +155,9 @@ export function ExternalObjectPill({ ) : null; const innerContent = ( <> - {ProviderIcon ? ( + {showProviderIcon && ProviderIcon ? (