From 29b1a65d1e6800a79782ab3efa07b84116797a6f Mon Sep 17 00:00:00 2001 From: "Vyctor H. Brzezowski" Date: Tue, 18 Aug 2026 07:59:37 -0300 Subject: [PATCH] improve(ui): compact GitHub links in chat (#125746) * improve(ui): compact GitHub links in chat * fix(ui): refine GitHub link pills --- ui/src/components/github-link-target.ts | 33 ++++++++++------- ui/src/components/markdown-links.test.ts | 27 +++++++++----- ui/src/components/markdown-parser.ts | 12 +++---- ui/src/e2e/github-link-hovercard.e2e.test.ts | 36 +++++++------------ ...t-github-link-presentation.browser.test.ts | 6 ++-- ui/src/styles/chat/text.css | 7 ---- 6 files changed, 59 insertions(+), 62 deletions(-) diff --git a/ui/src/components/github-link-target.ts b/ui/src/components/github-link-target.ts index 9a725e85518b..d5666340eaf7 100644 --- a/ui/src/components/github-link-target.ts +++ b/ui/src/components/github-link-target.ts @@ -22,7 +22,7 @@ function decodePathSegment(value: string): string | null { } } -export function parseGitHubItemPath(url: URL): GitHubItemTarget | null { +function parseGitHubItemPath(url: URL): GitHubItemTarget | null { const segments = url.pathname.split("/").filter(Boolean); const owner = decodePathSegment(segments[0] ?? ""); const repo = decodePathSegment(segments[1] ?? ""); @@ -52,18 +52,27 @@ export function parseGitHubLinkTarget(href: string): GitHubLinkTarget | null { return target ? { ...target, href: url.href } : null; } -export function formatGitHubItemReference(target: GitHubItemTarget): string { - return `${target.owner}/${target.repo}#${target.number}`; -} - -// Compaction to `owner/repo#N` is only safe when the URL names the generic -// item root: any trailing path segment (/files, /commits, an issue comment -// anchor) or query/fragment is a more specific destination than the compact -// label communicates, even though `parseGitHubItemPath` still resolves an -// identity for it (hovercards and navigation need that deep identity intact). -export function isGitHubItemRootPath(url: URL): boolean { +export function formatGitHubLinkLabel(url: URL): string { const segments = url.pathname.split("/").filter(Boolean); - return segments.length === 4 && !url.search && !url.hash; + const item = parseGitHubItemPath(url); + if (item && segments.length === 4 && !url.search && !url.hash) { + return `#${item.number}`; + } + if (item) { + return url.href; + } + if (segments.length === 2) { + return segments.map((segment) => decodePathSegment(segment) ?? segment).join("/"); + } + if (segments[2] === "blob" && segments.length > 4) { + const filename = decodePathSegment(segments.at(-1) ?? ""); + if (filename) { + return filename; + } + } + const fallbackSegments = segments.length > 2 ? segments.slice(2) : segments; + const path = fallbackSegments.map((segment) => decodePathSegment(segment) ?? segment); + return ["github.com", ...path].join("/"); } export function gitHubProfileUrl(login: string): string { diff --git a/ui/src/components/markdown-links.test.ts b/ui/src/components/markdown-links.test.ts index c87050512758..e1b498f43b4b 100644 --- a/ui/src/components/markdown-links.test.ts +++ b/ui/src/components/markdown-links.test.ts @@ -174,7 +174,7 @@ describe("toSanitizedMarkdownHtml links", () => { it("links http:// URLs", () => { const html = toSanitizedMarkdownHtml("Visit http://github.com/openclaw"); expect(html).toBe( - '

Visit http://github.com/openclaw

\n', + '

Visit github.com/openclaw

\n', ); }); @@ -596,11 +596,14 @@ describe("toSanitizedMarkdownHtml links", () => { describe("github link marks", () => { it.each([ - ["bare autolink", "https://github.com/openclaw/openclaw/pull/3434", "openclaw/openclaw#3434"], + ["bare pull request", "https://github.com/openclaw/openclaw/pull/3434", "#3434"], + ["bare issue", "https://github.com/openclaw/openclaw/issues/3435", "#3435"], + ["repository", "https://github.com/openclaw/openclaw", "openclaw/openclaw"], + ["repository file", "https://github.com/blader/humanizer/blob/main/SKILL.md", "SKILL.md"], [ - "bare issue autolink", - "https://github.com/openclaw/openclaw/issues/3435", - "openclaw/openclaw#3435", + "other path", + "https://github.com/openclaw/openclaw/actions/runs/123", + "github.com/actions/runs/123", ], ["issue shorthand", "[#3434](https://github.com/openclaw/openclaw/pull/3434)", "#3434"], ["labelled link", "[the fix](https://github.com/openclaw/openclaw/pull/3434)", "the fix"], @@ -621,12 +624,20 @@ describe("toSanitizedMarkdownHtml links", () => { ), ); const link = fragment.querySelector("a"); - expect(link?.textContent).toBe( - "a-very-long-organization-name/a-very-long-repository-name#3434", - ); + expect(link?.textContent).toBe("#3434"); expect(link?.classList.contains("markdown-bare-url")).toBe(true); }); + it("keeps the specific destination addressable after shortening its label", () => { + const input = "https://github.com/blader/humanizer/blob/main/SKILL.md"; + const fragment = htmlFragment(toSanitizedMarkdownHtml(input)); + const link = fragment.querySelector("a"); + expect(link?.classList.contains("markdown-github-link")).toBe(true); + expect(link?.textContent).toBe("SKILL.md"); + expect(link?.getAttribute("href")).toBe(input); + expect(link?.getAttribute("title")).toBe(input); + }); + it.each([ ["a files-tab path", "https://github.com/openclaw/openclaw/pull/3434/files"], ["a commits path", "https://github.com/openclaw/openclaw/pull/3434/commits"], diff --git a/ui/src/components/markdown-parser.ts b/ui/src/components/markdown-parser.ts index 4525bc8e28db..790c82d77211 100644 --- a/ui/src/components/markdown-parser.ts +++ b/ui/src/components/markdown-parser.ts @@ -3,11 +3,7 @@ import markdownItTaskLists from "markdown-it-task-lists"; import type Token from "markdown-it/lib/token.mjs"; import { t } from "../i18n/index.ts"; import { fileKindForPath, shortestFileLabels } from "./file-kind.ts"; -import { - formatGitHubItemReference, - isGitHubItemRootPath, - parseGitHubItemPath, -} from "./github-link-target.ts"; +import { formatGitHubLinkLabel } from "./github-link-target.ts"; import { installAssistantTranscriptRoleImageRenderer, installAssistantTranscriptRoleMarkdown, @@ -520,7 +516,6 @@ export function createMarkdownParser(): MarkdownIt { const generatedUrlLabel = open.markup === "linkify" || open.markup === "autolink"; const host = url.hostname.toLowerCase(); const githubLink = host === "github.com" || host === "www.github.com"; - const itemTarget = githubLink ? parseGitHubItemPath(url) : null; if (generatedUrlLabel) { open.attrJoin("class", BARE_URL_CLASS); } @@ -542,8 +537,9 @@ export function createMarkdownParser(): MarkdownIt { break; } } - if (generatedUrlLabel && itemTarget && labelToken && isGitHubItemRootPath(url)) { - labelToken.content = formatGitHubItemReference(itemTarget); + if (generatedUrlLabel && labelToken) { + labelToken.content = formatGitHubLinkLabel(url); + open.attrSet("title", href ?? url.href); } } } diff --git a/ui/src/e2e/github-link-hovercard.e2e.test.ts b/ui/src/e2e/github-link-hovercard.e2e.test.ts index ce69494d24cc..cb615c2d0490 100644 --- a/ui/src/e2e/github-link-hovercard.e2e.test.ts +++ b/ui/src/e2e/github-link-hovercard.e2e.test.ts @@ -102,7 +102,7 @@ async function openPullPreviewPage(): Promise<{ }); await page.goto(`${server.baseUrl}chat`); - const pullLink = page.getByRole("link", { name: "openclaw/openclaw#99816" }); + const pullLink = page.locator('a.markdown-github-link[href$="/pull/99816"]'); const card = page.locator(".github-link-hovercard"); await pullLink.waitFor({ state: "visible" }); return { card, page, pullLink }; @@ -194,6 +194,7 @@ describeControlUiE2e("GitHub link hover cards", () => { "then https://github.com/openclaw/openclaw/issues/99815.", "A [missing item](https://github.com/openclaw/openclaw/issues/999999) stays usable.", "The [repository](https://github.com/openclaw/openclaw) has no item preview.", + "The skill lives at https://github.com/blader/humanizer/blob/main/SKILL.md.", "Styling notes live in [the docs](https://docs.openclaw.ai/web/control-ui).", ].join(" "), }, @@ -227,36 +228,23 @@ describeControlUiE2e("GitHub link hover cards", () => { await expect.poll(() => page.locator("html").getAttribute("data-theme-mode")).toBe("light"); } - const longLink = page.getByRole("link", { - name: "a-very-long-organization-name/a-very-long-repository-name#99817", - }); - await page.setViewportSize({ height: 800, width: 360 }); - expect(await longLink.evaluate((element) => getComputedStyle(element).lineBreak)).toBe( - "anywhere", - ); - const longMessageBox = await longLink - .locator("xpath=ancestor::*[contains(@class, 'chat-text')]") - .boundingBox(); - const longLinkBox = await longLink.boundingBox(); - expect(longMessageBox).not.toBeNull(); - expect(longLinkBox).not.toBeNull(); - expect(longLinkBox!.x).toBeGreaterThanOrEqual(longMessageBox!.x); - expect(longLinkBox!.x + longLinkBox!.width).toBeLessThanOrEqual( - longMessageBox!.x + longMessageBox!.width, - ); - await page.setViewportSize({ height: 800, width: 1180 }); + await expect + .poll(() => page.getByRole("link", { name: "#99817" }).getAttribute("href")) + .toBe( + "https://github.com/a-very-long-organization-name/a-very-long-repository-name/issues/99817", + ); + await expect + .poll(() => page.getByRole("link", { name: "SKILL.md" }).getAttribute("href")) + .toBe("https://github.com/blader/humanizer/blob/main/SKILL.md"); - const pullLink = page.getByRole("link", { name: "openclaw/openclaw#99816" }); + const pullLink = page.locator('a.markdown-github-link[href$="/pull/99816"]'); - // The mark carries the link signal at rest, so the underline only returns on - // hover. Non-GitHub links keep the base underline, which keeps the rule scoped. const decorationLine = (link: Locator) => link.evaluate((element) => getComputedStyle(element).textDecorationLine); expect(await decorationLine(pullLink)).toBe("none"); expect(await decorationLine(page.getByRole("link", { name: "the docs" }))).toBe("underline"); await pullLink.hover(); - await expect.poll(() => decorationLine(pullLink)).toBe("underline"); const card = page.locator(".github-link-hovercard"); await expectText(card, "Merged"); await expectText(card, "openclaw/openclaw #99816"); @@ -272,7 +260,7 @@ describeControlUiE2e("GitHub link hover cards", () => { expect(pullBox!.x + pullBox!.width).toBeLessThanOrEqual(1180); expect(pullBox!.y + pullBox!.height).toBeLessThanOrEqual(800); - const issueLink = page.getByRole("link", { name: "openclaw/openclaw#99815" }); + const issueLink = page.locator('a.markdown-github-link[href$="/issues/99815"]'); await issueLink.hover(); await expectText(card, "Keep hover previews compact"); await expectText(card, "octocat"); diff --git a/ui/src/styles/chat-github-link-presentation.browser.test.ts b/ui/src/styles/chat-github-link-presentation.browser.test.ts index f08b51c9afff..5c419a7f5061 100644 --- a/ui/src/styles/chat-github-link-presentation.browser.test.ts +++ b/ui/src/styles/chat-github-link-presentation.browser.test.ts @@ -22,20 +22,20 @@ function readChatCss(): string { } // The three shapes the parser can produce, all carrying the same mark: -// a bare item URL whose label it rewrites to owner/repo#number, a bare URL to +// a bare item URL whose label it rewrites to #number, a compact fallback for // any other GitHub path, and an authored label. Only the first two carry // markdown-bare-url, so the sweep covers both wrap regimes. const LINK_FORMS = [ { className: "markdown-bare-url markdown-github-link", id: "human-ref", - label: "openclaw/openclaw#123309", + label: "#123309", lead: "then follow-up tracked in ", }, { className: "markdown-bare-url markdown-github-link", id: "bare-url", - label: "https://github.com/openclaw/openclaw/blob/main/ui/src/styles/chat/text.css#L254", + label: "text.css", lead: "then the owning rule lives at ", }, { diff --git a/ui/src/styles/chat/text.css b/ui/src/styles/chat/text.css index 15fb44bae398..0f0679d7c0ef 100644 --- a/ui/src/styles/chat/text.css +++ b/ui/src/styles/chat/text.css @@ -529,9 +529,6 @@ -webkit-mask: var(--github-mark) center / contain no-repeat; } -/* The glyph already marks these as links; a resting underline doubles the - signal and clutters dense URL text. Hover restores it as the affordance, - like the file-link convention below. */ .chat-text :where(a.markdown-github-link) { /* The mark is painted out of flow with its space reserved here, so nothing inside the anchor precedes the label: an in-flow atomic mark carries a soft @@ -543,10 +540,6 @@ text-decoration: none; } -.chat-text :where(a.markdown-github-link:hover) { - text-decoration: underline; -} - /* The inherited `overflow-wrap: anywhere` breaks a word only once every other option is gone, so a long URL is pushed whole onto the next line, leaving the line it should have filled ragged. Unconditional break points make the URL