Repository navigation
cmux NEXT webviews: fail hand-set popup roles and raw design values in new code outside ui/ - #33701
Conversation
check-ui-review gains a failing `hand-set-role` rule: role="menu|menuitem*|listbox|option| dialog|tablist|tab" written in TSX outside src/ui/ (tests and gallery exempt). Today's 55 occurrences are allowlisted by file + fingerprint; a new one fails, and a stale entry fails until `bun scripts/check-ui-review.ts --shrink-allowlist` drops it (it never adds). The copy and row rules still only warn. check-design-tokens extends its ratchet to raw colour literals, margin/inset/top/left spacing, border-radius, font-weight, literal durations and easings, in CSS and in TSX inline style objects. Today's counts go in the baseline (every pre-existing row is unchanged); a count may only go down (`--update-baseline` refuses to grow). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-fb75b7af9902231028aa
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-fb75b7af9902231028aa
|
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)
web and size checks: green Shipped in:
|
|
Owned by an agent session · session |
|
Found 1 test failure on Blacksmith runners: Failure
|
|
Review (codex, router-review): SHIP at 0bfb4d4 Reviewer: codex gpt-6.1-sol xhigh, fast tier. Follow-ups: 3. Each lands as an issue in this repo that links this receipt; the lane that opened the PR fixes it forward. SHIP at 0bfb4d4
Follow-ups:
|
Rows from base's own baseline are unchanged; the new kinds pick up what landed since (the next-step card, conversation margins, settings radii). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-fb75b7af9902231028aa
|
Review (codex, router-review): SHIP at 0d32b47 Reviewer: codex gpt-6-astra high, fast tier. Follow-ups: 3. Each lands as an issue in this repo that links this receipt; the lane that opened the PR fixes it forward. SHIP at 0d32b47
The CSS scan matches its baseline. Bun tests could not run because Bun and dependencies are absent. Follow-ups:
|
…mns use it listbox, tablist, their rows and an inline card in src/ui/, so the columns set no popup role by hand (#33701).
… once (#32980) * feat(picker): model menu as columns with an account avatar * test(picker): model menu tests follow the columns * chore(gallery): model menu plays open the columns * docs(whats-new): model menu columns highlight * fix(picker): focus, footer keys, predicted model and type scale * test(picker): gallery and browser checks follow the columns * fix(picker): a picked model carries the chat's effort ultracode carries as extra high; a level picked here carries to the next model. the chip focus waits a task where a page has no frames. * fix(picker): the menu stays put as the model name changes a quiet row packed to its end lines the menu up with the chip's end; the effort column has a fixed width. a reopen keeps the keyboard in the search field. * feat(picker): a model click switches at once; the menu stays where it opened * fix(picker): review fixes a second pick of a model still on its way keeps its level; fast turns off on a model just picked; end alignment looks past the chip's own wrapper. * fix(picker): ci reds for the columns menu the search field takes focus only as it mounts; a star's hover leaves its row as it was; tests follow the columns and the carried level. * fix(picker): design tokens in the columns css * feat(ui): DrivenList, the lists a search field drives; the model columns use it listbox, tablist, their rows and an inline card in src/ui/, so the columns set no popup role by hand (#33701). * fix(picker): the columns show the chat's readiness under the search the old menu's auth note, sign in again and check again (#31689), for every verdict, on every tab and during a search; review by leo.
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
Part of the cmux NEXT design-system pass (Leo: "there's just tons of design systems related stuff for spacing gutters icons etc whatever like we don't yet feel "designed" yet"). This is the lint piece: new code outside
webviews/src/ui/can no longer hand-roll a popup widget or write a raw design value, while today's tree passes on a shrinking allowlist.What changes
1.
hand-set-roleinscripts/check-ui-review.ts(fails)role="menu" | "menuitem" | "menuitemradio" | "menuitemcheckbox" | "listbox" | "option" | "dialog" | "tablist" | "tab"written in TSX outsidesrc/ui/: a JSX attribute (including a conditional such asrole={x ? "option" : undefined}) or arole: "option"property in a props object.querySelector('[role="menu"]')strings, other roles (group,alert,status),src/ui/, tests and gallery files are not flagged.<role>|<tag or props>|<enclosing component>, so moving lines does not churn the allowlist.ui-review-allowlist.jsonl. The checker's own scan generated them.hand-set-roleis the one rule inFAILING_RULES. A new occurrence fails, and so does a stale entry (the code is fixed but the entry is still listed), so the list only shrinks. The copy and row rules still only warn, as before.check:ui-reviewis already incheck:pieces, so this adds no CI job or step.2. Token literals in
scripts/check-design-tokens.ts(fails, same ratchet)The existing per-file baseline ratchet now also counts, in CSS and in TSX inline
style={{}}objects:#hex,rgb/rgba/hsl/hsla/hwb/lab/lch/oklab/oklch/color()var()and its fallback, a colour function orcolor-mixthat takes a token, masks,--x:token definitionsmargin*,inset*,top/right/bottom/left(padding/gap as before)-4pxis fine)border-radius*--cmux-menu-*)bold, and 700normal,var(); 700 for monograms lives in the baselinetransition/animationduration other than 0var(--cmux-motion-*)cubic-bezier(),ease,ease-in,ease-out,ease-in-outvar(--cmux-ease-out),linearpages/shared/desktop.cssand generated CSS are not scanned for the new kinds. Font sizes stay with the existing type-scale ratchet (scripts/cmux-next/check-type-scale.py), which already counts every literal font size; this PR does not duplicate it.How a migration PR shrinks the lists
After moving a surface onto
ui/and tokens, run fromwebviews/:--shrink-allowlistdrops only stale entries and never adds one;--update-baselinerefuses to grow any count. Both checks fail until the fixed entries are removed, so no list ever grows back.Overlap
check-design-tokens.ts(howheightis judged for icon boxes) and the baseline. The two changes are in separate functions; whichever lands second rebases and re-runs--update-baseline.Tests
test/check-ui-review.test.ts: positive (JSX, conditional, props object, every role), negative (other roles, selector strings,ui/, gallery, tests,.ts), allowlisted passes, new fails, stale entry fails until--shrink-allowlist, shrinking never adds, today's tree passes.test/design-tokens.test.ts: each kind positive and negative, var/fallback/token-composing colour, TSX inline styles, ratchet new/allowlisted/shrink.Observed / Not run
cmux-ci test): both test files,bun scripts/check-ui-review.ts,bun scripts/check-design-tokens.ts,vp check --no-fmt --no-lint. 64 tests pass, both checkers pass on today's tree, no type errors.vp fmt --checkclean. No runtime or rendering code changes, so no Performance table and no captures; no new loops, timers or observers.Changelog
none
Agent session: agent session
🤖 Generated with Claude Code
Open in [code]smith · Need help on this PR? Tag
@codesmith-botwith what you need.Summary by cubic
Adds two failing design-system lint rules so new code outside
webviews/src/ui/can't hand-roll a popup widget or write a raw design value.hand-set-roleincheck-ui-review.tsmenu,menuitem*,listbox,option,dialog,tablist, andtabroles set in TSX outsidesrc/ui/(tests, gallery, andui/files exempt).bun scripts/check-ui-review.ts --shrink-allowlistdrops stale entries and never adds.Token literals in
check-design-tokens.tsstyle={{}}objects.bun scripts/check-design-tokens.ts --update-baselinere-records counts and refuses to grow any, so a count can only shrink.Written for commit 0d32b47. Summary will update on new commits.