Skip to content

fix(ui): stop overriding Astryx's chat bubble radius - #3452

Open
Astro-Han wants to merge 1 commit into
mainfrom
fix/chat-bubble-radius-astryx-default
Open

fix(ui): stop overriding Astryx's chat bubble radius#3452
Astro-Han wants to merge 1 commit into
mainfrom
fix/chat-bubble-radius-astryx-default

Conversation

@Astro-Han

@Astro-Han Astro-Han commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Summary

The bubble and the composer are designed to share one radius token. ChatMessageBubble defaults to var(--radius-chat) (28px); ChatComposer reads the same token via --_chat-composer-radius. The product overrode the bubble twice — density="compact" dropped it to 12px, then .maka-chat-message-bubble-user pushed it to 10px — and left the composer alone.

image image

Full view, light (top) then dark (bottom). Story product-shell-official-appshell--native-conversation, 1180×788 @2x, scrolled to top.

image image

Corner zoom on the bubble/dock join, light (top) then dark (bottom). Same story and conditions. The AFTER bubble sits ~32px lower: the balanced tier lengthens the transcript.

Three deletions, no new value. 28px is what the component picks on its own once both overrides are gone.

density is a spacing tier, so dropping it also moves ChatMessage's children gap 2px → 4px and the transcript reads looser — bundled with the tier, intended. Bubble padding is not affected: Astryx's paddingCompact and paddingBalanced resolve to the same StyleX atoms.

DESIGN.md §6 moves in the same commit because it contradicted the change: chat bubbles leave the 10px card row, a 28px chat rung records what --radius-chat governs, and the Two-Name Rule no longer claims nothing in the product renders at 28px — ChatComposer already did, on main.

Refs #3446 — same root as its F1 finding, separate intent, none of its other work folded in.

Verification

Playwright computed-style sweep against a live Storybook dev server, before and after, same branch and same server. Page pinned, scrollTop 0, identical framing. Light and dark — identical results in both.

before after
user bubble 10px 28px
assistant bubble 12px 28px (ghost, transparent)
user bubble padding 12px 16px 12px 16px
composer drawer 28px 28px 0 0 28px 28px 0 0
list density compact balanced

Full-page sweep of every visible element's radius, font-size/line-height, colour and gap. Everything that moved is accounted for, nothing else:

  • radius 10px ×35→32, 12px ×3→1, 28px ×1→6 — distinct radii stay at 10, no new value in the ladder
  • gap 2px ×11→5, 4px ×22→27 — ChatMessage's childrenGap, the tier's own spacing
  • 435→431 visible elements — signature diff shows every other element is the same element with compact renamed to balanced; the four that leave are the last message's tail, pushed below the fold as scrollHeight grows 1473→1505px
  • no font-size/line-height pair or colour value added or removed

density="compact" on <Markdown> (chat-turn.tsx:1107) is left alone — a different density axis with its own [data-density="compact"] contract.

Ran: npm run format (no fixes), @maka/ui tsc build (clean), focused tests chat-turn-answer-identity / chat-turn-steering-order / chat-conversation-items / transcript-projection (23 pass, 0 fail). Not run: repo-wide suite (CI), desktop Storybook visual smoke.

Review focus

packages/ui/src/chat-surface-layout.tsx:35 says chat-surface-layout.test.tsx holds the balanced density value. That file does not exist in the repo — the ChatLayout density default is currently unguarded. Out of scope here, worth a follow-up.

DESIGN.md §6 is a deliberate revision of the design authority, written as a rule rather than an exception. Please review that wording as policy.

AI use

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code — the deletions, the DESIGN.md §6 revision, the CSS comment rewrite, the Playwright measurement harness, and this body. Every number above comes from that harness against a live server, not from reasoning about the cascade. Commit carries a Generated-by: Claude Code trailer.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

`ChatMessageBubble` resolves `border-radius: var(--radius-chat)` (28px), the
same token `ChatComposer` reads through `--_chat-composer-radius`. Bubble and
composer are designed to round together as one conversation surface.

The product diverged from that twice. `density="compact"` on both
`ChatMessageList` instances swapped the bubble's radius for
`var(--radius-container)` (12px), and `.maka-chat-message-bubble-user` then
pushed the filled bubble down to `var(--radius-surface)` (10px). The composer
was never overridden, so it kept rendering at 28px — a transcript and a dock
on the same surface differing by nearly 3x.

Remove both overrides rather than introduce a third value. 28px is what the
primitive picks on its own once the product stops arguing with it.

Dropping `density="compact"` also moves `ChatMessage`'s children gap from 2px
to 4px, which is the tier's own spacing and comes bundled with it. Bubble
padding is unaffected: Astryx's `paddingCompact` and `paddingBalanced` resolve
to the same StyleX atoms.

DESIGN.md §6 moves in the same commit so the document does not contradict the
code: chat bubbles leave the 10px card row, a 28px chat rung records what
`--radius-chat` governs, and the Two-Name Rule no longer claims that no product
consumer renders at 28px.

Generated-by: Claude Code
@Astro-Han
Astro-Han marked this pull request as ready for review August 21, 2026 19:17

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review of exact head 1a99e766ef824bc69d0f5a2548df6257c7cc1084 against current main@d62857a8357e9160926726a2a13096bc2dc2b91d.

The defect remains on main: both ChatMessageList call sites force compact, which activates Astryx's compact bubble radius, and product CSS then overrides the filled user bubble again to 10px while the composer independently stays on --radius-chat (28px). The patch removes both competing product decisions and lets the shared primitive/token own the shape; the measured padding result also confirms this is not compensating with a new value. I found no actionable P0-P2 defect. Required test is green and the PR includes before/after visual evidence.

Required conclusions:

  1. Optimal for the actual problem: yes; deleting the two overrides restores the existing Astryx source of truth.
  2. Production code to delete: the PR already deletes the unnecessary density="compact" props and product radius override; none further identified.
  3. Tests to delete/replace: none identified.
  4. Deeper refactor: no.
  5. Ready to merge: code-wise yes, but this automated review is not approval; the visual/design-policy change still needs independent human review.
  6. Residual risks/gaps: no automated computed-style regression protects the 28px pairing, so the supplied visual evidence and a human check are the current guard; future Astryx changes could move the default.

This affects user-visible layout and DESIGN.md policy, so CONTRIBUTING.md requires independent human judgment on the exact head.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants