mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-21 00:05:49 +02:00
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 <noreply@paperclip.ing>
This commit is contained in:
1 parent
43b005b704
commit
ccfd52bd24
6 files changed
+68
-5
No files matched your search
@@ -112,8 +112,26 @@ describe("ExternalObjectPill", () => {
|
||||
expect(html).toContain("PR 241 - Merged");
|
||||
expect(html).not.toContain("acme/web#241</span>");
|
||||
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</span>");
|
||||
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(
|
||||
<ExternalObjectPill
|
||||
object={{
|
||||
providerKey: "url",
|
||||
objectType: "link",
|
||||
statusCategory: "unknown",
|
||||
liveness: "unknown",
|
||||
displayTitle: null,
|
||||
url: "https://example.com/",
|
||||
}}
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(html).toContain('aria-label="URL — Not yet resolved"');
|
||||
expect(html).not.toContain("URL link");
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -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}` : ""}`;
|
||||
|
||||
|
||||
@@ -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");
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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"));
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in new issue
Block a user