From ccfd52bd244e3eba0907014051ba9e19cbe71ed1 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Fri, 26 Jun 2026 13:45:21 -0500 Subject: [PATCH] Fix generic URL rich object labels (#8662) Reviewed by CTO for PAP-12039. Scope is limited to external object URL/link label fallback and focused UI regression coverage; CI and Greptile are green. Co-Authored-By: Paperclip --- ui/src/components/ExternalObjectPill.test.tsx | 22 +++++++++++++++-- ui/src/components/ExternalObjectPill.tsx | 6 +++-- ui/src/components/IssueProperties.test.tsx | 24 +++++++++++++++++++ ui/src/components/IssueProperties.tsx | 3 ++- ui/src/lib/external-objects.test.ts | 7 ++++++ ui/src/lib/external-objects.ts | 11 +++++++++ 6 files changed, 68 insertions(+), 5 deletions(-) diff --git a/ui/src/components/ExternalObjectPill.test.tsx b/ui/src/components/ExternalObjectPill.test.tsx index 5899b116552..8ab1d5771ed 100644 --- a/ui/src/components/ExternalObjectPill.test.tsx +++ b/ui/src/components/ExternalObjectPill.test.tsx @@ -112,8 +112,26 @@ describe("ExternalObjectPill", () => { 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"'); + expect(html).not.toContain(">Github Pull Request"); + expect(html).toContain('aria-label="Github Pull Request — Merged: acme/web#241: Add rich object presentation metadata"'); + }); + + it("labels generic URL link objects as URL instead of URL link", () => { + const html = renderToStaticMarkup( + , + ); + + expect(html).toContain('aria-label="URL — Not yet resolved"'); + expect(html).not.toContain("URL link"); }); }); diff --git a/ui/src/components/ExternalObjectPill.tsx b/ui/src/components/ExternalObjectPill.tsx index 99617be12cf..55970a5847b 100644 --- a/ui/src/components/ExternalObjectPill.tsx +++ b/ui/src/components/ExternalObjectPill.tsx @@ -11,6 +11,7 @@ import { } from "../lib/status-colors"; import { externalObjectCategoryLabel, + externalObjectDisplayLabel, externalObjectLivenessLabel, externalObjectIconForKey, externalObjectProviderLabel, @@ -112,14 +113,15 @@ export function ExternalObjectPill({ const overlay = externalObjectLivenessOverlay[object.liveness] ?? ""; const providerLabel = externalObjectProviderLabel(object.providerKey); const typeLabel = externalObjectTypeLabel(object.objectType); - const displayKey = object.displayKey?.trim() || `${providerLabel} ${typeLabel}`; + const displayKey = externalObjectDisplayLabel(object.providerKey, object.objectType, 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}${ + const ariaKey = displayKey; + const ariaLabel = `${ariaKey} — ${statusLabel}${ object.liveness === "fresh" || object.liveness === "unknown" ? "" : ` (${livenessLabel})` }${object.displayTitle ? `: ${object.displayTitle}` : ""}`; diff --git a/ui/src/components/IssueProperties.test.tsx b/ui/src/components/IssueProperties.test.tsx index f9814b29098..9144bf8fa14 100644 --- a/ui/src/components/IssueProperties.test.tsx +++ b/ui/src/components/IssueProperties.test.tsx @@ -1967,12 +1967,36 @@ describe("IssueProperties", () => { sourceLabels: ["Comment"], }, }, + { + mentionCount: 1, + sourceLabels: ["Comment"], + pill: { + providerKey: "url", + objectType: "link", + displayKey: null, + iconKey: null, + statusCategory: "unknown", + statusIconKey: null, + statusLabel: null, + liveness: "unknown", + displayTitle: "https://example.com/release-notes", + url: "https://example.com/release-notes", + }, + group: { + object: null, + mentions: [], + mentionCount: 1, + sourceLabels: ["Comment"], + }, + }, ], }); await flush(); expect(container.textContent).toContain("Github Pull Request"); expect(container.textContent).toContain("Github Issue"); + expect(container.textContent).toContain("URL"); + expect(container.textContent).not.toContain("URL link"); expect(container.textContent).toContain("PR 241 - Merged"); expect(container.textContent).toContain("Merged"); expect(container.textContent).toContain("Open"); diff --git a/ui/src/components/IssueProperties.tsx b/ui/src/components/IssueProperties.tsx index b6bc31ea1f9..2524076483f 100644 --- a/ui/src/components/IssueProperties.tsx +++ b/ui/src/components/IssueProperties.tsx @@ -40,6 +40,7 @@ import { ExternalObjectStatusIcon } from "./ExternalObjectStatusIcon"; import type { IssueExternalObjectGroup } from "../hooks/useIssueExternalObjects"; import { externalObjectCategoryLabel, + externalObjectDisplayLabel, externalObjectIconForKey, externalObjectProviderLabel, externalObjectToneSeverity, @@ -214,7 +215,7 @@ function externalObjectRowDisplayKey(group: IssueExternalObjectGroup): string { if (pill.objectType === "pull_request") return "Github Pull Request"; if (pill.objectType === "issue") return "Github Issue"; } - return `${externalObjectProviderLabel(pill.providerKey)} ${externalObjectTypeLabel(pill.objectType)}`; + return externalObjectDisplayLabel(pill.providerKey, pill.objectType); } function externalObjectRowLabel(group: IssueExternalObjectGroup): React.ReactNode { diff --git a/ui/src/lib/external-objects.test.ts b/ui/src/lib/external-objects.test.ts index 3032d07056c..ab267c134c1 100644 --- a/ui/src/lib/external-objects.test.ts +++ b/ui/src/lib/external-objects.test.ts @@ -8,6 +8,7 @@ import { import { dominantExternalObjectTone, externalObjectCategoryLabel, + externalObjectDisplayLabel, externalObjectDominantCount, externalObjectFallbackTone, externalObjectIconForCategory, @@ -107,6 +108,12 @@ describe("external-objects helpers", () => { expect(externalObjectTypeLabel(null)).toBe("object"); }); + it("labels generic URL link objects as URL", () => { + expect(externalObjectDisplayLabel("url", "link")).toBe("URL"); + expect(externalObjectDisplayLabel("url", "link", "Canonical URL")).toBe("Canonical URL"); + expect(externalObjectDisplayLabel("github", "pull_request")).toBe("GitHub pull request"); + }); + it("orders tones from danger down to muted", () => { expect(externalObjectToneSeverity("danger")).toBeGreaterThan(externalObjectToneSeverity("warning")); expect(externalObjectToneSeverity("warning")).toBeGreaterThan(externalObjectToneSeverity("info")); diff --git a/ui/src/lib/external-objects.ts b/ui/src/lib/external-objects.ts index 89b6667c338..7a6dcbc342a 100644 --- a/ui/src/lib/external-objects.ts +++ b/ui/src/lib/external-objects.ts @@ -178,6 +178,17 @@ export function externalObjectTypeLabel(objectType: string | null | undefined): return OBJECT_TYPE_LABELS[objectType] ?? objectType.replace(/_/g, " "); } +export function externalObjectDisplayLabel( + providerKey: string | null | undefined, + objectType: string | null | undefined, + displayKey?: string | null, +): string { + const trimmedDisplayKey = displayKey?.trim(); + if (trimmedDisplayKey) return trimmedDisplayKey; + if (providerKey === "url" && objectType === "link") return "URL"; + return `${externalObjectProviderLabel(providerKey)} ${externalObjectTypeLabel(objectType)}`; +} + /** * Sort summary items by severity-first ordering: danger → warning → info → * success → muted/neutral. Within a tone, items keep their incoming order so