diff --git a/docs/design-system.md b/docs/design-system.md index 6a98d29a05..15c38c0044 100644 --- a/docs/design-system.md +++ b/docs/design-system.md @@ -87,7 +87,15 @@ Plugins define integration-specific semantic mappings in their own styles, using shared roles where shared improvements should flow through. The shared system owns palette values and generic paint roles, not integration-specific status names. Categorical purple (`--text-categorical-purple` paired with -`--surface-categorical-purple`) stays independent of customizable app accent. +`--surface-categorical-purple`, and `--icon-categorical-purple` for state icons) +stays independent of customizable app accent. Inside your own bubble the host +hands state icons their `-own` pair (`--icon-subtle-own`, `--icon-success-own`, +`--icon-categorical-purple-own`), which is the bubble's link ink at Buzzin’, +where the solid accent fill clears neither grey nor any state hue. It does so +through `--message-icon-subtle`, `--message-icon-success` and +`--message-icon-categorical-purple`, which a state icon reads with the shared +role as its fallback. Neutral insets in the bubble (code, tables, a revealed +spoiler) drop them, as they drop own mention pairing. GitHub owns its `--github-*` status/diff mappings and browser coverage of the resolved text/fill pairs in both themes, including shared-role updates and merged diff --git a/docs/plugin-architecture.md b/docs/plugin-architecture.md index 57cf83bc34..c82b1eabc4 100644 --- a/docs/plugin-architecture.md +++ b/docs/plugin-architecture.md @@ -1399,7 +1399,14 @@ does not fetch metadata or infer a service from names in paths or query strings. Its `order` is a function: recognised services and Buzz entities claim the default band, the globe case the catch-all band, so a renderer dedicated to one website wins it at the default. -It does not fetch titles. Messages currently recognize +It does not fetch titles. The bundled GitHub plugin contributes a pull request +renderer at a lower `order` for github.com pull request URLs: the same label +with the pull request's state (open, draft, merged, closed) as a coloured icon, +read with the person's `gh` sign-in or the public API and kept current while +shown. Repositories, issues and commits, and every pull request link when that +plugin is disabled, keep the Links presentation. If the renderer fails, a pull +request link shows as a plain link. +Messages currently recognize credential-free HTTP(S) and supported Buzz links. Markdown labels preserve their formatting, escaped pasted wrappers are normalized outside code, and paired `<…>` autolink wrappers are hidden in display. Buzz links use known channel names with corresponding icons, falling back @@ -1414,7 +1421,11 @@ link to the same destination, routed through the same host navigation handler. O session's bounded thread reader; closing disposes it. The reader supplies verified message content, author and timestamp and preserves edit/deletion/access handling. Previewing never acknowledges reading or switches community. The host offers a -resolved label to the bundled presentation via `LinkLabelContext`; no author API +resolved label to the bundled presentation via `LinkLabelContext`, and whether +the link is live or a composer decoration or preview via `LinkInteractiveContext`, +so a renderer reads nothing for a draft. A renderer that shows a status, such as +a pull request's state, describes the host anchor via `LinkDescriptionContext`, +so the link keeps its label or URL as its name; no author API contract or relay protocol changes are required. Known channel references and unambiguous signed person/agent mentions share the inline hover styling. Names in ordinary prose never create notification intent or establish an identity. diff --git a/scripts/design-system/check-contrast.mjs b/scripts/design-system/check-contrast.mjs index d12ba4b0e2..8d2bee6299 100644 --- a/scripts/design-system/check-contrast.mjs +++ b/scripts/design-system/check-contrast.mjs @@ -239,6 +239,19 @@ const BOUNDARY_SURFACES = [ "--surface-inset", "--surface-popover", ]; +/** + * State icons that also sit inside message bubbles: a pull request link's + * glyph and the sidebar's. Each is measured against the received bubble, and + * its `-own` pair against the own bubble, per accent family and level below. + * On the high level the own bubble is the solid accent step, which neither + * grey nor any one hue clears on every family, so there the `-own` pair is the + * bubble's link ink. + */ +const MESSAGE_ICON_ROLES = [ + "--icon-subtle", + "--icon-success", + "--icon-categorical-purple", +]; const BOUNDARY_ROLES = [ // Dots and insertion lines in the prominent color stand alone on a surface. "--affordance-prominent-mark", @@ -254,6 +267,7 @@ const BOUNDARY_ROLES = [ "--chart-series-2", "--chart-series-3", "--chart-series-4", + ...MESSAGE_ICON_ROLES, ]; const TINT_PAIRS = [ @@ -543,6 +557,20 @@ for (const family of accentFamilies) { `${family}/${level}/${mode}: --border-accent on ${surfaceRole} — ${ratio === null ? "unresolved color" : `${ratio.toFixed(3)}:1`}, needs 3:1`, ); } + for (const baseRole of MESSAGE_ICON_ROLES) { + for (const [role, surfaceRole] of [ + [baseRole, "--surface-message"], + [`${baseRole}-own`, "--surface-message-own"], + ]) { + const icon = resolve(map, role); + const surface = resolve(map, surfaceRole); + const ratio = icon && surface ? wcagRatio(icon, surface) : null; + if (ratio === null || ratio < 3) + boundaryFailures.push( + `${family}/${level}/${mode}: ${role} on ${surfaceRole} — ${ratio === null ? "unresolved color" : `${ratio.toFixed(3)}:1`}, needs 3:1`, + ); + } + } } } } diff --git a/src/app/pages.integration.test.mjs b/src/app/pages.integration.test.mjs index 0a7aefa715..37ba38979e 100644 --- a/src/app/pages.integration.test.mjs +++ b/src/app/pages.integration.test.mjs @@ -239,18 +239,19 @@ test("the app runtime exposes ready bundled pages and removes them on disable", ["buzz.agents"], ); assert.equal(services.conversation.inline.snapshot().length, 1); + const linkPlugins = () => + services.conversation.links + .snapshot() + .map((link) => link.pluginId) + .sort(); await vi.waitFor(() => - assert.equal(services.conversation.links.snapshot().length, 1), - ); - assert.equal( - services.conversation.links.snapshot()[0].pluginId, - "buzz.links", + assert.deepEqual(linkPlugins(), ["buzz.github", "buzz.links"]), ); await services.plugins.change("disable", "buzz.links"); - assert.equal(services.conversation.links.snapshot().length, 0); + assert.deepEqual(linkPlugins(), ["buzz.github"]); await services.plugins.change("enable", "buzz.links"); await vi.waitFor(() => - assert.equal(services.conversation.links.snapshot().length, 1), + assert.deepEqual(linkPlugins(), ["buzz.github", "buzz.links"]), ); const tools = services.conversation.tools.snapshot(); const inline = services.conversation.inline.snapshot(); diff --git a/src/bundled/github/GitHub.module.css b/src/bundled/github/GitHub.module.css index 667e23a4fc..4cbc07fd0d 100644 --- a/src/bundled/github/GitHub.module.css +++ b/src/bundled/github/GitHub.module.css @@ -393,10 +393,24 @@ justify-content: center; width: var(--space-5); flex: 0 0 var(--space-5); +} +/* The sidebar's icon and a message link's icon take one colour per state, as + the panel's chip above does; a link's text stays a link. Open and merged + use the icon-tier roles: the chip's step-12 text inks are indistinguishable + from a bubble's own ink. Draft, closed and not yet read are quiet: closed is + not red here. Only the sidebar shows an open one's checks and conflicts. + The sidebar's quiet icon is the row's subtle ink, which a hovered or + selected row rebinds; a message's icons take the pair the bubble hands + them, if any. */ +.soloPull { color: var(--text-subtle); } -.soloPull[data-tone="open"] { - color: var(--text-success); +.pullState { + color: var(--message-icon-subtle, var(--icon-subtle)); +} +.soloPull[data-tone="open"], +.pullState[data-pr-state="open"] { + color: var(--message-icon-success, var(--icon-success)); } .soloPull[data-tone="failure"] { color: var(--text-danger); @@ -404,8 +418,9 @@ .soloPull[data-tone="pending"] { color: var(--text-warning); } -.soloPull[data-tone="merged"] { - color: var(--text-categorical-purple); +.soloPull[data-tone="merged"], +.pullState[data-pr-state="merged"] { + color: var(--message-icon-categorical-purple, var(--icon-categorical-purple)); } .soloLink { display: flex; diff --git a/src/bundled/github/GitHub.module.test.ts b/src/bundled/github/GitHub.module.test.ts new file mode 100644 index 0000000000..bbb4505f26 --- /dev/null +++ b/src/bundled/github/GitHub.module.test.ts @@ -0,0 +1,80 @@ +import { readFileSync } from "node:fs"; +import { expect, it } from "vitest"; + +// A custom property inherits its computed value, so a role aliased at the +// root does not follow a host that rebinds what it points at. No DOM emulator +// resolves var(), so these read the stylesheets the state icons depend on. +const rules = (path: string) => + [ + ...readFileSync(path, "utf8") + .replace(/\/\*[\s\S]*?\*\//g, "") + .matchAll(/([^{}]+)\{([^{}]*)\}/g), + ].map(([, selector = "", body = ""]) => ({ + selectors: selector.split(",").map((part) => part.trim()), + declarations: new Map( + [...body.matchAll(/([\w-]+):\s*([^;]+);/g)].map( + ([, name = "", value = ""]) => [name, value.trim()], + ), + ), + })); +const github = rules("src/bundled/github/GitHub.module.css"); +const read = (value = "") => + [...value.matchAll(/var\((--[\w-]+)/g)].map(([, name]) => name); + +it("paints the sidebar's quiet icon with the ink a hovered or selected row rebinds", () => { + const rebound = new Set( + rules("src/shared/design-system/styles/components.css") + .filter(({ selectors }) => + selectors.some((selector) => selector.startsWith(".navigation-item")), + ) + .flatMap(({ declarations }) => [...declarations.keys()]) + .filter((name) => name.startsWith("--")), + ); + expect(rebound).toContain("--text-subtle"); + const quiet = github.find( + ({ selectors, declarations }) => + selectors.includes(".soloPull") && declarations.has("color"), + ); + const roles = read(quiet?.declarations.get("color")); + expect(roles.length).toBeGreaterThan(0); + for (const role of roles) expect(rebound).toContain(role); +}); + +it("keeps the own bubble's state icon pair out of its neutral insets", () => { + const messages = rules("src/features/messages/Messages.module.css"); + // What the own bubble hands the state icons inside it. + const handed = [ + ...(messages.find(({ selectors }) => + selectors.includes(".messageBubble[data-own]"), + )?.declarations ?? []), + ].flatMap(([name, value]) => { + const shared = /^var\((--icon-[\w-]+)-own\)$/.exec(value)?.[1]; + return shared ? [{ name, shared }] : []; + }); + expect(handed.map(({ shared }) => shared).sort()).toEqual([ + "--icon-categorical-purple", + "--icon-subtle", + "--icon-success", + ]); + // Code, tables and a revealed spoiler, which drop own mention pairing too. + const insets = messages.filter( + ({ declarations }) => declarations.get("--mention-text") === "initial", + ); + expect(insets.length).toBeGreaterThanOrEqual(2); + for (const { selectors, declarations } of insets) + for (const { name } of handed) + expect(declarations.get(name), `${name} in ${selectors.join(", ")}`).toBe( + "initial", + ); + // There, a message link's icon falls back to the shared role. + for (const { name, shared } of handed) + expect( + github.some(({ selectors, declarations }) => + selectors.some((selector) => selector.startsWith(".pullState")) + ? declarations.get("color")?.replace(/\s+/g, "") === + `var(${name},var(${shared}))` + : false, + ), + `.pullState reads ${name} with ${shared} as its fallback`, + ).toBe(true); +}); diff --git a/src/bundled/github/PullLink.test.tsx b/src/bundled/github/PullLink.test.tsx new file mode 100644 index 0000000000..0edeb1aae4 --- /dev/null +++ b/src/bundled/github/PullLink.test.tsx @@ -0,0 +1,280 @@ +// @vitest-environment jsdom +import "@testing-library/jest-dom/vitest"; +import { act, cleanup, render, screen, waitFor } from "@testing-library/react"; +import { renderToStaticMarkup } from "react-dom/server"; +import { afterEach, expect, it, vi } from "vitest"; +import type { Contribution } from "../../plugins/contributions"; +import type { LinkRenderer } from "../../features/conversation/contracts"; +import { + LinkDescriptionContext, + LinkInteractiveContext, + LinkLabelContext, +} from "../../features/conversation/LinkLabelContext"; +import { + MessageLink, + resolveLink, +} from "../../features/conversation/MessageLink"; +import { ToastProvider } from "../../shared/design-system/ui/Toast"; +import inlineStyles from "../../shared/InlineReference.module.css"; +import { apply as applyLinks } from "../links/index"; +import { apply } from "./index"; +import { PullLink } from "./PullLink"; +import { stateIcons } from "./pull-state-view"; +import type { PullState, PullStates } from "./pull-states"; + +afterEach(() => { + cleanup(); + vi.unstubAllGlobals(); +}); + +const pull = "https://github.com/block/buzz-app/pull/7"; + +/** A store the test moves by hand. */ +function fakeStates(initial?: PullState | "failed") { + let value = initial; + const listeners = new Set<() => void>(); + const unwatch = vi.fn(); + return { + watch: vi.fn((_url: string, listener: () => void) => { + listeners.add(listener); + return () => { + listeners.delete(listener); + unwatch(); + }; + }), + get: () => value, + unwatch, + set(next: PullState | "failed" | undefined) { + value = next; + for (const listener of listeners) listener(); + }, + } satisfies PullStates & Record; +} +const glyph = (Icon: (typeof stateIcons)[keyof typeof stateIcons]) => + renderToStaticMarkup( +