Uh oh!
There was an error while loading. Please reload this page.
fix(ui): move the single-line chip family onto Astryx Badge - #1881
Merged
Conversation
Astro-Hanforce-pushed
the
fix/chips-pin-box-height
branch
2 times, most recently
from
August 2, 2026 07:13
5fdb294 to
1db0e8eCompareSquashed for rebase onto #1893; final message written at the end.
Astro-Hanforce-pushed
the
fix/chips-pin-box-height
branch
from
August 2, 2026 10:14
38aae0f to
5263398Compare… exemption Review of the previous revision found three ways the guards were narrower than their own documentation, all of the same shape: a check that reports green because it cannot see the thing it claims to govern. - `isChip` required a rule to declare its own type. A compound chip delegates type to a child and a simple one may just inherit, so the arm was hiding three shipping chips with the #1879 defect while reporting the other 15 green. Two arms — pill radius and padding — see all 20 pill boxes, and no longer read a vocabulary that #1893 already moved once, so the unanchored `chips.length >= 8` floor that watched for exactly that drift is deleted with it. - The exemption set was one list honoured by skipping, so a mutation adding `height: 1px` to a rule excused as component-sized stayed green. It is now three reason-groups, each asserted: WRAPS must declare no block size (and is the wrap contract's own list rather than a second copy of it), COMPONENT_OWNED must declare none at all, PINNED_OFF_RULER must still be pinned. - The Badge contract read the unconditional view, on which `.settingsHealthBlockerBadge` — which lives entirely inside a breakpoint — merged to nothing, so the exemption naming it suppressed nothing. It reads a conditional-inclusive view now and asserts that the release is real. Also: `min-height`/`max-height`/`padding-inline-start` all constrained a box while passing both checks; the ruler accepted tiers it does not define; and the JSX scanner ended a tag at the first `>` in a prop, and then, once rewritten, at an apostrophe inside a prop comment — which silently dropped three live call sites until the mutation sweep caught it. Four boxes the widened scan surfaced, each measured live before its tier: `.maka-quote-chip-collapsed` and `.maka-deep-research-run-count` at 24px natural (pixel-neutral on `--h-control-sm`), `.maka-firstrun-step` at 29px (`--h-control-lg`, +3px), and `.maka-mcp-install-button`, a bare 34px square that swaps with a 28px `Button size="sm"` in the same slot.
…ip contracts Every finding below was reproduced as a mutation that passed green, and each fix is re-verified by the same mutation failing. Four root causes, three of which delete something. The predicate read text, not declarations. `/padding/` also matches `background-clip: padding-box`, which is how a scrollbar thumb — padding-less, and a box no reader perceives as an object — entered the chip population and earned an exemption group of its own. The group is deleted with the substring that created it, and the padding vocabulary becomes one shared authority. The same guard-narrower-than-CSS mistake, twice more. The Badge contract's `OWNED` gained the logical padding family and lost `padding-top/right/bottom/ left`, so `padding-left: 9px` on a Badge className redrew the component's box while green. And the off-ruler exemption proved "still pinned" with the constrains-a-block-size vocabulary, so rewriting its `height` as `min-height` handed the box back to the line box and stayed green. Which merge view follows from which question, applied to all three sites rather than one. A must-NOT contract has no conditional distinction: pinning a wrapping row, or re-adding the height override #1879 removed, inside `@media (max-width: 620px)` passed the unconditional view. A must-HOLD PAIR needs the opposite — the flattened view let a Badge release `height` in one breakpoint and `white-space` in the mutually exclusive one, releasing the box at no viewport at all, so releases are now read per cascade context. A scan that cannot read a call site has to say so. `<Badge {...{ className: 'x' }} />` is legal JSX that the static scan cannot read and the `className={` scan does not match, so both contracts concluded there was nothing to govern. Spreads join computed classNames as unreadable; `//` prop comments parse. (TypeScript 7 is the Go port and ships no JS compiler API, so an AST is not available to reach for — but the property that matters is loud-vs-silent, not lexer-vs-parser.) Three smaller ones: rungs are read from the ruler's own `:root` scope, so a `--h-control-*` declared under `.dark` is no longer a tier a chip may name; `--radius-pill` must exist, because renaming it reported CHIP POPULATION 0 and passed; `.maka-sidebar-update-button` returns to COMPONENT_OWNED, where an entry that was dead weight as a skip is the only assertion reaching it. The e2e no longer credits the tier-choice arithmetic to a check that does not exist. Measured with a throwaway probe: all four newly pinned chips resolve to zero nodes in both booted fixtures, so that choice is unasserted, and the comment says so. Verified: 10/10 mutations caught, each confirmed to land before being trusted. lint, format:check, typecheck, desktop 1364, ui 241, e2e 77, check-dead-css.
Astro-Han
marked this pull request as ready for review
August 2, 2026 11:46
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes#1879.
The issue asked for pinned box heights so a
line-heightchange can no longer move layout. Pinning was the right diagnosis and — for most of the family — the wrong owner. Astryx already shipsBadge, and its box IS the recipe these chips hand-wrote: 20px off--spacing-5, pill radius, supporting tier,align-items: center,nowrap. Twelve chips are<Badge variant=...>at the call site now, and their height, chrome and per-state colour left product CSS with them. Net +347 / −304 across 27 files: the conversion pays for the guards.What stays product CSS is what Badge does not do — a chip with no chrome at all (
.maka-message-time-inline,.maka-nav-kbd), one that truncates (.maka-skill-governance-summary span), and the compound permission pill that holds a Badge inside it. Those keep a--h-control-*pin.Three defects the first revision shipped
Eleven pins sat below their own arithmetic floor. The floor rule (line box + padding + border, under
border-box) was applied by hand in four CSS comments and skipped everywhere else. Measured live, those chips were 2–4px shorter than they read and their declared vertical padding was vertically inert:Not a clipping bug — the line box centres and the ink fits, verified including CJK — but an undocumented shrink. The rule now lives in the e2e, not in prose: a chip's
height: autonatural height must fit its pinned tier.The derived scan was a narrower whitelist. Three independent bypasses, all now closed and mutation-verified:
.plan-proposal-revisionwas invisible, five lines from a chip the scan caughtheight: var(--h-control-xs); … height: autopassed both checks while the browser saw no pinparseCssBlockskeyed multi-line selector lists by their last lineThe containment assertion was inert twice over. It read
scrollHeight/clientHeightafter restoring the stressed leading, and on anoverflow: visiblebox the two are equal regardless. Replaced with the floor measure, which can actually fail.Astryx layering
--h-control-md/lg/xlnow derive from--size-element-sm/md/lginstead of restating 28/32/36, so a chip and the Astryx control beside it can no longer agree by coincidence. Verified live:md→ 28,lg→ 32..astryx-selectorrules were forcing an AstryxSelectorto 24px — below its own smallest tier (--size-element-sm= 28) — and desyncing it fromModelChipStatic, the inert state of the same chip, aButton size="sm"at 28px. The pill changed height when it became clickable. Both declare no height now. Their contract exemption previously read "declaring a height here would reintroduce the override fix(ui): pin the box height of single-line chips so leading cannot move them #1879 removed" while the rule declared exactly that; the reason is true of every entry under it now..maka-sidebar-update-buttonused thebackgroundshorthand, which also resetsbackground-image— where Astryx ghost keeps its hover and pressed overlays. Thevariantthe component was passed had been silently cancelled. Radius moves to--_button-radius; four declarations that restated Button's own computed values are gone.What was tried and reverted
Widening the scan to
--radius-controlso squared badges were covered: it reported 27 offenders includingpreblocks, failure banners, sidebar rows and buttons — every one a box a pin would clip.--radius-controlis this repo's general control shape, not a chip signal. The squared chips that motivated it areBadges now, so that population is empty rather than unguarded.Verification
npm run lint0 ·npm --workspace @maka/desktop run typecheck0 · desktop tests 1325 pass · ui tests 239 pass · e2e 73/73 · contract 15/15 ·check-dead-css --checkclean ·astryx:theme --checkcurrent.npm run format:checkfails onscripts/check-console.mjs, which this branch does not touch (git diff main...HEAD -- scripts/check-console.mjsis empty) — inherited from #1880.Every guard mutation-verified with
cpbackup/restore:height: var(--h-control-xs); … height: autodisplay: blockheightadded to a wrapping boxReceived: 28vs ≤ 20All restored green. The first attempt at the floor mutation was ineffective — the injected
padding-blockwas overridden by a laterpadding: 0in the same rule — and was redone rather than counted.Live evidence
Plan reminders: the countdown chip is now the same object as the
已触发/已暂停badges it sits beside, which is the convergence the component swap buys.Permissions:
影响功能is a neutral Badge; the status chips route through the samestatusBadgeVariantseam as the rest of Settings.Review focus
The visual direction was chosen against a side-by-side render of every affected chip in both colour schemes. Two deliberate appearance changes worth a second opinion:
.providerCatalogBadgehad a comment reading "compact squared corners, never pills". Badge hardcodes--radius-fullwith no radius prop, so that intent is retired rather than kept as an override.grantedreadssuccesslike every other status badge in Settings.