Size and dim plugin icons with a data-icon-root contract

[ymichael]

Human comments

What was wrong

Container styles reached into icon internals with [&>svg] and [&_svg], which assume every icon renders a bare <svg>. That assumption is false for plugin icons: Icon wraps plugin-registered custom icon components in a <span>, and PluginCompactIconMask renders a <span> with a CSS mask. Those selectors therefore skipped plugin icons entirely.

The visible symptom was the lame-duck plugin's icon in the sidebar footer. COARSE_POINTER_CHILD_ICON_BUTTON_CLASS bumps [&>svg] from size-4 to size-5 on narrow coarse-pointer viewports, so on mobile the duck stayed 16px while Settings, Remote access, and Report a bug went to 20px. Separately, SIDEBAR_FOOTER_ACTION_CLASS dims its glyph with [&>svg]:opacity-80, so the duck rendered at opacity 1 against its neighbours' 0.8 — on every pointer type, not just coarse. This affects any plugin icon in any of these containers, not just the footer; PluginIcon is rendered from 21 files.

What changed

Each icon now marks exactly one root element with data-icon-root: BuiltinIcon's svg, ExtendedIcon's svg, Icon's custom-icon wrapper span, and PluginCompactIconMask's span. The 34 container selectors that style icons target that marker — [&>[data-icon-root]] and [&_[data-icon-root]] — across shared-ui (button, toggle, command, context-menu, dropdown-menu, alert, breadcrumb, coarse-pointer-sizing), apps/web (button), and 11 apps/app components.

Dropping the svg arm is load-bearing rather than cosmetic. Keeping it (as :is(svg,[data-icon-root])) is safe under the child combinator but broken under the descendant combinator, which 26 of the 34 sites use: it matches the wrapper and the svg inside it, compounding the declaration. Measured in Chrome, opacity-80 applied that way yields an effective 0.64 rather than 0.8. Marking only the outermost element makes both combinators resolve to exactly one node.

To preserve current rendering, raw SVG glyphs that these rules already styled are marked as roots too: the disclosure chevron, GithubFaviconIcon, both context-window chevrons, and the locally-rendered breadcrumb and command lucide icons. Deliberately left unmarked, because they are self-sized graphics rather than icons: the SplitPaneMiniMap glyph, the context-window gauge, and the h-0 w-0 svg defs.

Four files keep the svg selectors and are allowlisted, because they style third-party SVG rather than icons: markdown-mermaid-diagram (mermaid), chart (recharts), calendar (react-day-picker), and one story.

No wire, CLI, or plugin SDK surface changes.

How you verified

New apps/app/src/components/ui/icon-root.test.tsx:

• asserts each of the three icon shapes renders exactly one data-icon-root
• asserts both .host > [data-icon-root] and .host [data-icon-root] resolve to exactly one node per shape — zero would mean the rule missed the icon, two would mean it applies twice
• pins the double-match regression directly: :is(svg,[data-icon-root]) as a descendant matches 2 nodes where the marker matches 1
• scans apps/app/src, apps/web/src, and packages/shared-ui/src for new [&_svg]: / [&>svg]: utilities outside the allowlist, and asserts the allowlist has no stale entries

The guard was mutation-tested rather than only observed passing: unmarking the wrapper span and reintroducing one [&_svg] selector each fail it (4 of 5 assertions).

Commands, all clean:

pnpm exec turbo run typecheck lint test --filter=@bb/shared-ui --filter=@bb/app --filter=@bb/web

@bb/app 4496 tests / 524 files, @bb/web 124 tests / 23 files, 0 lint errors. One pre-existing assertion in FilePreview.test.tsx pinned the literal old class string and was updated.

Manual verification drove the real app (pnpm start:worktree) with the lame-duck plugin installed, via CDP with pointer: coarse emulation confirmed in-page. Before and after are both real builds of this checkout, with the served CSS hash checked each pass to rule out a stale bundle.

view	glyph	before	after
mobile, coarse	duck	16×16, opacity 1	20×20, opacity 0.8
mobile, coarse	other three	20×20, opacity 0.8	unchanged
desktop, fine	duck	16×16, opacity 1	16×16, opacity 0.8
desktop, fine	other three	16×16, opacity 0.8	unchanged
context menu item	Hide	16×16, opacity 1	unchanged

Full-frame pixel diffs confirm nothing else moved: the mobile frame differs only in one 54×50 region (the duck); the desktop frame differs only in the duck and the animated bb logo, which was cropped and confirmed to be the same geometry at a different animation phase; the context-menu frame differs only in that logo. A sweep of all 26 icon roots rendered on the root view found no zero-sized or oversized icons.

Not covered by manual checks: the dev instance had no threads, so timeline surfaces such as MessageActionBar rest on unit tests rather than a screenshot.

🤖 Generated with Claude Code

AGENT GENERATED