mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
fix(ui): humanize bare GitHub item links (#122285)
This commit is contained in:
committed by
GitHub
parent
84c7d45f15
commit
65f4320147
@@ -5,6 +5,7 @@ import type { ControlUiGitHubPreview } from "../../../src/gateway/control-ui-con
|
||||
import type { GatewayBrowserClient } from "../api/gateway.ts";
|
||||
import { i18n, t } from "../i18n/index.ts";
|
||||
import { formatRelativeTimestamp } from "../lib/format.ts";
|
||||
import { parseGitHubItemPath, type GitHubItemTarget } from "./github-link-target.ts";
|
||||
|
||||
const GITHUB_HOST = "github.com";
|
||||
const OPEN_DELAY_MS = 250;
|
||||
@@ -14,14 +15,8 @@ const CACHE_LIMIT = 100;
|
||||
const VIEWPORT_PADDING = 12;
|
||||
const CARD_GAP = 10;
|
||||
|
||||
type GitHubLinkKind = "issue" | "pull";
|
||||
|
||||
type GitHubLinkTarget = {
|
||||
type GitHubLinkTarget = GitHubItemTarget & {
|
||||
href: string;
|
||||
kind: GitHubLinkKind;
|
||||
number: number;
|
||||
owner: string;
|
||||
repo: string;
|
||||
};
|
||||
|
||||
type GitHubPreview = GitHubLinkTarget & ControlUiGitHubPreview;
|
||||
@@ -59,15 +54,6 @@ function optionalNumber(record: Record<string, unknown>, key: string): number |
|
||||
return typeof value === "number" && Number.isFinite(value) ? value : undefined;
|
||||
}
|
||||
|
||||
function decodePathSegment(value: string): string | null {
|
||||
try {
|
||||
const decoded = decodeURIComponent(value).trim();
|
||||
return decoded && decoded !== "." && decoded !== ".." ? decoded : null;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
function parseGitHubIssueOrPullRequestLink(href: string): GitHubLinkTarget | null {
|
||||
let url: URL;
|
||||
try {
|
||||
@@ -81,19 +67,8 @@ function parseGitHubIssueOrPullRequestLink(href: string): GitHubLinkTarget | nul
|
||||
if (url.username || url.password || (url.port && url.port !== "443")) {
|
||||
return null;
|
||||
}
|
||||
const segments = url.pathname.split("/").filter(Boolean);
|
||||
const owner = decodePathSegment(segments[0] ?? "");
|
||||
const repo = decodePathSegment(segments[1] ?? "");
|
||||
const surface = segments[2];
|
||||
const numberText = segments[3] ?? "";
|
||||
if (!owner || !repo || !/^[1-9]\d{0,9}$/.test(numberText)) {
|
||||
return null;
|
||||
}
|
||||
const kind = surface === "issues" ? "issue" : surface === "pull" ? "pull" : null;
|
||||
if (!kind) {
|
||||
return null;
|
||||
}
|
||||
return { href: url.href, kind, number: Number(numberText), owner, repo };
|
||||
const target = parseGitHubItemPath(url);
|
||||
return target ? { ...target, href: url.href } : null;
|
||||
}
|
||||
|
||||
export function isGitHubPullRequestLink(href: string): boolean {
|
||||
|
||||
@@ -0,0 +1,32 @@
|
||||
export type GitHubItemTarget = {
|
||||
kind: "issue" | "pull";
|
||||
number: number;
|
||||
owner: string;
|
||||
repo: string;
|
||||
};
|
||||
|
||||
function decodePathSegment(value: string): string | null {
|
||||
try {
|
||||
const decoded = decodeURIComponent(value).trim();
|
||||
return decoded && decoded !== "." && decoded !== ".." ? decoded : null;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
export function parseGitHubItemPath(url: URL): GitHubItemTarget | null {
|
||||
const segments = url.pathname.split("/").filter(Boolean);
|
||||
const owner = decodePathSegment(segments[0] ?? "");
|
||||
const repo = decodePathSegment(segments[1] ?? "");
|
||||
const surface = segments[2];
|
||||
const numberText = segments[3] ?? "";
|
||||
if (!owner || !repo || !/^[1-9]\d{0,9}$/.test(numberText)) {
|
||||
return null;
|
||||
}
|
||||
const kind = surface === "issues" ? "issue" : surface === "pull" ? "pull" : null;
|
||||
return kind ? { kind, number: Number(numberText), owner, repo } : null;
|
||||
}
|
||||
|
||||
export function formatGitHubItemReference(target: GitHubItemTarget): string {
|
||||
return `${target.owner}/${target.repo}#${target.number}`;
|
||||
}
|
||||
@@ -478,10 +478,11 @@ describe("toSanitizedMarkdownHtml links", () => {
|
||||
|
||||
describe("github link marks", () => {
|
||||
it.each([
|
||||
["bare autolink", "https://github.com/openclaw/openclaw/pull/3434", "openclaw/openclaw#3434"],
|
||||
[
|
||||
"bare autolink",
|
||||
"https://github.com/openclaw/openclaw/pull/3434",
|
||||
"https://github.com/openclaw/openclaw/pull/3434",
|
||||
"bare issue autolink",
|
||||
"https://github.com/openclaw/openclaw/issues/3435",
|
||||
"openclaw/openclaw#3435",
|
||||
],
|
||||
["issue shorthand", "[#3434](https://github.com/openclaw/openclaw/pull/3434)", "#3434"],
|
||||
["labelled link", "[the fix](https://github.com/openclaw/openclaw/pull/3434)", "the fix"],
|
||||
@@ -492,11 +493,22 @@ describe("toSanitizedMarkdownHtml links", () => {
|
||||
const fragment = htmlFragment(toSanitizedMarkdownHtml(input));
|
||||
const link = fragment.querySelector<HTMLAnchorElement>("a");
|
||||
expect(link?.classList.contains("markdown-github-link")).toBe(true);
|
||||
// The mark is CSS-only: the anchor keeps its authored text so copied text
|
||||
// and screen-reader output stay unchanged.
|
||||
expect(link?.textContent).toBe(expectedText);
|
||||
});
|
||||
|
||||
it("keeps long generated item references breakable after compaction", () => {
|
||||
const fragment = htmlFragment(
|
||||
toSanitizedMarkdownHtml(
|
||||
"https://github.com/a-very-long-organization-name/a-very-long-repository-name/issues/3434",
|
||||
),
|
||||
);
|
||||
const link = fragment.querySelector<HTMLAnchorElement>("a");
|
||||
expect(link?.textContent).toBe(
|
||||
"a-very-long-organization-name/a-very-long-repository-name#3434",
|
||||
);
|
||||
expect(link?.classList.contains("markdown-bare-url")).toBe(true);
|
||||
});
|
||||
|
||||
it.each([
|
||||
["non-github host", "[docs](https://example.com/openclaw)"],
|
||||
["lookalike host", "[docs](https://notgithub.com/openclaw)"],
|
||||
|
||||
@@ -3,6 +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, parseGitHubItemPath } from "./github-link-target.ts";
|
||||
import {
|
||||
installAssistantTranscriptRoleImageRenderer,
|
||||
installAssistantTranscriptRoleMarkdown,
|
||||
@@ -473,13 +474,17 @@ export function createMarkdownParser(): MarkdownIt {
|
||||
if (!url) {
|
||||
continue;
|
||||
}
|
||||
if (open.markup === "linkify" || open.markup === "autolink") {
|
||||
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);
|
||||
}
|
||||
const host = url.hostname.toLowerCase();
|
||||
if (host !== "github.com" && host !== "www.github.com") {
|
||||
if (!githubLink) {
|
||||
continue;
|
||||
}
|
||||
let labelToken: Token | null = null;
|
||||
for (let cursor = index + 1; cursor < children.length; cursor++) {
|
||||
const token = children[cursor];
|
||||
if (!token || token.type === "link_close") {
|
||||
@@ -490,9 +495,13 @@ export function createMarkdownParser(): MarkdownIt {
|
||||
token.content.trim() !== ""
|
||||
) {
|
||||
open.attrJoin("class", GITHUB_LINK_CLASS);
|
||||
labelToken = token;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (generatedUrlLabel && itemTarget && labelToken) {
|
||||
labelToken.content = formatGitHubItemReference(itemTarget);
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
// Control UI tests cover GitHub link hover card behavior.
|
||||
import { mkdir } from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
import { chromium, type Browser, type BrowserContext, type Locator } from "playwright";
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest";
|
||||
import {
|
||||
@@ -119,8 +121,8 @@ describeControlUiE2e("GitHub link hover cards", () => {
|
||||
{
|
||||
type: "text",
|
||||
text: [
|
||||
"Review [#99816](https://github.com/openclaw/openclaw/pull/99816),",
|
||||
"then [#99815](https://github.com/openclaw/openclaw/issues/99815).",
|
||||
"Review https://github.com/openclaw/openclaw/pull/99816,",
|
||||
"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.",
|
||||
"Styling notes live in [the docs](https://docs.openclaw.ai/web/control-ui).",
|
||||
@@ -130,11 +132,52 @@ describeControlUiE2e("GitHub link hover cards", () => {
|
||||
role: "assistant",
|
||||
timestamp: Date.now(),
|
||||
},
|
||||
{
|
||||
content: [
|
||||
{
|
||||
type: "text",
|
||||
text: "Narrow reference https://github.com/a-very-long-organization-name/a-very-long-repository-name/issues/99817",
|
||||
},
|
||||
],
|
||||
role: "assistant",
|
||||
timestamp: Date.now(),
|
||||
},
|
||||
],
|
||||
});
|
||||
await page.goto(`${server.baseUrl}chat`);
|
||||
|
||||
const pullLink = page.getByRole("link", { name: "#99816" });
|
||||
const message = page.locator(".chat-text").filter({ hasText: "Review" });
|
||||
const artifactDir = process.env.OPENCLAW_UI_E2E_ARTIFACT_DIR?.trim();
|
||||
if (artifactDir) {
|
||||
await mkdir(artifactDir, { recursive: true });
|
||||
await message.screenshot({ path: path.join(artifactDir, "github-references-light.png") });
|
||||
await page.emulateMedia({ colorScheme: "dark" });
|
||||
await expect.poll(() => page.locator("html").getAttribute("data-theme-mode")).toBe("dark");
|
||||
await message.screenshot({ path: path.join(artifactDir, "github-references-dark.png") });
|
||||
await page.emulateMedia({ colorScheme: "light" });
|
||||
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 });
|
||||
|
||||
const pullLink = page.getByRole("link", { name: "openclaw/openclaw#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.
|
||||
@@ -160,7 +203,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: "#99815" });
|
||||
const issueLink = page.getByRole("link", { name: "openclaw/openclaw#99815" });
|
||||
await issueLink.hover();
|
||||
await expectText(card, "Keep hover previews compact");
|
||||
await expectText(card, "octocat");
|
||||
@@ -175,7 +218,7 @@ describeControlUiE2e("GitHub link hover cards", () => {
|
||||
expect((await gateway.getRequests("controlUi.githubPreview")).length).toBe(2);
|
||||
|
||||
await page.mouse.move(1, 1);
|
||||
await page.getByRole("link", { name: "repository" }).hover();
|
||||
await page.getByRole("link", { exact: true, name: "repository" }).hover();
|
||||
await page.clock.runFor(300);
|
||||
await expect.poll(() => card.count()).toBe(0);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user