From c05b85c4212f672c8d8b3fa41f9e025ed74da418 Mon Sep 17 00:00:00 2001 From: morgmart <98432065+morgmart@users.noreply.github.com> Date: Sun, 6 Sep 2026 15:20:42 -0700 Subject: [PATCH] desktop-new: tile address model and shared InlineTile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements the foundation the composer sits on: a reference is an address (kind + identity), never the letters of a name. One shared InlineTile serves both the composer and the conversation, mounted in the editor through a React node view. - shared/tiles/address.ts: kind + id, canonical buzz:// form, parse/format. - shared/tiles/faceResolver.ts: resolves the displayed face from an address at render time, so a rename never edits anyone's draft or enters undo history. Community-scoped cache with an explicit reset. - shared/ui/InlineTile.tsx: the one component, person/agent/channel kinds. - features/composer/tileNode.ts: the document node. Stores only the address. - New semantic tokens: --space-tile-inset, --space-tile-gap, --radius-tile. - Specimen page at /design/components/inline-tile covering kinds, in-sentence use, read-only, unresolved, truncation, and activation. Findings while building, all now covered by tests: - A React node view REPAIRS a caret boundary. With static markup, Home before a leading tile landed after it; with the node view both Home and ArrowLeft land correctly. Only the pointer click on a leading tile's left edge still fails — now the single known caret gap, still marked test.fail(). - useSyncExternalStore needs a reference-stable snapshot. Building a fresh unresolved face per call was an infinite render loop that unmounted the whole editor. Unresolved faces are cached like resolved ones. - An unresolved tile must not announce an abbreviated identity to a screen reader. TileFace now carries , and the accessible name says "Unresolved person" rather than reading the stand-in aloud. Falsifiability checked by breaking each protection in turn: selectable:false, the address text projection, and the community reset each turn tests red. Signed-off-by: morgmart <98432065+morgmart@users.noreply.github.com> --- .../src/features/composer-spike/tileNode.ts | 93 --------- .../composer-spike/ui/ComposerSpikePage.tsx | 93 +++++---- desktop-new/src/features/composer/tileNode.ts | 101 ++++++++++ .../src/features/composer/ui/TileNodeView.tsx | 32 ++++ .../design-system/ui/componentSpecimens.tsx | 99 ++++++++++ desktop-new/src/shared/styles/globals.css | 1 + desktop-new/src/shared/styles/tiles.css | 119 ++++++++++++ desktop-new/src/shared/styles/tokens.css | 9 + desktop-new/src/shared/tiles/address.test.ts | 54 ++++++ desktop-new/src/shared/tiles/address.ts | 63 +++++++ .../src/shared/tiles/faceResolver.test.ts | 143 ++++++++++++++ desktop-new/src/shared/tiles/faceResolver.ts | 153 +++++++++++++++ desktop-new/src/shared/ui/InlineTile.tsx | 128 +++++++++++++ desktop-new/src/shared/ui/registry.ts | 10 + desktop-new/tests/composer-spike.spec.ts | 178 ++++++++++++++++-- 15 files changed, 1117 insertions(+), 159 deletions(-) delete mode 100644 desktop-new/src/features/composer-spike/tileNode.ts create mode 100644 desktop-new/src/features/composer/tileNode.ts create mode 100644 desktop-new/src/features/composer/ui/TileNodeView.tsx create mode 100644 desktop-new/src/shared/styles/tiles.css create mode 100644 desktop-new/src/shared/tiles/address.test.ts create mode 100644 desktop-new/src/shared/tiles/address.ts create mode 100644 desktop-new/src/shared/tiles/faceResolver.test.ts create mode 100644 desktop-new/src/shared/tiles/faceResolver.ts create mode 100644 desktop-new/src/shared/ui/InlineTile.tsx diff --git a/desktop-new/src/features/composer-spike/tileNode.ts b/desktop-new/src/features/composer-spike/tileNode.ts deleted file mode 100644 index 56125bb9e4..0000000000 --- a/desktop-new/src/features/composer-spike/tileNode.ts +++ /dev/null @@ -1,93 +0,0 @@ -import { mergeAttributes, Node } from "@tiptap/core"; - -/** - * Minimal inline tile for the composition-input spike. - * - * Deliberately the smallest thing that reproduces the risk: an - * `inline` + `atom` node whose DOM is a non-editable box. That box is what - * Chrome and Safari mis-handle at caret boundaries, and what a composing IME - * has to survive sitting next to. Nothing here is production shape — no - * resolver, no shared component, no address model. The spike answers one - * question: does staged character input beside such a node stay intact? - */ -export const TILE_NODE_NAME = "spikeTile"; - -export const SpikeTileNode = Node.create({ - name: TILE_NODE_NAME, - group: "inline", - inline: true, - /** - * Redundant for this node and kept as documentation of intent. ProseMirror - * computes `isAtom` as `isLeaf || spec.atom`, and a node with no content - * schema is already a leaf — so flipping this to `false` changes nothing, - * verified by running the spike with it off. It becomes load-bearing the - * moment a tile holds content. - */ - atom: true, - /** - * Load-bearing, and the single most important line in the file. With - * `selectable: true` an arrow key selects the tile and the very next - * keystroke REPLACES it — the tile silently vanishes mid-sentence. Verified - * by flipping it: three spike tests go red. TipTap's own mention extension - * ships `selectable: false` for this reason. - */ - selectable: false, - - addAttributes() { - return { - kind: { - default: "person", - parseHTML: (element) => - (element as HTMLElement).getAttribute("data-kind") ?? "person", - renderHTML: (attributes) => ({ "data-kind": String(attributes.kind) }), - }, - id: { - default: "", - parseHTML: (element) => - (element as HTMLElement).getAttribute("data-id") ?? "", - renderHTML: (attributes) => ({ "data-id": String(attributes.id) }), - }, - label: { - default: "", - parseHTML: (element) => - (element as HTMLElement).getAttribute("data-label") ?? "", - renderHTML: (attributes) => ({ - "data-label": String(attributes.label), - }), - }, - }; - }, - - parseHTML() { - return [{ tag: "span[data-spike-tile]" }]; - }, - - renderHTML({ node, HTMLAttributes }) { - return [ - "span", - mergeAttributes(HTMLAttributes, { - "data-spike-tile": "", - contenteditable: "false", - class: "spike-tile", - }), - `@${String(node.attrs.label)}`, - ]; - }, - - /** - * The address, not the label. A spike-scale stand-in for the contract that - * a sent message refers to the identity rather than the letters of a name. - */ - renderText({ node }) { - return `buzz://${String(node.attrs.kind)}/${String(node.attrs.id)}`; - }, - - /** - * Whole-tile deletion needs NO custom handler. With `atom: true` and - * `selectable: false`, Backspace from just after a tile removes it entirely — - * verified by deleting this comment's former handler and re-running the spike - * five times. TipTap's own mention extension adds a Backspace handler only to - * change the behaviour (leave the trigger character behind), not to make - * deletion work at all. - */ -}); diff --git a/desktop-new/src/features/composer-spike/ui/ComposerSpikePage.tsx b/desktop-new/src/features/composer-spike/ui/ComposerSpikePage.tsx index 03c7cb8a77..7e24d78abe 100644 --- a/desktop-new/src/features/composer-spike/ui/ComposerSpikePage.tsx +++ b/desktop-new/src/features/composer-spike/ui/ComposerSpikePage.tsx @@ -1,47 +1,64 @@ -import { useEffect, type MouseEvent as ReactMouseEvent } from "react"; +import { type MouseEvent as ReactMouseEvent, useEffect } from "react"; import { EditorContent, useEditor } from "@tiptap/react"; import StarterKit from "@tiptap/starter-kit"; -import { SpikeTileNode, TILE_NODE_NAME } from "../tileNode"; +import { TILE_NODE_NAME, TileNode } from "@/features/composer/tileNode"; +import type { TileAddress } from "@/shared/tiles/address"; +import { resetTileFaces, tileFaces } from "@/shared/tiles/faceResolver"; declare global { interface Window { - /** Spike-only: lets the test read live editor state without render timing. */ + /** Spike-only: lets a test read live editor state without render timing. */ __SPIKE_READ__?: () => { json: unknown; text: string; addresses: string[]; }; + /** Spike-only: renames an identity the way a profile update would. */ + __SPIKE_RENAME__?: (address: TileAddress, label: string) => void; + /** Spike-only: simulates switching community. */ + __SPIKE_RESET_FACES__?: () => void; } } +const MORGAN: TileAddress = { kind: "person", id: "pk-morgan" }; +const ALEX: TileAddress = { kind: "person", id: "pk-alex" }; + /** - * Composition-input spike harness. + * Harness for the tile composer's editing contract. * - * Not a product surface and not a design-system specimen. It exists so a test - * can drive real `compositionstart` / `compositionupdate` / `compositionend` - * sequences against an inline atom tile in the actual application shell, and - * read back whether the document survived. Delete this route once the spike's - * result is recorded in the plan. + * Not a product surface. It mounts the real TileNode, the real node view, and + * the real shared InlineTile so browser tests bind production seams rather + * than a test-only stand-in. Delete it once the product composer exists, + * folding its assertions into that composer's tests. */ export function ComposerSpikePage() { const editor = useEditor({ extensions: [ StarterKit.configure({ heading: false, link: false }), - SpikeTileNode, + TileNode, ], content: { type: "doc", content: [{ type: "paragraph" }] }, editorProps: { attributes: { "data-testid": "spike-editor", - "aria-label": "Composition spike composer", + "aria-label": "Tile composer harness", class: "spike-editor", }, }, }); + // Seed the faces these tiles resolve to. The product resolves them from real + // identity; the harness only needs them present. Deliberately no cleanup: + // resetting on unmount would make a community-reset test tear down the very + // harness it is measuring. + useEffect(() => { + tileFaces.put(MORGAN, { label: "Morgan", loading: false, resolved: true }); + tileFaces.put(ALEX, { label: "Alex", loading: false, resolved: true }); + }, []); + // Read live editor state on demand. A React-rendered readout lags editor - // transactions, which made the harness look like a browser defect. + // transactions, which made an earlier harness look like a browser defect. useEffect(() => { if (!editor) return; window.__SPIKE_READ__ = () => { @@ -59,33 +76,36 @@ export function ComposerSpikePage() { .map((node) => `${node.attrs?.kind}/${node.attrs?.id}`), }; }; + window.__SPIKE_RENAME__ = (address, label) => + tileFaces.put(address, { label, loading: false, resolved: true }); + window.__SPIKE_RESET_FACES__ = () => resetTileFaces(); return () => { window.__SPIKE_READ__ = undefined; + window.__SPIKE_RENAME__ = undefined; + window.__SPIKE_RESET_FACES__ = undefined; }; }, [editor]); if (!editor) return null; - // Insert buttons must not take focus, or the caret leaves the editor and - // every keyboard assertion afterwards measures the wrong thing. A real - // picker has the same obligation. + // Insert controls must not take focus, or the caret leaves the editor and + // every keyboard assertion afterwards measures the wrong thing. The real + // tile picker owes the same. const keepFocus = (event: ReactMouseEvent) => event.preventDefault(); - const insertTile = (label: string, id: string) => + const insertTile = (address: TileAddress) => editor .chain() .focus() - .insertContent({ - type: TILE_NODE_NAME, - attrs: { kind: "person", id, label }, - }) + .insertContent({ type: TILE_NODE_NAME, attrs: address }) .run(); return (
-

