From e433981defc02ce7a14d067758c9a281bb3c9751 Mon Sep 17 00:00:00 2001 From: mintaka Date: Sat, 12 Sep 2026 20:52:28 -0400 Subject: [PATCH] feat(ui): split ActivityBarItem into its glyph and avatar arms (RIG-3738) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ActivityBarItem.icon: string` carried two unrelated things: a fixed chrome symbol on the four static tabs, and a person's initial on the fleet tabs. The name fit neither, and `agentId`/`unreachable` sat on the shared interface though only fleet tabs ever have them. The item now splits at the item, not the field: `GlyphTabItem` carries `name: GlyphName` and renders ``; `AvatarTabItem` carries `letter` plus the `agentId` and `unreachable` that only it uses. A field-level union was rejected — it keeps a field called `icon` whose value is sometimes a person's initial. The union, the constructors, the render branch and the CSS land together because they must: the activity-bar loop reads both `tab.icon` and `tab.agentId` off the un-narrowed item, so landing the type without the render branch is a strict-mode error and leaves a red commit mid-stack. Narrowing removed a non-null assertion in `AgentUnreachable` — its `agentId` is now required by the type rather than asserted at the use site. `store.ts` is unchanged: `rightTabGroups` holds a genuinely mixed list, so the union is already the right type there. The glyph box is pinned to a whole-pixel offset. An 11px box flex-centered in the tab's 32px content box lands at 10.5px, which smears every 1px `crispEdges` cell across two device pixels; whole-integer margins that fill the axis exactly leave no free space for centering to halve. The SVG is also seated at the box's top edge: an inline-level replaced box rides the text baseline, so without that it renders 2px below the box the margins just placed. `state-dot.css` carries the same rule for the same reason. Measured in Chromium at dpr 1 and 2: the glyph lands at (11, 11) with no overflow; flex centering would put the box at 11.5px, and without the seating rule the glyph sat at y=13. `bridge-colheads.png` is recaptured here rather than with the rest of the baselines, because this is the commit that repaints those pixels. Its clip is computed from the column-head boxes and runs 1050px wide, so it overlaps the activity bar; at 855x41 the 0.001 diff-pixel RATIO allows about 35 pixels and the four glyphs change 99. The full-page captures contain the same changed pixels but pass, their delta swamped by a budget proportional to mostly unchanged area — so they are recaptured later, with the rest. This surface had no tests. It now covers both render arms, the StateDot's presence and absence, the constructors, and the margin arithmetic the whole-pixel offset depends on. Ref: RIG-3738, RIG-3603. Design: docs/designs/ui/compass-glyph-primitives/design.md (DL-367). Co-authored-by: Matt Wilkinson --- apps/ui/e2e/__screens__/bridge-colheads.png | Bin 2544 -> 2446 bytes apps/ui/src/app.css | 27 +++- .../RightSidebar.activitybar.test.tsx | 146 ++++++++++++++++++ apps/ui/src/components/RightSidebar.tsx | 48 +++--- apps/ui/src/constants.test.ts | 60 ++++++- apps/ui/src/constants.ts | 96 ++++++++---- apps/ui/src/store.test.ts | 4 +- 7 files changed, 328 insertions(+), 53 deletions(-) create mode 100644 apps/ui/src/components/RightSidebar.activitybar.test.tsx diff --git a/apps/ui/e2e/__screens__/bridge-colheads.png b/apps/ui/e2e/__screens__/bridge-colheads.png index aef6656cc7a9b83a90b175c56cd1ea3e9cc34968..dba46ca05239933bb7e8c3e861cb3cae836d6593 100644 GIT binary patch literal 2446 zcmb_e`9Bkm8(%~cC3mFUB1g_xLVX>hP>8g!966FJ_nD(1IhJdVS#lq9ZnVv)v@-X# zAxtQ9N1Ai^uK(fl%kw<%_YcqWyq@Rv9Le`AOt?8kH~|0vw<*Zb3IJfWW7_f@EKF$& zF-!pfPXEu;@Rm(z;p+5?C7335YJ&!cT#S@D$pP^>I~#pkLOMY!OC!q|TW>zr;Fe!* zgwVi{soR!|lcqH|^hmW5=h`q=iww_q0}$VE!efrvr^;FjUU4U8ers)CdLcO=5#NRK zzCExbwX#V2M!U!ZZ0exx6vkgC=8DMz&ZB>BgM?L1B#GDL&Zb=8jV@;c^E>^J(PIH@ zNZwB{frHvKQBV_Ru7H;j87wE5`U8hN&p(**-$M_+e*s>m{y>FL&yJesijdOz9&z5y zBrQ`br+MWNh57Bn0pwY~-I(G`xw+C|m80k)iZ6w{;QD*_=K#$k>gPuB+xPE3wm2O( zd)`G^fI!*@1y0uw5B_v5RvPeZC7z>pP=X2g3KMC5!tucjn=Pfczo=tFMk$(}5}v1+ zEf|b zyQwALj<%O7o|dSF9zHDIz~FF!VL2nSA{Q1rBDQ326v5lp$nhAGygYmS?9g=GY-}`8 z7zy9)X?6#T6nr&?S{ByLQvK_l5PwYG;U1;^zjjpfH7rGpvqp!1L@@P~{Hw)?XAvB6<7W-nPi$dQHU&f)_S8ZWJ^jWM*j zDa3|6dS!p#Td>D#<##5G`#+UsOxm0$A2n563t z^$OF&ztX_eF1z~=vLy!g2Z$*eEbPJLvFWt{oQJBqCMoR=mqNgqGm9$X{G2Hf5ldE(P%?6gnSLo)z?0`_yt zV7#M1IL6o(R>vihl0A2qomVg|X$7rKYU<7|E@RX$^P6`q%$v?N6B+N327*FCtKHTk z>Z@JzR6U$cx+VJYp|NIpe>X_3p>FZi6DzAZ#%JB+BzeqtXRV?1*{aRc#n zvz3T*>r`yW!GxVTZ*l^&!I2VwS%}WCv6Zrl&uFaz1_1~>->c`43c3=xgRu6FT33lF z%k+PCoJ5FFSd15pe_h{vJB637|$WzGLQ;3cJ{*`1X&R$)UFh|LR3~7sC zyIr+FGT=a9sR3Yo<2EQZe?l%W51CsdEYjc8(|F?|?$^0_EEfCzbDUJ-xuNvs+TOcm zW&-1e;yj}^cFpuktExPNW`3`2O^`3qC;6e|9rMi@_v$?ET4X`9%bh8~N7@ekc!wR2 z#%6Sa@-O?mKrdgs?lX_jtdIN7AVG3h6#B7! za`OvyHnHDWvZ2+M=Pvf$df z!0wi7X9$!qrzFL#rFLE6I2>QD5YYnHlpO98R_hL`9vHjC&ILhY{IQzUQczS^$A>lo`N4 zO%?CoqI%4F9F*k>Se7<#UtXp6RmrPmPJjRa$(3)%=^r6?BPV7vw8rhQt>xjzuj-X8 zEG_w74k}@9@_xMoknCD^+jWK}AU!_`5`{v?+W47;6)*}_X(LM3-aca2K~ZMtI@ZU^ z>50BbR_3uDuVk`VRZPAXOUZR3R1i{@NdnrYqPuoDT$F{GHl(DpR{0+(B*FIY>G3;n sCH`N-Lm>d*_&C{^lZ)w@{ogyq3ZS$$jbClua{d>~)X2iH?lvUu|D&LvJpcdz literal 2544 zcmb_e`8(8)6Mu&!Wt|D1NUl~`r0$dGu>w8e$ss! z1d*ib>Q=#=guNUobWOK>%7PlZZ$7t&I>SEgwIB0{SDr!wQkY?)q(du*WYj}+tg`d%?W~0rlaNUK zHW$!Ag!4-{NYO{?1SF0ewhR2y;^sJ1Gg5_`f1#50e-2;q{4D^CB{VfPEiPgj@(2Xk zX)TOA7$H{?T;yk#>l>mSTtT6lmm-mnZUEb*irSEEta8%Z7;P{@F(Z<>mw4g? z`(D^C?nHqYLRWTe1>}hxY`>WpW^G;L?DVS_=uTJ)A>kS>zM3Zp$AhaISRX zi`-}LO}*F*ioD3&9yKcw8?+M<7gP8CF*je>y~_*rY`(YB$==sG{Vj}GpCRngwhp2?UJ;86 z3Z_ygx!?u{y2akXqV%2OdcpEwfBhRpGW|yFn?W;W)YYKh8)q>ljsA9M^trCJ>}tr| z{e^Ci$&g}}G#kLD<+`-wm>{zTfjX?`yG4Yc&J}7#YI^R+!D=TSGMN{Pu52xSzZHJ4 zL8SP5k?z^rPCax7i@Yb6>gl<-EVw!}WKKMXW+WDOyrz2*d!FKRXr4D}8qm_U#R7uq zPdKGvvBn3&XDN`uNSH@0^(3R`E7tyKcM z&@8p?++m(yIGkc5nw*w(4wsy48;XyKiJ>QWZ92TR$zzDW+i%TVReu?}A099?b*nKu z&M6rWv%iku2X&H5_PS+>oTLGBPxzld+;LDY;=S( zIJo*ADWILDz47b1vY-ILDIIOe(Z#(0RKi$`RW*6dfCBdx;c1n zjF5v2qbM9F@f`!=GoO7HN*_KGAMtr@=F4M#$Jg9`akV+nQ;W7J#4QuO;(n$K&qQNgKn4WhJH?$RkZfa~5Ctc)7OjbS84 z40*Ed=t?~>bXy)4Jda_uta-JmVVZuqMvYsy4D^mcN{6-7Y~tZkqzHWd;@KAotMFD%v6yxWdx z-NYF=0N^hn3{^uW_4K&AGT8zR+0P>bNI&&0bE@dxqq9Lj@3uy$iTj^TN#;MGG$eQU zBpAis%-QADM%GO9gLpHm3mniQNZ9@-;1A3OlK|=&`mCSiq4Y@_Fi2%AV z>CQ^Z-I?Ix33+QEtd2#5D+fX|6`#+|nYTIQ0YIhC;T_@obxKA=V(B{}D>4wZi?^J# z?A@$ogaV`FRu-Rt2_Jc&@qMf02w`p}bK>qP{~=jME1Fyx4i1A%6PF)KhS=Q{H&V5w ztq0-q)a9QWAUL}WG~6xwV-LRF)QMB%S@-FN`L2>5N86eTUlL;dS#4bU>}h9px<$#j zK!TsksbaHLz0mY1@;Xhft{E!f8Cmpu(n9Z44VorlWW6m;f0X26m4J$yx1UTmuf z+eu|ySpUQD@%fID%d9qz?-uCnO}4liJaR=v`1EOat;Zi8WMr)HoFR^yNwYt$6ZwMq zlgQontp^kKalWt+Ww-mAzad-EM@1!rqAXkTPT=X&MdMd~D+x)-{|qSBkmh=Jt! z`4V2}brp@uKfK`=Pb4iiG*v!9pXf=9R#6p?d}kjoHGR-kV%GJl>4uJ0nya7g*@g#E zj%fiXue;jXY*qf2E8ePSIt|$o&St(FoKTp+Rv7?9TX4yHnwI<{VRH7SZz^ol?uQCL(Rl`b@CC0#bwT74Mb4n$388=mXKL^G_}dXv{OA;&V#Q0r;RhNZ zkXCC{OVz8d7|5ahrXZoc&rnQ+I6O4X%*O=ZMY@!j)Y$p0r9~dOc645Nd$4!^hhzHi zxYv4dc}bim9DEX)hVL7LOeYn2Ih_=27rG#z?`LlQ=8>y@7IoFpOj5&c@v-f6Znl&P z>JA5Bui^B=a4W8^tNH`sDy3yG%-g7fA`mg87$hw71P{Pd2m04~4&U&4u12ErS~W@N z6cBMrCt$F(N6LC9g*z}dBm*?-G`pp$y`5BkuB1aD=12W(ABIkyW`@=4>;jiGxuf6R zI^5d@<5Kyx1Eo1+>OU0BqNe>u>;<^yAw7s-(5N9P;Q6S(tYoZ(532Tm*3f=EmF=&m xiA$dEZ&cEM2wCg^aBwixH?w@0ne#7UV+RQ3Ghhoq|NEf_(AP17S8Llx{R@s-v{L{8 diff --git a/apps/ui/src/app.css b/apps/ui/src/app.css index a7e771054..d2001e93c 100644 --- a/apps/ui/src/app.css +++ b/apps/ui/src/app.css @@ -2261,7 +2261,32 @@ opacity: 0.55; } -.r-tab .r-tab-icon { +/* The glyph arm is an 11px 1-bit SVG box; the avatar arm is a 15px mono + * letter. They split so the glyph box carries NO font metrics (the SVG is the + * content) and the letter keeps its type. */ +.r-tab .r-tab-icon[data-kind="glyph"] { + display: block; + width: 11px; + height: 11px; + /* D2: an odd 11px box centered in the tab's 32px content box (34px − + * 2×1px border, box-sizing: border-box) lands at (32 − 11) / 2 = 10.5px — + * a half pixel that smears every 1px crispEdges cell across two device + * pixels. Whole-integer margins that fill the content box exactly + * (10 + 11 + 11 = 32 per axis) leave zero free space for centering to + * split, so the box's offset is its whole-pixel margin (10px). */ + margin: 10px 11px 11px 10px; +} + +/* Seat the SVG at the box's top edge. An inline-level replaced box rides the + * parent's text baseline, so the inherited line-height pushes the glyph 2px + * down and out of its box — the whole-pixel margin above would then describe + * the span, not the pixels. state-dot.css carries the same rule for the same + * reason. */ +.r-tab .r-tab-icon[data-kind="glyph"] > svg { + display: block; +} + +.r-tab .r-tab-icon[data-kind="avatar"] { font-size: 15px; line-height: 1; } diff --git a/apps/ui/src/components/RightSidebar.activitybar.test.tsx b/apps/ui/src/components/RightSidebar.activitybar.test.tsx new file mode 100644 index 000000000..38f1d6f4d --- /dev/null +++ b/apps/ui/src/components/RightSidebar.activitybar.test.tsx @@ -0,0 +1,146 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import { render } from "@solidjs/testing-library"; +import { flush } from "solid-js"; +import { STUB_COMMS_STATE } from "../comms-stub"; +import { StoreContext } from "../context"; +import { type AppStore, createAppStore } from "../store"; +import { testQueryClient } from "../test-support"; +import { RightSidebar } from "./RightSidebar"; + +// Render spec for the activity-bar tab icon (RightSidebar.tsx, the `.r-tab-icon` +// span). The item type split at RIG-3603 into a glyph arm (a fixed 1-bit +// ``) and an avatar arm (a person's initial as text + StateDot); this +// file defends that BOTH arms render as their contract says — no coverage +// existed before. FleetPane/tab loop are reached through the exported +// RightSidebar, the same seam a real activity-bar click uses. +function mountRightSidebar(): { store: AppStore; container: HTMLElement } { + let store!: AppStore; + const { container } = render(() => { + store = createAppStore({ + initialComms: STUB_COMMS_STATE, + queryClient: testQueryClient(), + }); + return ( + + + + ); + }); + return { store, container }; +} + +// The tab button for a given aria-label, so a case targets one arm rather than +// reading the first `.r-tab` and hoping it is the intended one. +function tabByLabel(container: HTMLElement, label: string): HTMLButtonElement { + const button = [ + ...container.querySelectorAll("nav.r-activity .r-tab"), + ].find((b) => b.getAttribute("aria-label") === label); + if (!button) throw new Error(`no activity-bar tab labelled "${label}"`); + return button; +} + +describe("RightSidebar activity bar tab icons", () => { + // pinAgent write-throughs to the process-wide happy-dom localStorage, so clear + // it around every case (the fleet-pane suite's discipline). + beforeEach(() => globalThis.localStorage.clear()); + afterEach(() => globalThis.localStorage.clear()); + + // The glyph arm: a static tab draws a 1-bit `` — an SVG with + // crispEdges and lit `` cells, carrying NO text. A regression to the + // old `{tab.icon}` string would render text and no SVG, reddening both legs. + test("a static tab renders a crispEdges SVG glyph with no text", () => { + const { container } = mountRightSidebar(); + const icon = tabByLabel(container, "Fleet status").querySelector( + ".r-tab-icon", + ); + expect(icon).not.toBeNull(); + expect(icon?.getAttribute("data-kind")).toBe("glyph"); + const svg = icon?.querySelector("svg"); + expect(svg).not.toBeNull(); + expect(svg?.getAttribute("shape-rendering")).toBe("crispEdges"); + // Lit cells prove it is the real bitmap, not an empty box. + expect(svg?.querySelectorAll("rect").length).toBeGreaterThan(0); + // No initial leaked through — the glyph arm is textless. + expect(icon?.textContent?.trim()).toBe(""); + }); + + // The avatar arm: a resolvable fleet tab renders the handle's initial as TEXT + // (no SVG glyph) plus the agent's StateDot badge. "compass-ui" → "C". + test("a resolvable fleet tab renders its initial as text plus a StateDot", () => { + const { store, container } = mountRightSidebar(); + store.pinAgent("acc-compass-ui"); + flush(); + const tab = tabByLabel(container, "compass-ui"); + const icon = tab.querySelector(".r-tab-icon"); + expect(icon?.getAttribute("data-kind")).toBe("avatar"); + expect(icon?.textContent?.trim()).toBe("C"); + // The avatar arm draws text, not a Glyph SVG. + expect(icon?.querySelector("svg")).toBeNull(); + // The live agent badges the tab with a StateDot. + expect(tab.querySelector(".cx-state-dot")).not.toBeNull(); + }); + + // An unreachable pin (an id resolving to no fixture agent) still renders its + // initial, but carries NO StateDot — the absent badge is the visual mark of a + // dead pin (RIG-1645), so this reddens if the tab badges an unresolved agent. + test("an unreachable fleet tab renders its initial but no StateDot", () => { + globalThis.localStorage.setItem( + "compass.pinnedAgents.acc-matt", + JSON.stringify([{ id: "acc-ghost", handle: "ghosthandle" }]), + ); + const { container } = mountRightSidebar(); + flush(); + const tab = tabByLabel(container, "ghosthandle (unreachable)"); + const icon = tab.querySelector(".r-tab-icon"); + expect(icon?.getAttribute("data-kind")).toBe("avatar"); + expect(icon?.textContent?.trim()).toBe("G"); + expect(tab.querySelector(".cx-state-dot")).toBeNull(); + }); + + // D2 — the whole-pixel offset. happy-dom applies no stylesheet and computes + // no layout, so real geometry is NOT observable here. This is a PROXY: it + // parses app.css and asserts the mechanism that guarantees the offset — the + // glyph box is an integer 11px square whose integer margins fill .r-tab's + // 32px content box (34px − 2×1px border, box-sizing: border-box) EXACTLY on + // each axis. With zero free space, flex centering has no slack to halve, so + // the box's offset is its whole-pixel margin, not the 10.5px a centered 11px + // box would take. It proves the declared geometry is whole-pixel; it does NOT + // prove the browser rasterizes it there (that is the T6 visual baseline). + test("the glyph box CSS pins a whole-pixel offset in the 34px tab (D2 proxy)", () => { + const css = readFileSync(join(import.meta.dir, "../app.css"), "utf8"); + const rule = css.match( + /\.r-tab \.r-tab-icon\[data-kind="glyph"\]\s*\{([^}]*)\}/, + )?.[1]; + expect(rule).toBeDefined(); + const decl = (prop: string): string | undefined => + rule + ?.match(new RegExp(`(?:^|[;{\\s])${prop}\\s*:\\s*([^;]+);`))?.[1] + .trim(); + const px = (v: string | undefined): number => { + const n = Number(v?.replace("px", "")); + expect(Number.isInteger(n)).toBe(true); + return n; + }; + const width = px(decl("width")); + const height = px(decl("height")); + expect(width).toBe(11); + expect(height).toBe(11); + // margin shorthand: top right bottom left. + const margins = (decl("margin") ?? "").split(/\s+/); + expect(margins.length).toBe(4); + const [mt, mr, mb, ml] = margins.map((m) => px(m)); + // The 32px content box is filled exactly on each axis — no centering slack. + expect(ml + width + mr).toBe(32); + expect(mt + height + mb).toBe(32); + // Seating: without `display: block` on the SVG itself the glyph rides the + // text baseline and leaves the box the margins above just placed, so this + // geometry would describe the span and not the pixels a user sees. + const seat = css.match( + /\.r-tab \.r-tab-icon\[data-kind="glyph"\]\s*>\s*svg\s*\{([^}]*)\}/, + )?.[1]; + expect(seat).toBeDefined(); + expect(seat).toMatch(/display\s*:\s*block/); + }); +}); diff --git a/apps/ui/src/components/RightSidebar.tsx b/apps/ui/src/components/RightSidebar.tsx index d5d5ced5d..198c1075c 100644 --- a/apps/ui/src/components/RightSidebar.tsx +++ b/apps/ui/src/components/RightSidebar.tsx @@ -15,7 +15,7 @@ import { primaryPr, } from "../board-render"; import type { Channel } from "../comms-stub"; -import type { ActivityBarItem } from "../constants"; +import type { AvatarTabItem } from "../constants"; import { useStore } from "../context"; import { type Agent, @@ -27,6 +27,7 @@ import { STUB_FILES, } from "../stub-data"; import { ChannelView } from "./ChannelView"; +import { Glyph } from "./Glyph"; import { RuntimeMarker } from "./RuntimeMarker"; import { StateDot } from "./StateDot"; @@ -426,10 +427,9 @@ const RepoBranchDropdown: Component = () => { * agent's full workspace via store.openAgent. Only rendered for a RESOLVABLE * pin (RIG-1645 P2): the pane arm resolves reachability before choosing this * vs the unreachable block, so there is no unresolved-agentId fallback here. */ -const FleetPane: Component<{ item: ActivityBarItem }> = (props) => { +const FleetPane: Component<{ item: AvatarTabItem }> = (props) => { const store = useStore(); - const agent = (): Agent | undefined => - props.item.agentId ? store.agentById(props.item.agentId) : undefined; + const agent = (): Agent | undefined => store.agentById(props.item.agentId); return ( {(a) => { @@ -461,7 +461,7 @@ const FleetPane: Component<{ item: ActivityBarItem }> = (props) => { * affordance for an unreachable pin, whose left-tree row is gone (the tree * renders the VISIBLE set). Unpinning routes through `store.unpinAgent`, which * drops the pin and falls the active tab back to `status`. */ -const AgentUnreachable: Component<{ item: ActivityBarItem }> = (props) => { +const AgentUnreachable: Component<{ item: AvatarTabItem }> = (props) => { const store = useStore(); return (
@@ -471,13 +471,7 @@ const AgentUnreachable: Component<{ item: ActivityBarItem }> = (props) => { @@ -574,13 +568,13 @@ export const RightSidebar: Component = () => { // item-construction site. Never undefined for a pinned `agent:` tab (that was // the blank-pane gap); undefined only for a non-`agent:` tab or an `agent:` // tab with no matching pin (which falls through to `status`). - const activeFleetItem = (): ActivityBarItem | undefined => { + const activeFleetItem = (): AvatarTabItem | undefined => { const active = store.activeRightTab(); if (!active.startsWith("agent:")) return undefined; return store .rightTabGroups() .flatMap((g) => g.items) - .find((i) => i.id === active); + .find((i): i is AvatarTabItem => i.kind === "avatar" && i.id === active); }; return ( @@ -659,8 +653,14 @@ export const RightSidebar: Component = () => { {(tab) => { + // Only the avatar arm carries an agentId / unreachable + // mark and a StateDot; the glyph arm draws a fixed symbol. const agent = (): Agent | undefined => - tab.agentId ? store.agentById(tab.agentId) : undefined; + tab.kind === "avatar" + ? store.agentById(tab.agentId) + : undefined; + const unreachable = (): boolean => + tab.kind === "avatar" && tab.unreachable === true; return (