fix(ui): render message action rows with Astryx Icon and IconButton

[Astro-Han]

Summary

The action rows under user and assistant messages used Astryx IconButton and ChatMessageMetadata for the shell, but everything inside them was product-owned: lucide glyphs at a product 14px rung, styles.css resetting the button's border, radius, ink and pending/copied colours, disabled and busy expressed as aria-disabled + data-* attributes, a meta "button" whose click did nothing, and a positional selector that deleted Astryx's timestamp separator on the user side.

This PR makes the rows use the component contract as designed:

• Every glyph goes through Astryx Icon at size="sm": copy / check by semantic name, regenerate / branch / edit as SVG components. No pixel constant on these call sites.
• IconButton keeps its own border, radius, ink, hover, focus, disabled and busy treatment. isDisabled replaces the aria-disabled + click-guard pair (Astryx keeps a tooltip'd disabled button focusable itself); isLoading replaces data-pending / aria-busy for copy-in-flight and regenerate/branch-pending. The .maka-turn-footer-action geometry and colour overrides are gone.
• The turn meta (model · duration · cost) is rendered as text in the assistant footer row instead of an info pseudo-button with a tooltip. The info action id and its three locale labels are removed; the text is derived inside TurnView from the turn's own fields, so the ratcheted renderer view model loses the derivation instead of growing.
• Both senders share one reveal rule keyed on .maka-chat-message: time and delivery status stay visible, actions and meta surface on hover / focus-within. The user row's time and delivery status move into the footer slot so Astryx never draws a separator and the > span + span positional hide is deleted.

Left as is: the inset focus ring on footer actions (a scrollport clipping workaround with its own note), and the lineage badges and host-origin markers, which are not action rows.

Fixes #5133
Refs #5132 for the product-model questions this PR deliberately does not touch (which actions exist and when they are enabled).

Verification

• @maka/ui build, chat-turn-answer-identity.test 16/16 (copy feedback tests now assert aria-busy instead of data-pending), desktop renderer + storybook typecheck, renderer architecture check against a9b4f21af (ledger refreshed: the turn view model drops from 1395 to 1273 tokens), npm run format, npm run lint.
• Storybook product-shell-official-appshell--native-conversation built from main a9b4f21af (BEFORE) and this branch (AFTER), same 1280×900 viewport, light and dark, hovered so the rows are revealed. DOM checks on AFTER: glyph 16×16 inside a 28×28 IconButton, border-radius from Astryx (10px), no border, no [data-action="info"], meta text present in the footer.
• Not run: Electron E2E specs that assert the 重新生成 button by role and name (unchanged accessible names).

AI use

Select exactly one:

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

Tool(s) and scope: Claude Code (Fable 5.1) — audit, implementation, tests, and Storybook before/after captures; reviewed by the author.

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

[jackwener]

APPROVE, bound to 72cce24e7960f002580b308e1ca37047e92c08cf. One P3 inline; nothing P0–P2. Hosted test and label are terminal green on this exact head, which has not moved.

Taking the review focus points in order, with what I actually checked.

Astryx Icon does take over sizing, and the product constant is gone from these rows. All six action-row icons now render through <Icon … size="sm" />, and size="sm" resolves to 1rem in Astryx's own token table — so the glyph is 16×16 at a 16px root, from the design system rather than from ICON_SIZE.control. ICON_SIZE survives in this file only for meta badges, the elapsed timers and a system message, none of which this PR claims to convert; worth stating precisely rather than as "no constants remain".

isDisabled and the old click guard are equivalent, and isLoading really does prevent a second click. This was the one point that could have been a reachable defect, so it is the subject of the inline P3 — the equivalence holds, but it now rests on Astryx's handleClick rather than on anything this repo pins.

Delivery status and actions keep their order under row-reverse. ChatMessageMetadata renders {footer != null && footer} without a wrapper, so the fragment's children become direct flex children of a row that is row-reverse for the user variant. The moved element is the fragment's first child, exactly where the timestamp slot used to sit, and the separator that stood between them is gone — so the sequence is identical to before, not merely similar.

That separator is genuinely gone rather than hidden differently, which is what makes deleting the positional selector safe. Astryx emits it only under timestamp != null && (footer != null || statusConfig != null); with nothing left in the timestamp slot the condition is false and no · is produced. .maka-message-meta > span + span { display: none } had no target left.

The live footer is unaffected, and for a structural reason. The previous CSS put opacity: 0 on .maka-turn-footer itself and therefore needed .maka-turn-footer[data-live-streaming="true"] { opacity: 1 } as an exception. Moving the fade down to .maka-turn-footer-action / .maka-turn-footer-meta means the activity row — which carries neither class — is simply never faded. The exception is not preserved; it is no longer needed.

The unified hover rule reaches both senders. .maka-chat-message is on the user wrapper (maka-chat-message maka-user-message) and the assistant wrapper (maka-chat-message maka-assistant-answer), and TurnFooter renders inside the latter as a sibling of .maka-assistant-answer-content — so .maka-chat-message:hover .maka-turn-footer-action matches on both sides. I checked this because replacing a per-sender selector with a shared ancestor is exactly where one side silently stops matching.

The aria-busy assertion keeps its discriminating power — verified by mutation, not by watching it pass. Forcing isLoading to false in the compiled TurnFooter fails footer copy preserves raw text, blocks overlapping writes and resets success after 1400ms with actual: null, expected: 'true'. The switch from data-pending to aria-busy therefore still fails when the loading state is unwired; it did not become a tautology.

On the before/after captures: I cannot verify image content, and I did not build Storybook. Two of the DOM claims made alongside them are checkable from source and hold — [data-action="info"] cannot exist because info is removed from both TurnFooterActionId and deriveTurnFooterActions, and the meta text is rendered as .maka-turn-footer-meta in the footer. The 28×28 control box and the 10px radius I did not confirm.

Scope and limits. Static review of all eight files plus the Astryx Button, Icon and ChatMessageMetadata sources that now own the removed behaviour; a local packages/ui build used to run and mutate chat-turn-answer-identity. I did not run the desktop suite, did not build or inspect Storybook, and did not exercise a real browser — so the ordering and visibility conclusions above come from reading the layout rules and the vendor components, not from rendered geometry. This is a single review line.

简体中文

绑定 72cce24e7960f002580b308e1ca37047e92c08cf,行内一条 P3,无 P0–P2。两项托管检查在此 exact head 终态为绿,head 未移动。按你列的重点逐条回答。

Astryx Icon 确实接管了尺寸,产品常量在这些行里已消失。 动作栏六处图标全部走 <Icon … size="sm" />,而 size="sm" 在 Astryx 自己的令牌表中解析为 1rem —— 即 16px 根字号下的 16×16,来自设计系统而非 ICON_SIZE.control。ICON_SIZE 在本文件中仅存于 meta 徽章、计时器与一条系统消息上,这些并非本 PR 声称要转换的范围;应当这样精确表述,而不是笼统说"已无常量"。

isDisabled 与旧的点击守卫等价,isLoading 也确实阻止二次点击。 这是唯一可能成为可达缺陷的一点,故行内 P3 正是针对它 —— 等价性成立,但它现在完全依赖 Astryx 的 handleClick,而本仓库没有任何东西钉住它。

row-reverse 下投递状态与动作的顺序不变。 ChatMessageMetadata 以 {footer != null && footer} 直接渲染、不加包裹,故 fragment 的子元素成为该行的直接 flex 子项;被移动的元素正是 fragment 的第一个子项,恰好是原 timestamp 槽的位置,而两者之间的分隔点已不存在 —— 所以顺序与改动前完全一致,不只是相近。

那个分隔点是真的没有了,而不是换了种方式藏起来 —— 这才使删除位置选择器是安全的。Astryx 仅在 timestamp != null && (footer != null || statusConfig != null) 时才输出它;timestamp 槽既已清空,条件为假,· 根本不产生,.maka-message-meta > span + span 已无作用对象。

live footer 不受影响,而且原因是结构性的。 旧 CSS 把 opacity: 0 加在 .maka-turn-footer 自身,因此必须再补 [data-live-streaming="true"] { opacity: 1 } 这条例外;把淡入下放到 .maka-turn-footer-action / .maka-turn-footer-meta 之后,活动行两个类都不带,自然从不被淡出。那条例外不是被保留,而是不再需要。

统一后的 hover 规则两侧都覆盖到。 .maka-chat-message 同时存在于用户外层与助手外层,且 TurnFooter 渲染在助手外层内部、与 .maka-assistant-answer-content 同级。我专门核这一点,是因为"把按发送方分开的选择器换成共同祖先"正是某一侧会静默失配的典型位置。

aria-busy 断言仍有区分力 —— 用变异验证,而非看它通过。 在编译产物中把 TurnFooter 的 isLoading 强制为 false,footer copy preserves raw text… 立即失败,报 actual: null, expected: 'true'。故从 data-pending 换到 aria-busy 并未变成恒真断言。

关于前后对比图:我无法验证图片内容,也没有构建 Storybook。与之并列的两条 DOM 断言可从源码核实且成立 —— [data-action="info"] 不可能存在(info 已从 TurnFooterActionId 与 deriveTurnFooterActions 中一并移除),meta 文本确以 .maka-turn-footer-meta 渲染于 footer。28×28 控件盒与 10px 圆角我未确认。

本次审查的边界:对八个文件的静态审查,外加对 Astryx Button、Icon、ChatMessageMetadata 三处源码的阅读(被移除的行为现由它们承担);本地构建 packages/ui 以运行并变异 chat-turn-answer-identity。未运行 desktop 套件、未构建或查看 Storybook、未在真实浏览器中验证 —— 因此上述顺序与显隐结论来自布局规则与厂商组件的阅读,而非渲染几何。这是单独一条评审线。

Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.

[jackwener]

[P3] Both JS click guards were removed in this change, and nothing in this repository pins the behaviour they provided.

Two guards disappeared together:

• here, onClick={() => { if (props.editDisabled) return; props.onEditUserMessage?.(); }} became onClick={() => props.onEditUserMessage?.()};
• in TurnFooter, handleClick lost its opening if (!action.enabled) return;.

The behaviour is correct today, and I checked it rather than assuming it. Astryx's Button.handleClick returns before ever calling the consumer's handler:

if (buttonDisabled || (actionInFlightRef.current && !isInterruptible)) {
  e.preventDefault();
  return;
}
props.onClick?.(e);

with buttonDisabled = isDisabled || groupDisabled || (isLoadingState && !isInterruptible) and isInterruptible = false by default — which this file does not override. So isDisabled blocks the click, and isLoading blocks it too, which is what keeps a pending regenerate or branch from firing twice. Keyboard activation is covered separately: handleKeyDown suppresses Enter and Space whenever useAriaDisabled is in effect.

That last part matters here, because every one of these buttons passes tooltip, and useAriaDisabled = tooltip != null && buttonDisabled means the native disabled attribute is deliberately not set — the element stays focusable so a keyboard user can reach the tooltip that explains why it is unavailable. A reviewer who stops at "aria-disabled, so it is still clickable" would call this a regression; it is not.

What is missing is a local guarantee. The correctness now rests entirely on a vendor implementation detail. No test in packages/ui/src/__tests__ clicks a disabled or pending message action and asserts nothing happened — the aria-disabled assertions in the composer tests cover a different surface, and chat-turn-answer-identity asserts aria-busy is set without ever exercising a click while it is. An Astryx upgrade that let aria-disabled buttons fire onClick, or a later edit adding isInterruptible, would restore the double-fire this change is careful about, and the suite would stay green.

Minimal fix: one test that clicks a pending footer action and asserts the handler did not run — the existing copy test already reaches the pending state, so it is a click and one callCount() assertion away.

Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.