fix: refine chat message chrome - #212
Merged
Merged
Conversation
Contributor
Author
|
辛苦看一下：）@Astro-Han |
Contributor
|
来啦！ |
Contributor
|
几个非阻塞的小建议，标了下优先级：P2 是建议修，P3 有空再弄，都不挡合并。 [P2] 角色识别：头像和名字拿掉之后，屏幕阅读器用户不太容易判断每段是谁说的，system 提示也会跟助手回答混在一起。要不要给每条消息的容器补一个 [P2] 助手消息的复制入口：之前助手气泡悬停时右上角有复制按钮，这版换成时间戳了。底部 [P2] 时间戳只在 hover / focus 时显示：用户消息这边没有可聚焦的子元素，触屏和纯键盘用户基本看不到时间。如果时间是想让人看到的，可以考虑给一个不依赖 hover 的路径。 [P2] 小字的可读性： [P3] footer 里 retry / regenerate 的优先级：这次从 primary 调成了 secondary。这个改动跟 PR 主题（消息外观）关系不大，如果是有意的，建议在描述里说一句，或者单独拆出来，免得 review 时被忽略。 [P3] 顺手的清理：删除 整体方向很赞，这些都是细节。先合并了，我开个 follow-up 把这几条一起修掉。辛苦啦 🙏 |
Contributor
Author
|
感谢认真 review并指出这些细节😊，我会尽快修复并更新：） |
Astro-Han
added a commit
that referenced
this pull request
Jun 24, 2026
* fix(chat): refine message chrome follow-up to #212 Follow-up to #212 addressing the review notes plus the design pass the owner approved in-session. User messages: - Re-tint the user message into a width-capped block (--chat-user-bg, 10px radius) anchored right, so the right-alignment reads even for long messages — transparent right-aligned text did not. - Add an always-visible time + a copy affordance (reusing the existing MessageCopyButton) in a meta row beneath the block. Readability + a11y: - Timestamps are no longer hover-gated (were opacity:0 until hover, hidden on touch + from assistive tech); they render quiet at the 12px chrome size. - Unify the chrome type scale: summary, footer, and time all sit at 12px (was a mix of 9-10px). The turn summary collapses to one quiet caption line (model · tools · duration · tokens) instead of a cluster of tiny pills. - Footer actions render uniformly (icon + label, 28px min target). This supersedes the PR-UI-17 primary/secondary label-collapse, which after #212 had also reclassified retry/regenerate to secondary (undeclared) and made copy hard to find. - Add aria-labels to the user/assistant message containers. - Looser rhythm: 24px between turns, 8px within a turn. Cleanup: - Remove dead helpers avatarInitial / messageRoleLabel (zero call sites after MessageMeta was dropped) and the now-unused MessageTimeInline. * test(visual-smoke): render seeded chat fixtures + honor theme override The visual-smoke capture path could not verify chat-surface PRs. The renderer reads turns via runtime.getMessages (the RuntimeEvent read-model projection), but fixtures seed session.jsonl directly without a RuntimeEvent ledger, so getSessionView returned an empty view and capture rendered the empty hero instead of the seeded conversation. Separately, refreshShellSettings re-applied the persisted 'auto' theme over the fixture's MAKA_VISUAL_SMOKE_THEME override (a race), so light variants captured dark. Both fixes are guarded by the visual-smoke fixture and have zero effect on real users: - sessions:readMessages reads from the on-disk session store when a fixture is active, so seeded turns render. - refreshShellSettings prefers the visual-smoke theme override when one is present (getState() returns null for real users). This unblocks before/after screenshots for the chat chrome change in this PR. It is a self-contained harness fix and can be split into its own PR. * fix(chat): reveal user-message copy + space the meta row Review of the AFTER screenshot caught two regressions in the user-turn meta row: - The copy affordance never rendered. `.maka-message-copy` is absolute-positioned + opacity:0, revealed only inside a hovered `.maka-bubble-with-actions`. The user meta row is not that wrapper, so the button was permanently invisible and anchored to the wrong box. Scope an inline, always-visible override for the meta-row copy. - The relative time hugged the bubble. `.message.user` is a flex column with no gap, so the meta row sat flush against the block. Add a 6px gap (below the 8px turn rhythm, so the meta still reads as belonging to the bubble). * fix(chat): unify user-message copy with the assistant footer copy The two copy affordances had drifted into different control families: the assistant footer copy is a UiButton ghost action (icon + "复制", 8px radius, foreground-50) while the user-message copy was an icon-only 22px circle. Render the user copy through MessageCopyButton's new `footerStyle`, which reuses the same UiButton primitive + the shared `.maka-turn-footer-action` class and 12px icon metrics — so the two copies are identical by construction rather than a look-alike override. Drops the now-dead bespoke meta-row CSS.
Closed
Astro-Han
added a commit
that referenced
this pull request
Jul 7, 2026
…ase B) (#620) * fix(chat-message): remove stale PR#212 caret hacks, clear pre-code border leak, square blockquote/table chrome (#546) Four prose rendering defects at the 13px base, each locked by a new PROSE-POLISH-13PX-0 contract: - Drop the :nth-last-child(2) margin/inline hacks: they assumed the bubble ended with an inline timestamp child (PR #212), which moved to the turn footer long ago, so they instead zeroed/inlined the second-to-last markdown block (paragraph glued under a heading, table or code block). The streaming caret keeps its inline ride via a .maka-bubble-streaming-scoped rule. - Reset border on .maka-code-block pre code: the inline-code pill border painted a rounded outline around every wrapped line box inside pre. - Zero trailing/leading margins inside blockquote (8px/20px padding asymmetry). - Table border-collapse: separate + border-spacing: 0 so the outer hairline and border-radius actually render (collapse voids both). * refactor(chat-message): re-pin prose heading ladder to integer px at the 13px base (#546) The h1-h4 em values were derived for the 15px era (22/19/16/14) and landed on fractional pixels after the #616 re-tier: 19.07/16.47/13.87/ 12.13, with h3 only 0.87px above body and h4 below body. Re-pin to integer targets 19/16/14/13; h4 sits at 1em and reads as a heading via weight + secondary color (GitHub's h4 convention). Extend the LongFormArticle story with h3/h4/hr samples so the lower ladder is visually reviewable (hr written as *** — the storybook baseline token scan reads a quoted triple-dash as a custom property). * fix(chat-message): shrink-wrap prose tables to content width (#546) A short three-column table stretched to the full 72ch measure reads as voids between columns. Adopt GitHub's markdown-table shape: display: block + width: max-content lets narrow tables hug their cells, capped at the prose measure with overflow-x: auto so over-wide tables scroll horizontally. th gets white-space: nowrap so column-squeezed tables raise their min-content width and hit the scroller instead of pulverizing short CJK headers into vertical stacks; body cells still wrap. Adds a WideTable story pinning the scroll branch. * refactor(chat-message): frameless prose tables with reinforced header rule (#546) Picked from a four-way style comparison (card+tinted header / GitHub grid+zebra / frameless / brand-tint header): drop the outer border, radius and th fill — the 3% th tint was invisible in practice and the card frame boxed the table in for no informational gain. The header now reads via semibold + a foreground-alpha rule visibly stronger than the hairline row separators. Shrink-wrap + overflow-x scroll behavior stays. Contract re-pinned to the frameless shape. * refactor(chat-message): key heading and table-row rhythm to the paragraph gap (#546) Measured against the 12px paragraph gap, headings sat at 16px above / 8px below — too close to a plain paragraph break to separate sections, and tighter below than the body rhythm itself. Re-key to 20px above / 12px below (space-5 / space-3): sections separate clearly and the heading-to-body gap now equals the paragraph rhythm. Frameless table rows measured 36.5px for 13px content (inter-row whitespace 2.5x the paragraph line gap); drop cell vertical padding 8 -> 6 so rows land at ~32px. * fix(chat-message): slim the inline-code pill so hard-break lines breathe (#546) The Geist Mono font box is already 15.5px at the 0.92em pill size; with 1px vertical padding + 1px border the pill's ink box hit 19.5px — the entire line box. Consecutive hard-break lines carrying pills touched (measured 1.5px from pill edge to the next line's glyphs, 0 between two pills). Drop the vertical padding and the 3%-alpha border (invisible in both themes): the pill ink box returns to the font box and inter-line air recovers to ~4-5.5px, matching plain text lines. * fix(chat-message): scope frameless-table last-row reset to tbody (#546 Phase B) The GFM thead row is its parent's :last-child too, so the unscoped `.maka-prose tr:last-child th` (0,2,2) out-specified `.maka-prose th` (0,1,1) and erased the reinforced header rule — the frameless table's header/body split never painted (th computed border-bottom-width was 0). Found independently by two external reviewers; verified by computed-style probe before and after. Contract test now bans unscoped `.maka-prose tr:last-child` and pins the tbody-scoped reset. WideTable story comment updated for the frameless style (no rounded border anymore). * fix(chat-message): drop the streaming caret once the truncation badge shows (#546 Phase B) With the truncated badge appended after the Markdown, the trailing <p> is no longer :last-child: the caret-inline rule stops matching and the caret wraps onto its own line under the badge — blinking after "已截断" while promising text that will never render (output beyond the single-turn cap is dropped, chat-view.tsx). Suppress the ::after via :has(> [role="status"]); verified by computed-style probe (content none with badge, ▎ without). * test(desktop): reword pre-code border guard for the now-borderless pill The inline-code pill no longer carries a border (dropped in the pill slimming pass), so the guard's rationale read as if a live leak still existed. Keep the reset pinned as a regression guard and say so. * test(desktop): close prose contract gaps flagged in review; correct border-spacing comment - Negative assertion: ban p:last-child inlining on .maka-prose / .maka-bubble-assistant (the caret test only asserted the streaming variant exists — the regression it guards against was untestable). - Assert blockquote > :first-child margin-top: 0 to match the test's 'both ends' claim. - The table comment claimed border-collapse AND border-spacing are inherited; border-spacing is not — its initial value being 0 is what keeps the anonymous inner table seamless. Say so. * fix(chat-message): revert caret suppression under the truncation badge The rule assumed the badge means output froze. It doesn't: applyAssistantDelta sets truncated for the per-delta cap too (a single over-large delta), the flag is sticky in session-events, and later deltas keep appending — so the rule hid a live streaming indicator. CSS can't distinguish per-delta from total-cap truncation without new TSX plumbing, which isn't worth a caret's resting position. Documented the accepted caret-after-badge behavior instead. Flagged by the codex merge-gate review. * test(desktop): pin border-collapse: separate, the declaration that actually works The prose-table contract locked only the no-op border-spacing: 0 (intent documentation — not inherited, initial value already 0) while the declaration that actually reaches the anonymous inner table box, border-collapse: separate, was unpinned. Flagged in the pi merge-gate review.
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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
Verification