Composition input spike

+

Tile composer harness

- Drives staged character input beside an inline atom tile. Temporary. + Exercises the production tile node, node view, and shared InlineTile. + Temporary.

@@ -93,7 +113,7 @@ export function ComposerSpikePage() { type="button" data-testid="insert-morgan" onMouseDown={keepFocus} - onClick={() => insertTile("Morgan", "pk-morgan")} + onClick={() => insertTile(MORGAN)} className="rounded-md bg-inset px-3 py-1 text-body text-primary" > Insert Morgan @@ -102,7 +122,7 @@ export function ComposerSpikePage() { type="button" data-testid="insert-alex" onMouseDown={keepFocus} - onClick={() => insertTile("Alex", "pk-alex")} + onClick={() => insertTile(ALEX)} className="rounded-md bg-inset px-3 py-1 text-body text-primary" > Insert Alex @@ -121,31 +141,6 @@ export function ComposerSpikePage() {
- - - {JSON.stringify(editor.getJSON())} - - - {editor.getText()} - - - {String( - editor - .getJSON() - .content?.[0]?.content?.filter( - (n: { type: string }) => n.type === TILE_NODE_NAME, - ).length ?? 0, - )} -
); } diff --git a/desktop-new/src/features/composer/tileNode.ts b/desktop-new/src/features/composer/tileNode.ts new file mode 100644 index 0000000000..7bd8192948 --- /dev/null +++ b/desktop-new/src/features/composer/tileNode.ts @@ -0,0 +1,101 @@ +import { mergeAttributes, Node } from "@tiptap/core"; +import { ReactNodeViewRenderer } from "@tiptap/react"; + +import { + formatTileAddress, + isTileKind, + type TileAddress, +} from "@/shared/tiles/address"; + +import { TileNodeView } from "./ui/TileNodeView"; + +export const TILE_NODE_NAME = "tile"; + +/** + * A tile in the composer document. + * + * The document stores an address — a kind and an identity — and nothing else. + * No display name, because a name in the document would make every profile + * rename an edit to the person's unsent message, dirtying the draft and + * polluting undo history. The face is resolved at render time instead. + * + * Two flags carry the interaction contract, both verified by the composition + * spike (see the Desktop New plan): + * + * - `selectable: false` is load-bearing. With it true, an arrow key selects the + * tile and the very next keystroke REPLACES it, so the tile silently + * disappears mid-sentence. TipTap's own mention extension ships false for the + * same reason. + * - `atom: true` is currently redundant — ProseMirror computes `isAtom` as + * `isLeaf || spec.atom` and a contentless node is already a leaf — and is + * kept as a statement of intent. It becomes load-bearing if a tile ever + * holds content. + * + * Whole-tile deletion needs no keyboard handler: Backspace from just after a + * tile removes it entirely, natively. The spike proved this by removing a + * hand-written handler and re-running. + */ +export const TileNode = Node.create({ + name: TILE_NODE_NAME, + group: "inline", + inline: true, + atom: true, + selectable: false, + + addAttributes() { + return { + kind: { + default: "person", + parseHTML: (element) => + (element as HTMLElement).getAttribute("data-kind") ?? "person", + renderHTML: (attributes) => ({ "data-kind": String(attributes.kind) }), + }, + id: { + default: "", + parseHTML: (element) => + (element as HTMLElement).getAttribute("data-id") ?? "", + renderHTML: (attributes) => ({ "data-id": String(attributes.id) }), + }, + }; + }, + + parseHTML() { + return [{ tag: "span[data-tile]" }]; + }, + + renderHTML({ HTMLAttributes }) { + return [ + "span", + mergeAttributes(HTMLAttributes, { + "data-tile": "", + "data-kind": undefined, + }), + ]; + }, + + /** + * The plain-text projection of a tile is its address, never its label. + * + * This is what a sent message carries and what an agent reads. A reader that + * knows nothing about tiles still receives a meaningful `buzz://` link rather + * than broken markup, so the reference degrades rather than disappearing. + */ + renderText({ node }) { + const address = tileAddressFromAttrs(node.attrs); + return address ? formatTileAddress(address) : ""; + }, + + addNodeView() { + return ReactNodeViewRenderer(TileNodeView); + }, +}); + +/** Reads an address off node attributes, or null when they are not a valid one. */ +export function tileAddressFromAttrs( + attrs: Record, +): TileAddress | null { + const kind = String(attrs.kind ?? ""); + const id = String(attrs.id ?? ""); + if (!isTileKind(kind) || id.length === 0) return null; + return { kind, id }; +} diff --git a/desktop-new/src/features/composer/ui/TileNodeView.tsx b/desktop-new/src/features/composer/ui/TileNodeView.tsx new file mode 100644 index 0000000000..0958a0373d --- /dev/null +++ b/desktop-new/src/features/composer/ui/TileNodeView.tsx @@ -0,0 +1,32 @@ +import { NodeViewWrapper, type ReactNodeViewProps } from "@tiptap/react"; + +import { InlineTile } from "@/shared/ui/InlineTile"; + +import { tileAddressFromAttrs } from "../tileNode"; + +/** + * Mounts the shared InlineTile inside the editor document. + * + * This is the seam that lets one component serve both the composer and the + * conversation. It is deliberately thin: it reads the address off the node and + * renders, holding no state of its own, so there is nothing here to drift from + * the read-only rendering path. + * + * The tile is not interactive inside the composer. While writing, a click's + * meaning is "put my caret here", not "open this person's profile" — and a + * focusable control inside a contenteditable region takes the caret out of the + * editor. Detail-on-click belongs to the conversation, where a click has no + * competing meaning. + */ +export function TileNodeView({ node }: ReactNodeViewProps) { + const address = tileAddressFromAttrs(node.attrs); + + return ( + // Note: TipTap's `NodeViewWrapper` spreads its own props onto the rendered + // element, so `as` also lands as a literal DOM attribute. Cosmetic and + // upstream; not worth patching the dependency over. + + {address ? : null} + + ); +} diff --git a/desktop-new/src/features/design-system/ui/componentSpecimens.tsx b/desktop-new/src/features/design-system/ui/componentSpecimens.tsx index 5f88a9cbee..34da75c8de 100644 --- a/desktop-new/src/features/design-system/ui/componentSpecimens.tsx +++ b/desktop-new/src/features/design-system/ui/componentSpecimens.tsx @@ -8,6 +8,7 @@ import { import type { ReactNode } from "react"; import { useState } from "react"; import { Avatar } from "@/shared/ui/Avatar"; +import { InlineTile } from "@/shared/ui/InlineTile"; import { Button } from "@/shared/ui/Button"; import { IconButton } from "@/shared/ui/IconButton"; import { NavigatorRow } from "@/shared/ui/NavigatorRow"; @@ -16,6 +17,8 @@ import { PanelHeader } from "@/shared/ui/PanelHeader"; import { SearchField } from "@/shared/ui/SearchField"; import { SegmentedNavigation } from "@/shared/ui/SegmentedNavigation"; import { WorkspaceSurface } from "@/shared/ui/WorkspaceSurface"; +import type { TileAddress } from "@/shared/tiles/address"; +import { tileFaces } from "@/shared/tiles/faceResolver"; const DESTINATIONS = [ { value: "home", label: "Home" }, @@ -380,10 +383,106 @@ function NavigatorRowSpecimen() { ); } +const TILE_PERSON: TileAddress = { kind: "person", id: "pk-morgan" }; +const TILE_AGENT: TileAddress = { kind: "agent", id: "pk-vogue" }; +const TILE_CHANNEL: TileAddress = { kind: "channel", id: "ch-design" }; +const TILE_UNRESOLVED: TileAddress = { + kind: "person", + id: "9f2c4a1b7e5d8306a4b2c1d9e8f7a6b5c4d3e2f1a0b9c8d7e6f5a4b3c2d1e0f9", +}; +const TILE_LONG: TileAddress = { kind: "person", id: "pk-long" }; + +/** + * Seeds faces so the specimens show resolved tiles. The product resolves these + * from real identity; the page only needs the shapes to be visible. + */ +function seedTileFaces() { + tileFaces.put(TILE_PERSON, { + label: "Morgan", + status: "online", + loading: false, + resolved: true, + }); + tileFaces.put(TILE_AGENT, { + label: "Vogue", + status: "busy", + loading: false, + resolved: true, + }); + tileFaces.put(TILE_CHANNEL, { + label: "design", + loading: false, + resolved: true, + }); + tileFaces.put(TILE_LONG, { + label: "A deliberately very long display name that must truncate", + loading: false, + resolved: true, + }); +} + +function InlineTileSpecimen() { + const [activated, setActivated] = useState(null); + seedTileFaces(); + + return ( +
+ +
+ + + +
+ + + +

+ Asked and{" "} + to look at the thread in{" "} + before the review. +

+ + + +

+ A tile in a sent message is not a control when the surface has no + detail to open:{" "} + +

+ + + +
+ +
+ + + +
+ +
+ + + +
+ setActivated(address.id)} + /> + + {activated ? `Opened ${activated}` : "Not activated"} + +
+ +
+ ); +} + export const COMPONENT_SPECIMENS: Record ReactNode> = { button: ButtonSpecimen, "icon-button": IconButtonSpecimen, avatar: AvatarSpecimen, + "inline-tile": InlineTileSpecimen, "workspace-surface": WorkspaceSurfaceSpecimen, "segmented-navigation": SegmentedNavigationSpecimen, "panel-header": PanelHeaderSpecimen, diff --git a/desktop-new/src/shared/styles/globals.css b/desktop-new/src/shared/styles/globals.css index 086dfc1ae1..e458e61f6e 100644 --- a/desktop-new/src/shared/styles/globals.css +++ b/desktop-new/src/shared/styles/globals.css @@ -2,6 +2,7 @@ @import "./tokens.css"; @import "./typography.css"; @import "./components.css"; +@import "./tiles.css"; @import "./design-components.css"; @import "./product.css"; @import "./agents.css"; diff --git a/desktop-new/src/shared/styles/tiles.css b/desktop-new/src/shared/styles/tiles.css new file mode 100644 index 0000000000..17f6b412e6 --- /dev/null +++ b/desktop-new/src/shared/styles/tiles.css @@ -0,0 +1,119 @@ +@layer components { + /** + * Inline tile — a reference to a person, agent, or channel sitting in a line + * of text. Shared by the composer and the conversation, so the same + * reference looks identical whether it is being written or read. + * + * `inline-flex` is deliberate here, unlike the existing client's mention + * decorations: a tile is a real object the caret cannot enter, so it does + * not need to behave as a run of text and can align its icon properly. + */ + .inline-tile { + display: inline-flex; + max-width: 16rem; + align-items: center; + gap: var(--space-tile-gap); + border: 0; + border-radius: var(--radius-tile); + padding-inline: var(--space-tile-inset); + background: var(--bg-accent-tint); + color: var(--text-accent); + font: inherit; + /* Sits on the text baseline without stretching the line it lives in. */ + line-height: 1.25; + vertical-align: baseline; + /* A tile is an object: dragging or double-clicking must not select into + its label the way it would with text. */ + user-select: none; + transition: + background-color var(--duration-state) var(--easing-state), + color var(--duration-state) var(--easing-state); + } + + button.inline-tile { + cursor: pointer; + } + + button.inline-tile:hover { + background: var(--bg-accent-tint-hover); + } + + .inline-tile:focus-visible { + outline: 2px solid var(--ring-focus); + outline-offset: 2px; + } + + /* A channel is structure, not an identity, so it takes the neutral inset + rather than the accent tint. Colour is signal; a #channel reference is + not a signal about a person. */ + .inline-tile[data-kind="channel"] { + background: var(--bg-inset); + color: var(--text-secondary); + } + + button.inline-tile[data-kind="channel"]:hover { + background: var(--bg-inset); + color: var(--text-primary); + } + + .inline-tile-label { + min-width: 0; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + } + + .inline-tile-icon { + width: 0.875em; + height: 0.875em; + flex: 0 0 auto; + } + + .inline-tile-avatar { + width: 0.875em; + height: 0.875em; + flex: 0 0 auto; + border-radius: var(--radius-pill); + object-fit: cover; + } + + .inline-tile-status { + width: 0.375em; + height: 0.375em; + flex: 0 0 auto; + border-radius: var(--radius-pill); + } + + .inline-tile-status[data-status="online"] { + background: var(--bg-success); + } + + .inline-tile-status[data-status="busy"] { + background: var(--bg-warning); + } + + .inline-tile-status[data-status="offline"] { + background: var(--border-secondary); + } + + /* Still resolving its face. Quiet rather than animated: a pulsing tile in a + sentence draws the eye away from the words around it. */ + .inline-tile[data-loading] .inline-tile-label { + color: var(--text-tertiary); + } + + @media (prefers-reduced-motion: reduce) { + .inline-tile { + transition-duration: 0ms; + } + } +} + +@layer components { + /* The editor's wrapper around a tile node. Must not introduce its own box: + any padding or display change here shifts the caret target at the tile's + edges, which is the one boundary the browser already gets wrong. */ + .tile-node-view { + display: inline; + } +} diff --git a/desktop-new/src/shared/styles/tokens.css b/desktop-new/src/shared/styles/tokens.css index 41e1b0f8d1..d0df35302e 100644 --- a/desktop-new/src/shared/styles/tokens.css +++ b/desktop-new/src/shared/styles/tokens.css @@ -292,6 +292,10 @@ --space-panel-inset: var(--space-5); --space-control-inset: var(--space-3); --space-row-gap: var(--space-2); + /* An inline tile sits in a line of text, so its inset and gap are tighter + than a control's and are its own repeated relationship. */ + --space-tile-inset: var(--space-1); + --space-tile-gap: 0.1875rem; --size-topbar: 3.5rem; --size-panel-header: 3.5rem; --size-control: 2.25rem; @@ -301,6 +305,9 @@ --radius-control: var(--corner-control); --radius-row: var(--corner-row); --radius-pill: var(--corner-round); + /* A tile reads as a chip in running text, so it takes a tighter corner than + a control and gets its own role rather than borrowing one. */ + --radius-tile: 0.375rem; --duration-fast: var(--time-fast); --duration-state: var(--time-state); --duration-settle: var(--time-settle); @@ -434,4 +441,6 @@ --spacing-panel-inset: var(--space-panel-inset); --spacing-control-inset: var(--space-control-inset); --spacing-row-gap: var(--space-row-gap); + --spacing-tile-inset: var(--space-tile-inset); + --spacing-tile-gap: var(--space-tile-gap); } diff --git a/desktop-new/src/shared/tiles/address.test.ts b/desktop-new/src/shared/tiles/address.test.ts new file mode 100644 index 0000000000..1927099530 --- /dev/null +++ b/desktop-new/src/shared/tiles/address.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, it } from "vitest"; + +import { + formatTileAddress, + parseTileAddress, + sameTileAddress, + tileAddressKey, +} from "./address"; + +describe("tile address", () => { + it("round-trips every kind through its canonical form", () => { + for (const address of [ + { kind: "person" as const, id: "pk-morgan" }, + { kind: "agent" as const, id: "pk-vogue" }, + { kind: "channel" as const, id: "8f14e45f-ea11-4d3a-9f2c-1b7e5d830612" }, + ]) { + expect(parseTileAddress(formatTileAddress(address))).toEqual(address); + } + }); + + it("refuses text that is not an address rather than guessing", () => { + for (const value of [ + "@Morgan", + "buzz://person/", + "buzz://unknown/pk-morgan", + "https://example.com/person/pk-morgan", + "", + ]) { + expect(parseTileAddress(value)).toBeNull(); + } + }); + + /** + * The property the whole model exists for. Two identities that share a + * display name are different addresses, and nothing about the name enters + * the comparison — so there is no same-name collision to disambiguate and no + * need to write an identity into visible text. + */ + it("distinguishes two identities that share a display name", () => { + const first = { kind: "person" as const, id: "pk-morgan-1" }; + const second = { kind: "person" as const, id: "pk-morgan-2" }; + + expect(sameTileAddress(first, second)).toBe(false); + expect(tileAddressKey(first)).not.toBe(tileAddressKey(second)); + }); + + it("keeps kinds apart even when an id repeats across them", () => { + const person = { kind: "person" as const, id: "shared-id" }; + const agent = { kind: "agent" as const, id: "shared-id" }; + + expect(sameTileAddress(person, agent)).toBe(false); + expect(formatTileAddress(person)).not.toBe(formatTileAddress(agent)); + }); +}); diff --git a/desktop-new/src/shared/tiles/address.ts b/desktop-new/src/shared/tiles/address.ts new file mode 100644 index 0000000000..64b77106f1 --- /dev/null +++ b/desktop-new/src/shared/tiles/address.ts @@ -0,0 +1,63 @@ +/** + * A tile address: what a reference in a message actually *is*. + * + * The whole point is that a reference carries the identity of the thing it + * points at, never the letters of its name. The existing client stores literal + * `@Label` text plus a separate `label -> pubkey` map and re-matches by string + * at send time; that is why two people with the same display name force a + * 64-character key into the visible text, why a pasted label needs + * network-fenced verification, and why editing a name silently unbinds a + * recipient. An address has no second thing to drift out of agreement with. + * + * A display name is resolved *from* an address for presentation and is never + * treated as its meaning. Renames and same-name identities therefore cost + * nothing. + */ + +/** The kinds of thing a tile may refer to in version 1. */ +export const TILE_KINDS = ["person", "agent", "channel"] as const; + +export type TileKind = (typeof TILE_KINDS)[number]; + +export type TileAddress = { + kind: TileKind; + /** Pubkey for a person or agent; channel id for a channel. */ + id: string; +}; + +export function isTileKind(value: string): value is TileKind { + return (TILE_KINDS as readonly string[]).includes(value); +} + +/** + * The address's canonical string form, used in a message body and in the + * editor's own serialization. + * + * `buzz://` entity links already exist across this codebase, so a reader that + * knows nothing about tiles still receives something meaningful rather than + * broken markup — the reference degrades to a readable link instead of + * disappearing. + */ +export function formatTileAddress(address: TileAddress): string { + return `buzz://${address.kind}/${address.id}`; +} + +const ADDRESS_PATTERN = /^buzz:\/\/([a-z]+)\/([A-Za-z0-9._:-]+)$/; + +/** Parses a canonical address, or returns null when the text is not one. */ +export function parseTileAddress(value: string): TileAddress | null { + const match = ADDRESS_PATTERN.exec(value.trim()); + if (!match) return null; + const [, kind, id] = match; + if (!isTileKind(kind) || id.length === 0) return null; + return { kind, id }; +} + +/** Stable key for maps and React keys. Not a display value. */ +export function tileAddressKey(address: TileAddress): string { + return `${address.kind}:${address.id}`; +} + +export function sameTileAddress(a: TileAddress, b: TileAddress): boolean { + return a.kind === b.kind && a.id === b.id; +} diff --git a/desktop-new/src/shared/tiles/faceResolver.test.ts b/desktop-new/src/shared/tiles/faceResolver.test.ts new file mode 100644 index 0000000000..9c9b0bc857 --- /dev/null +++ b/desktop-new/src/shared/tiles/faceResolver.test.ts @@ -0,0 +1,143 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; + +import type { TileAddress } from "./address"; +import { fallbackLabel, resetTileFaces, tileFaces } from "./faceResolver"; + +const MORGAN: TileAddress = { kind: "person", id: "pk-morgan" }; +const FULL_KEY: TileAddress = { + kind: "person", + id: "9f2c4a1b7e5d8306a4b2c1d9e8f7a6b5c4d3e2f1a0b9c8d7e6f5a4b3c2d1e0f9", +}; + +afterEach(() => { + resetTileFaces(); +}); + +describe("tile face resolution", () => { + it("always returns something renderable, so a tile is never faceless", () => { + const face = tileFaces.get(MORGAN); + expect(face.label.length).toBeGreaterThan(0); + }); + + /** + * A face that shows a full identity is the defect the current client + * shipped: a same-name collision there forces a 64-character key into + * visible text. An unresolved tile abbreviates instead. + */ + it("never exposes a full identity in a fallback label", () => { + expect(fallbackLabel(FULL_KEY)).not.toContain(FULL_KEY.id); + expect(fallbackLabel(FULL_KEY).length).toBeLessThan(20); + expect(tileFaces.get(FULL_KEY).label).not.toContain(FULL_KEY.id); + }); + + it("resolves a name from a source without the caller knowing", () => { + tileFaces.setSource({ + peek: (address) => + address.id === MORGAN.id + ? { label: "Morgan", loading: false, resolved: true } + : undefined, + }); + + expect(tileFaces.get(MORGAN).label).toBe("Morgan"); + }); + + it("requests an unknown address once, however often it renders", () => { + const request = vi.fn(); + tileFaces.setSource({ peek: () => undefined, request }); + + tileFaces.get(MORGAN); + tileFaces.get(MORGAN); + tileFaces.get(MORGAN); + + expect(request).toHaveBeenCalledTimes(1); + }); + + /** + * A rename must reach every tile without the document changing, because a + * document change would dirty the person's draft and enter undo history. + * Subscribers are how the face updates independently of the document. + */ + it("notifies subscribers when a name changes", () => { + const listener = vi.fn(); + const unsubscribe = tileFaces.subscribe(listener); + + tileFaces.put(MORGAN, { label: "Morgan", loading: false, resolved: true }); + expect(listener).toHaveBeenCalled(); + expect(tileFaces.get(MORGAN).label).toBe("Morgan"); + + tileFaces.put(MORGAN, { + label: "Morgan Mulvaney", + loading: false, + resolved: true, + }); + expect(tileFaces.get(MORGAN).label).toBe("Morgan Mulvaney"); + + unsubscribe(); + }); + + it("stops notifying after unsubscribe", () => { + const listener = vi.fn(); + tileFaces.subscribe(listener)(); + + tileFaces.put(MORGAN, { label: "Morgan", loading: false, resolved: true }); + + expect(listener).not.toHaveBeenCalled(); + }); + + /** + * Community-scoped cache. A name resolved in one community is not valid in + * another, and a stale name surviving a switch is exactly the leak the + * desktop client's reset inventory exists to prevent. + */ + it("forgets every resolved name when the community resets", () => { + tileFaces.put(MORGAN, { label: "Morgan", loading: false, resolved: true }); + expect(tileFaces.get(MORGAN).label).toBe("Morgan"); + + resetTileFaces(); + + expect(tileFaces.get(MORGAN).label).not.toBe("Morgan"); + }); + + it("drops the previous community's source on reset", () => { + tileFaces.setSource({ + peek: () => ({ + label: "Old community name", + loading: false, + resolved: true, + }), + }); + expect(tileFaces.get(MORGAN).label).toBe("Old community name"); + + resetTileFaces(); + + expect(tileFaces.get(MORGAN).label).not.toBe("Old community name"); + }); +}); + +describe("assistive semantics depend on resolution", () => { + it("marks an unresolved face as unresolved rather than naming it", () => { + const face = tileFaces.get(FULL_KEY); + expect(face.resolved).toBe(false); + }); + + it("marks a resolved face as resolved", () => { + tileFaces.put(MORGAN, { label: "Morgan", loading: false, resolved: true }); + expect(tileFaces.get(MORGAN).resolved).toBe(true); + }); + + /** + * A snapshot must be reference-stable between changes or React's + * `useSyncExternalStore` re-renders forever and tears down the subtree. + * That is not hypothetical: an earlier version built a fresh unresolved face + * on every call and unmounted the whole composer. + */ + it("returns a reference-stable face between changes", () => { + const first = tileFaces.get(MORGAN); + expect(tileFaces.get(MORGAN)).toBe(first); + + tileFaces.put(MORGAN, { label: "Morgan", loading: false, resolved: true }); + const second = tileFaces.get(MORGAN); + expect(second).not.toBe(first); + expect(tileFaces.get(MORGAN)).toBe(second); + }); +}); diff --git a/desktop-new/src/shared/tiles/faceResolver.ts b/desktop-new/src/shared/tiles/faceResolver.ts new file mode 100644 index 0000000000..5164703ef2 --- /dev/null +++ b/desktop-new/src/shared/tiles/faceResolver.ts @@ -0,0 +1,153 @@ +import { type TileAddress, type TileKind, tileAddressKey } from "./address"; + +/** + * What a tile shows. Derived from an address, never stored in the document. + * + * Keeping the face out of the document is the load-bearing decision: if a + * display name lived in the draft, every profile rename would be an edit to + * the person's unsent message — dirtying it and polluting undo history. So the + * document holds the address and the face is looked up at render time. + */ +export type TileFace = { + /** The compact label a person reads. Never the identity. */ + label: string; + avatarUrl?: string | null; + /** Live status where the kind has one; absent means "no status to show". */ + status?: "online" | "busy" | "offline"; + /** True while the face is still being fetched, so a tile can show a skeleton. */ + loading: boolean; + /** + * Whether `label` is a real display name or a stand-in derived from the id. + * Assistive semantics need this: announcing an abbreviated identity tells a + * screen-reader user nothing, so an unresolved tile says so instead. + */ + resolved: boolean; +}; + +/** The face shown before anything is known, and if a lookup finds nothing. */ +export function unresolvedFace(address: TileAddress): TileFace { + return { label: fallbackLabel(address), loading: false, resolved: false }; +} + +/** + * A readable stand-in when an identity cannot be resolved. + * + * Deliberately an abbreviation of the id rather than a raw 64-character key: a + * tile's face must never expose the full identity, because a face that shows a + * key is the defect the current client shipped. + */ +export function fallbackLabel(address: TileAddress): string { + const { id } = address; + if (id.length <= 12) return id; + return `${id.slice(0, 8)}…${id.slice(-4)}`; +} + +export type TileFaceSource = { + /** Synchronous read from whatever this surface already knows. */ + peek: (address: TileAddress) => TileFace | undefined; + /** Kicks off a fetch for an address the cache does not hold yet. */ + request?: (address: TileAddress) => void; +}; + +type Listener = () => void; + +/** + * Resolves tile faces from addresses, and tells subscribers when one changes. + * + * This is a module-level cache holding community-scoped data, so it must be + * reset when the community changes or a name from the previous community leaks + * into the new one. See `resetTileFaces`. + */ +class TileFaceRegistry { + private faces = new Map(); + private listeners = new Set(); + private source: TileFaceSource | null = null; + private requested = new Set(); + + setSource(source: TileFaceSource | null): void { + this.source = source; + this.requested.clear(); + this.emit(); + } + + /** + * The current face for an address. Always returns something renderable — + * a tile never has "no face", it has an unresolved one. + * + * The returned object must be reference-stable between changes. React's + * `useSyncExternalStore` compares snapshots by identity, so handing back a + * fresh object each call is an infinite render loop that tears down the + * whole subtree — which is exactly what a freshly-built unresolved face did + * here. Unresolved faces are therefore cached like resolved ones. + */ + get(address: TileAddress): TileFace { + const key = tileAddressKey(address); + const known = this.faces.get(key); + if (known) return known; + + const peeked = this.source?.peek(address); + if (peeked) { + this.faces.set(key, peeked); + return peeked; + } + + // Ask once per address, and only if someone is actually rendering it. + if (this.source?.request && !this.requested.has(key)) { + this.requested.add(key); + this.source.request(address); + } + + const unresolved: TileFace = { + label: fallbackLabel(address), + loading: Boolean(this.source), + resolved: false, + }; + this.faces.set(key, unresolved); + return unresolved; + } + + /** Records a resolved face and notifies anything rendering that address. */ + put(address: TileAddress, face: TileFace): void { + this.faces.set(tileAddressKey(address), face); + this.emit(); + } + + subscribe(listener: Listener): () => void { + this.listeners.add(listener); + return () => { + this.listeners.delete(listener); + }; + } + + reset(): void { + this.faces.clear(); + this.requested.clear(); + this.source = null; + this.emit(); + } + + private emit(): void { + for (const listener of this.listeners) listener(); + } +} + +export const tileFaces = new TileFaceRegistry(); + +/** + * Clears every resolved face. + * + * Must be called when the community changes. A face resolved in one community + * is not valid in another, and a stale name surviving a switch is the exact + * leak the desktop client's `resetCommunityState()` inventory exists to + * prevent. + */ +export function resetTileFaces(): void { + tileFaces.reset(); +} + +/** Kind-level presentation facts, independent of any lookup. */ +export const TILE_KIND_TRIGGER: Record = { + person: "@", + agent: "@", + channel: "#", +}; diff --git a/desktop-new/src/shared/ui/InlineTile.tsx b/desktop-new/src/shared/ui/InlineTile.tsx new file mode 100644 index 0000000000..66ddd7dcd6 --- /dev/null +++ b/desktop-new/src/shared/ui/InlineTile.tsx @@ -0,0 +1,128 @@ +import { IconHash, IconRobot, IconUser } from "@tabler/icons-react"; +import { useSyncExternalStore } from "react"; + +import type { TileAddress, TileKind } from "@/shared/tiles/address"; +import { + TILE_KIND_TRIGGER, + type TileFace, + tileFaces, +} from "@/shared/tiles/faceResolver"; + +/** + * A reference to a person, agent, or channel, shown inline in a sentence. + * + * One component serves both the composer and the conversation. Two + * implementations of one face drift, and a reference that looks different + * depending on whether it is being written or being read is the kind of + * disconnected feature this client exists to avoid. + * + * A tile is an object, not styled text: the caret cannot sit inside it, one + * deletion removes it whole, and it carries its identity rather than its name. + * The label is resolved from the address at render time, so a rename updates + * every tile without editing anyone's draft. + */ + +const KIND_ICON: Record = { + person: IconUser, + agent: IconRobot, + channel: IconHash, +}; + +/** Subscribes to face changes so a rename repaints without a document edit. */ +function useTileFace(address: TileAddress): TileFace { + return useSyncExternalStore( + (listener) => tileFaces.subscribe(listener), + () => tileFaces.get(address), + ); +} + +export function InlineTile({ + address, + interactive = true, + onActivate, +}: { + address: TileAddress; + /** + * Whether the tile is a control. Read-only contexts pass false so a tile + * does not claim an interactive screen-reader stop it cannot honour. + */ + interactive?: boolean; + onActivate?: (address: TileAddress) => void; +}) { + const face = useTileFace(address); + const Icon = KIND_ICON[address.kind]; + const trigger = TILE_KIND_TRIGGER[address.kind]; + + const content = ( + <> + {face.avatarUrl ? ( + + ) : ( +