improve(chat): make prose and simple lists one selectable text block#997
Open
karkarl wants to merge 2 commits into
Open
improve(chat): make prose and simple lists one selectable text block#997karkarl wants to merge 2 commits into
karkarl wants to merge 2 commits into
Conversation
…selection Chat prose was rendered as many separate TextBlocks, so a user could not drag-select text across paragraphs of one message. Add a poolable RichTextBlock primitive to the in-house FunctionalUI reconciler and route both assistant and user message text through one RichTextBlock per message (a Paragraph per block), giving each message a single continuous, selection-friendly text scope while keeping chat virtualizable. - FunctionalUI: RichTextBlockElement record, RichTextBlock() factory, Set(Action<RichTextBlock>), reconciler arm, ConfigureRichTextBlock (leaves Blocks to the setter), ApplyModifiers case. - ChatMarkdownRenderer: coalesce consecutive paragraph/heading blocks into one RichTextBlock; lone text blocks stay TextBlocks; lists/code/tables/etc stay separate selectable siblings. Structural block-run cache preserves active selection across re-renders independent of AST-cache eviction. Inert posture preserved (AppendInlines only). - OpenClawChatTimeline: user bubble renders as a single-Paragraph RichTextBlock preserving font, foreground, wrap and SelectionHighlightColor. - Tests: FunctionalUI primitive tests, renderer coalescing tests, updated user-bubble contract + virtualization proof text collection. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Extend the assistant-bubble text coalescing so simple (text-only, possibly nested) bullet/ordered/task lists flow into the same per-message RichTextBlock as surrounding prose, making paragraphs + headings + list items one continuous drag-selection scope. Lists whose items carry a code block, table, blockquote, thematic break, or raw block stay as their own Grid-based selectable island (RenderList), preserving that chrome. List items render as hanging-indent Paragraphs inside the shared RichTextBlock: a per-item marker (task box, ordered number, bullet), fixed hanging indent so wrapped lines / continuation paragraphs align at the content column, per-level indentation for nesting, and heading formatting preserved for heading list items. Extend the selection-preservation cache (BlockEqual) with structural ListEqual so the bounded AST cache evicting and re-parsing an unchanged message does not force a Blocks rebuild that would wipe the active selection. Update MarkdownRendererCoalesceTests / MarkdownRendererListTests: simple lists now assert RichTextBlock coalescing; the issue openclaw#636 Grid wrap guards retarget to complex (blockquote-bearing) lists that still use the Grid island path. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
clawsweeper
Bot
added
proof: 📸 screenshot
|
Codex review: needs real behavior proof before merge. Reviewed July 15, 2026, 7:43 PM ET / 23:43 UTC. Summary Reproducibility: yes. at the source level: recycle the same RichTextBlock through assistant content A, user content B, then assistant content A again; the assistant cache can match A while the live Blocks still contain B. A real WinUI execution of that sequence was not supplied. Review metrics: 3 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Use one authoritative per-control render-state cache shared by every RichTextBlock population path, cover assistant-to-user-to-assistant pool reuse with a regression test, and provide a current-head native recording showing the intended selection boundary. Do we have a high-confidence way to reproduce the issue? Yes at the source level: recycle the same RichTextBlock through assistant content A, user content B, then assistant content A again; the assistant cache can match A while the live Blocks still contain B. A real WinUI execution of that sequence was not supplied. Is this the best way to solve the issue? No, not as written: coalescing into RichTextBlock is a reasonable design, but selection preservation must use cache state shared across all writers of a pooled control rather than independent ConditionalWeakTables. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against bf0fa8a9bde4. Label changesLabel changes:
Label justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
|
ClawSweeper status: review started. I am starting a fresh review of this pull request: improve(chat): make prose and simple lists one selectable text block This is item 1/1 in the current shard. Shard 0/1. This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking. Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted. |
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
1 participant
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.
Related: #995
Related: #996
What Problem This Solves
In the tray chat timeline, a message's text was fragmented into many separate text controls: each paragraph, heading, and list item was its own box. A user could not drag-select a whole assistant reply in one gesture, which was awkward and annoying when trying to copy part of an answer.
Why This Change Was Made
Each chat message now renders through a single per-message
RichTextBlock, and consecutive prose (paragraphs + headings) plus simple text-only lists (bullet / ordered / task, including nested) coalesce into that one control, so the whole run is one continuous drag-selection scope.Blocks that need their own chrome stay as separate selectable islands: code blocks, tables, block quotes, thematic breaks, and any list whose items contain one of those. This split is a WinUI constraint, not a preference:
InlineUIContaineris excluded fromSelectedTextand breaks selection at its boundary, and there is no built-in way to run one selection across multiple controls. So chrome-bearing content cannot be embedded inline without losing selection.Supporting pieces:
RichTextBlockprimitive added to the in-houseOpenClawTray.FunctionalUIlayer (factory,Set, modifiers).RichTextBlock(per-item marker, fixed hanging indent so wrapped lines align at the content column, per-level nesting, heading formatting preserved).The larger follow-ups are tracked separately: a custom cross-control selection engine for whole-transcript selection (#995), and porting FunctionalUI to Reactor to stop hand-maintaining bespoke primitives (#996).
User Impact
Users can drag-select an assistant reply's prose and bullet / numbered lists as one continuous selection instead of fighting per-line boxes. Code blocks and tables stay distinctly styled and individually selectable. No change to how messages are authored or sent.
Evidence
Record-level renderer tests assert the Element shape directly: prose + a simple list collapses to one
RichTextBlockElement, while a list containing a non-text block stays its own island. Full required suites plus focused renderer tests pass (counts below).Change Type
Scope
winnodeValidation
All run on branch head
47b335d2(ARM64 host,-p:Platform=x64for WinUI/UITests):./build.ps1-> all 5 projects built successfullydotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj-> Passed 2754, Skipped 31, Failed 0dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj-> Passed 1705, Failed 0dotnet test ./tests/OpenClawTray.FunctionalUI.Tests/...-> Passed 14, Failed 0dotnet test ./tests/OpenClaw.Tray.UITests/...-> Passed 106, Failed 0--filter FullyQualifiedName~MarkdownRenderer-> Passed 12, Failed 0Real Behavior Proof
OpenClawGatewayWSL distro; focused renderer tests headless.47b335d2dotnet test --filter FullyQualifiedName~MarkdownRenderer -p:Platform=x64; (2) relaunched the isolated app (run-app-local.ps1 -NoBuild -Isolated -AllowNonMain, PID 34000) to drag-select a message containing prose + a bullet list + a fenced code block.MarkdownRendererCoalesceTests/MarkdownRendererListTestsassert prose + simple list -> oneRichTextBlockElement, lone simple list ->RichTextBlockElement, and a list containing a block quote -> separateStackElementisland. 12/12 pass.RichTextBlock; code blocks stay their own styled island; Copy button grabs the whole message.autoreviewcould not run becausecodex/claude/piCLIs are not installed in this environment; a rubber-duck review did run and both findings (heading-in-list formatting, true hanging indent) were fixed.Security Impact
Hyperlink/BitmapImageintroduced,AppendInlinesunchanged, rendering stays non-interactive.Compatibility and Migration
Review Conversations