Conversation
Owner
Author
Review: feat(web): redesign SyntheticMessageBlock — markdown, subtle card, collapsibleVerdict: Approve with minor nit This is a clean, well-executed redesign. The implementation follows the design spec precisely and reuses the existing What's done well
Minor nit
Not a blocker
|
Owner
Author
|
Thanks for the thorough review! Nit fixed (commit
|
…e card, collapsible Replace the purple-background synthetic message card with a cleaner, more readable design: - **Subtle dark card**: `bg-gray-900 border border-gray-700` instead of purple background. Purple is now accent-only (small dot + header label). - **Markdown rendering**: text blocks now render through `MarkdownRenderer` (headings, bold, lists, inline code, fenced code blocks) instead of raw monospace text. - **Collapsible content**: defaults to ~8 lines collapsed with "Show more / Show less" toggle (mirrors ThinkingBlock). Uses `useLayoutEffect` for initial measurement and `ResizeObserver` to re-check after MarkdownRenderer's async HTML update. Header shows line count when collapsed. - **No copy inside card**: copy button stays in the action row below (where it already was); not duplicated inside the card. Update tests: mock `MarkdownRenderer` for unit isolation, add collapse/expand tests (using `scrollHeight` mock), update overflow-protection tests to check `.prose` class instead of removed `whitespace-pre-wrap`/`break-words` utility classes.
…ownRenderer in overflow tests - Fix comment on contentRef div: was incorrectly described as "outside the maxHeight container" — it is inside, but scrollHeight works because the div has no overflow clipping of its own. - Add explicit MarkdownRenderer mock to overflow-protection.test.tsx for clarity, matching the approach used in SyntheticMessageBlock.test.tsx.
0ba6a0c to
3f400b7
Compare
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
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.
Redesigns the synthetic message card in Space task threads.
Changes:
bg-gray-900 border border-gray-700) replaces purple background. Purple is accent-only: small dot + "Synthetic" label in header.MarkdownRenderer(headings, bold, lists, inline/fenced code) instead of raw monospace text.Tests updated:
MarkdownRendererfor unit isolation; new collapse/expand tests usingscrollHeightmock.overflow-protection.test.tsx: updated assertions to check.proseclass (provided byMarkdownRenderer) instead of removed Tailwind utility classes.Pre-existing failure:
SpaceIslandflaky test fails intermittently in full-suite runs due to test ordering / Suspense timing — confirmed pre-existing, unrelated to this PR.