Repository navigation
cmux NEXT webviews: icons draw on one scale (12/14/16/20), with a ratcheting icon-scale check - #33595
Conversation
…on-scale check
Leo-ask: ask-65aae6c0e2
- New scripts/check-icon-scale.ts (bun run check:icon-scale) and its test: a
component `size={N}`, a square inline svg, or a square CSS icon box off the
icon scale (design/grid.md) fails unless it says icon-scale-allow: <reason>
or is in scripts/icon-scale-baseline.tsv, which only shrinks.
- Moves 20 off-scale icons onto the scale: 13 -> 14 (diff viewer pickers,
menus, New Tab search), 15 -> 16 (file icon, model picker star),
10/11 -> 12 (chevrons, close marks, checks, the stepper's +/-).
- check-design-tokens judged every icon `height` as a control height
(20/24/28/32), so icons on the icon scale read as debt; an icon box's height
is now judged by the icon scale, and its baseline shrinks by those rows.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-521afe665035bd125dcb
Bugbot needs on-demand usage enabledBugbot uses usage-based billing for this team and requires on-demand usage to be enabled. A team admin can enable on-demand usage in the Cursor dashboard. |
|
Review (subagent claude-opus-5-5 medium): SHIP at 408bdfe Checked the checker's regexes (min-width/stroke-width don't match, Visual verdict: no images; +1/+2 px icon sizes only [follow-up]. |
…-system-grid-debt # Conflicts: # webviews/scripts/design-tokens-baseline.jsonl
Bugbot needs on-demand usage enabledBugbot uses usage-based billing for this team and requires on-demand usage to be enabled. A team admin can enable on-demand usage in the Cursor dashboard. |
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
Gallery before/after for
States
Pick other states with a Shipped in:
|
|
Review (codex gpt-6.1-sol high, sweep, via the review router; full diff from merge base 5dee1ab to this head): SHIP at a571f2f [P2] [follow-up] |
|
Review (subagent claude-opus-5-5 medium): SHIP at a571f2f This head is a base merge only. Every file except the regenerated The gallery health "own reds" ( Visual verdict: no images; the icon sizes change by +1 or +2 px only [follow-up]. |
|
Found 1 test failure on Blacksmith runners: Failure
|
|
Review (next-review patch-id; carried): SHIP at 7c3e517 The independent SHIP at a571f2f carries to this head: the PR's own change ( |
|
Owned by an agent session · session |
Conflicts: newtab/screen.css takes base's --grid-glyph chevron size (12px);
design-tokens baseline regenerated (shrink only). ChiefWorkPanel.tsx's
size={10} icon from base is recorded in the icon-scale baseline.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-6cb858a4b67b21dba9a3
|
Review (subagent claude-opus-5-5 medium): SHIP at 4f0787f Checked the base merge only: the PR's diff vs base is unchanged apart from newtab/screen.css dropping out, because base already sizes |
|
Review (codex, router-review): SHIP at 4f0787f Reviewer: codex gpt-6-astra high, fast tier. Follow-ups: 2. Each lands as an issue in this repo that links this receipt; the lane that opened the PR fixes it forward. SHIP at 4f0787f Visual verdict: no introduced visual regression in the 16 supplied before/after pairs.
Follow-ups:
|
…+ draws at 12px (#34494) * base red: Settings' progress fill is not a surface; the model menu's + draws at 12px render.test.tsx's 'no rule declares a background other than transparent or the two interaction tokens' failed on .app-update-progress's value fill (var(--text)), added by #33176: a progress bar's value is not a container, so the test now exempts only the progress-value pseudo-elements. check-icon-scale failed on QuickModelColumns.tsx's PlusIcon at 11px (#32980, which landed an hour after the check, #33595). 12 is on the scale, matches its viewBox and the CheckIcon beside it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-dfc0ecf416bf21757d6b * render.test.tsx: formatter layout Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-dfc0ecf416bf21757d6b --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
Leo-ask: ask-65aae6c0e2
This PR handles the icons part of that ask. Icons are the one design-system scale with no check. The spec (
design/grid.md, "Icon sizes") puts icons at 12, 14, 16 or 20 (artwork at 24 or larger), but nothing enforced it, so the webviews still drew icons at 13, 15, 10 and 11 px. Icons that should match then sit 1–2 px apart.What changes
webviews/scripts/check-icon-scale.ts,bun run check:icon-scale, withtest/check-icon-scale.test.ts). It reads three ways an icon size is set: a componentsize={N}, a square inline<svg width height>, and a square CSS box whose selector names an svg, icon, glyph, chevron or spinner. A size under 24 that isn't 12, 14, 16 or 20 fails unless its line saysicon-scale-allow: <reason>orscripts/icon-scale-baseline.tsvrecords it. Like the design-token ratchet, the baseline can only shrink. Dots (::before/::after), buttons,<input size>, gallery files and prototypes are skipped.check-design-tokens.tsjudged everyheightas a control height (20/24/28/32), so a 14 or 16 px icon counted as debt and an off-scale 13 px icon looked no worse. Now an icon box's height is judged by the icon scale. Its baseline loses those false rows (about 30 entries across 13 files).pages/chief-views), the AppKit-metric 6 px menu chevrons, the 8 px close marks in attachment chips (they sit in tight corner badges and need a visual pass first), and the remaining 18 px harness and artwork marks.design/grid.md(leo-workspace) gets a line pointing to the check.Risk
Low. The change is CSS and SVG size attributes on icons (+1 or +2 px each), plus a new lint and a lint fix. No logic changes. Per the cmux rule against new CI gates,
check:icon-scaleis a package script and is not added tocheck:pieces; its regression test runs with the webview tests.Changelog
none
Observed / Not run
bun test test/check-icon-scale.test.ts test/design-tokens.test.ts test/check-menu-gutters.test.ts(19 pass),bun scripts/check-icon-scale.ts,bun scripts/check-design-tokens.ts(pass after--update-baseline, which only shrank rows),scripts/web-lint.sh(pass; this host skips the type-awarevp lint/vp check, so CI runs those), andprepush-gate.py(no new violations).🤖 Generated with Claude Code
Agent session: agent session
Open in [code]smith · Need help on this PR? Tag
@codesmith-botwith what you need.Summary by cubic
Draws webview icons at one scale (12, 14, 16 or 20 px; artwork 24+) so matching marks that previously sat 1–2 px apart now align, and adds a lint that enforces the scale.
check:icon-scalescript fails any icon under 24 px that isn't 12, 14, 16 or 20 unless allowed inline or recorded in a baseline that only shrinks; it's not added to the CI gate, and its test runs with the webview tests.check-design-tokensso an icon box's height is judged by the icon scale instead of the control-height scale (20/24/28/32), removing false design-token debt rows.The feat-cmux-next merge keeps the newtab chevron on the base's 12 px size and records ChiefWorkPanel's 10 px icon in the baseline, so no delivered code sits off the scale.
Written for commit 4f0787f. Summary will update on new commits.