Skip to content

Fix chat bubble text vanish on hover-out and tool-card collapse on click - #589

Merged
kmahone merged 3 commits into
openclaw:masterfrom
RBrid:user/rbrid/ChatSelectionLoss1
Jun 5, 2026
Merged

Fix chat bubble text vanish on hover-out and tool-card collapse on click#589
kmahone merged 3 commits into
openclaw:masterfrom
RBrid:user/rbrid/ChatSelectionLoss1

Conversation

@RBrid

@RBrid RBrid commented May 29, 2026

Copy link
Copy Markdown
Contributor

Fixes two bugs in the tray chat timeline (OpenClawChatTimeline.cs):

  1. Tool-output bubble text vanishes when the mouse leaves the bubble — markdown Inlines were being rebuilt on every template-driven re-render, and the rebuild occasionally produced an empty TextBlock. The fix extends the ConditionalWeakTable-backed ApplyPlainSelectableInlines memoization to the tool-output code block so the rendered text is stable across re-renders.
  • The same root cause hit regular markdown bubbles too — that half was already fixed on master in commit bf21e7b ("Fix native chat UI bugs in tray app", item 2b), which introduced the s_plainCache and ApplyPlainSelectableInlines for user/markdown bubbles. This PR extends the existing mechanism to the tool-output path that bf21e7b didn't cover.
  • A second, tool-output-specific root cause is also fixed here: new FontFamily("…") was being allocated on every render. Reassigning a referentially-different FontFamily to a TextBlock invalidates its inline runs even when the source string is identical, which made multi-line wrapped text vanish during pointer-exit re-renders. The new per-DispatcherQueue FontFamily cache (s_monoFontByDispatcher) eliminates that allocation while respecting FontFamily's thread affinity.
  1. Tool-output card collapses when the user clicks inside the body (e.g. to start a text selection) — the entire card was a single Button, so any click inside (including the expanded body) toggled it. Restructured into a header-only Button with the expanded body as a sibling. Corner rounding adjusts based on hasExpandedBody so the visual chrome is unchanged.

Files changed

  • src/OpenClaw.Tray.WinUI/Chat/OpenClawChatTimeline.cs

Key regions (post-merge with master):

  • Lines ~134-167: per-DispatcherQueue FontFamily cache (s_monoFontByDispatcher)
  • Lines ~1544-1571: extends Inlines cache to tool-output bubble
  • Lines ~1639-1685: header-only Button restructure with hasExpandedBody rounding

Validation

  • ./build.ps1 — green (0 warnings, 0 errors across Shared, Cli, WinNode.Cli, SetupEngine.UI, Tray.WinUI)
  • OpenClaw.Shared.Tests — 2049 passed, 29 skipped
  • OpenClaw.Tray.Tests — 936 passed
  • Manual UI test: both repro cases (tool-output text vanish on hover-out, collapse on click-to-select) confirmed fixed; no regression to regular text bubbles.

Review

Underwent a dual-model adversarial review (Claude Opus + GPT Codex) of the diff. Consensus hardening applied:

  • Switched from singleton Lazy to per-DispatcherQueue ConditionalWeakTable keying (FontFamily is a thread-affine DependencyObject — a process-wide singleton would crash on a second dispatcher).

A follow-up a11y issue (HIGH severity, 60% confidence: tool-output body sits outside the header Button's automation subtree → Narrator may miss body content) was deferred to keep this PR scoped to the user-reported bug.

Screenshot with disappearing tool content:
image

After fix:
image

RBrid and others added 2 commits May 29, 2026 11:26
Bug openclaw#1: Selecting text in a chat bubble lost the selection as soon as
the pointer left the bubble. ChatMarkdownRenderer was re-applying the
same MdInline list to TextBlock.Inlines on every render, which wiped
the Run instances the selection pointed to.

Cache the last applied inline list on each TextBlock via
ConditionalWeakTable and skip re-application when the list is
reference-equal. Add reflection-based equality guard tests for MdInline
to ensure new fields are included in equality comparisons.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When the mouse exited an assistant bubble containing a tool-output panel, the wrapped text inside vanished, leaving an empty rectangle. Clicking anywhere in the panel also toggled collapse. Both behaviors are fixed:

* Phase A: Memoize tool-output Inlines via ConditionalWeakTable so parent re-renders (e.g. hover-out flipping `hoveredEntries`) no longer rebuild the TextBlock content and invalidate live selections.

* Restructure the tool card so only the header is a Button. The body sits in a sibling row, so clicks/selections inside the body no longer collapse the panel. Bottom-corner rounding is driven by `hasExpandedBody` to keep the visual seam consistent.

* Hoist the `Cascadia Code` FontFamily out of the per-render lambda. FontFamily is a DependencyObject with thread affinity, so the static field is backed by a per-DispatcherQueue ConditionalWeakTable cache (mirrors the existing brush-cache pattern). Off-dispatcher callers (tests / design-time) get a one-shot uncached instance.

Validation: `./build.ps1` green; Shared tests 2031 passed / 29 skipped; Tray tests 868 passed; manual repro confirmed cured by user.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@clawsweeper

clawsweeper Bot commented May 29, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed June 5, 2026, 1:55 PM ET / 17:55 UTC.

Summary
The PR updates tray chat markdown/tool-output rendering caches, changes expanded tool-card rows to header-only toggles, and adds shared markdown inline equality guard tests.

Reproducibility: Source-reproducible, but not runtime-reproduced here: current master still wraps the tool body in the toggle Button and renders tool output through direct TextBlock text with a fresh FontFamily. The supplied screenshots corroborate the empty-output symptom but do not exercise both interactions after the fix.

Review metrics: none identified.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦐 gold shrimp
Patch quality: 🐚 platinum hermit
Result: blocked until stronger real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P1] Add a short recording or paired screenshots showing tool-output text remains visible after hover-out and selecting inside the expanded body does not collapse the card.
  • [P1] Get maintainer direction on whether the possible screen-reader relationship issue can remain a follow-up or needs wiring before merge.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: The screenshots partially prove before/after text restoration, but merge still needs a short recording or screenshots showing hover-out persistence and click-to-select without collapse; redact private details, update the PR body to trigger re-review, or ask a maintainer to comment @clawsweeper re-review.

Mantis proof suggestion
A desktop visual proof lane would directly show the hover-out and click-to-select interactions that screenshots do not prove. A maintainer can ask Mantis to capture proof by posting a new PR comment that starts with the OpenClaw Mantis account mention, followed by:

visual task: verify tray tool-output text remains visible after hover-out and selecting inside expanded tool output does not collapse the card.

Risk before merge

  • [P1] The contributor proof is still screenshot-only: it shows text restoration, but not that text remains visible after hover-out or that selecting inside the expanded body no longer collapses the card.
  • [P1] The PR body explicitly defers a possible accessibility issue where moving the expanded body outside the header Button may weaken the screen-reader relationship for tool-output body content.

Maintainer options:

  1. Require interaction proof and a11y direction (recommended)
    Ask for a short recording or paired screenshots covering hover-out and click-to-select, then get maintainer direction on whether the accessibility concern can remain a follow-up.
  2. Accept scoped follow-up risk
    Maintainers can choose to land the bug fix with the current screenshot proof if they explicitly accept tracking the possible screen-reader relationship issue separately.

Next step before merge

  • [P1] Human merge handling is needed for the real behavior proof gate and the acknowledged accessibility tradeoff; no narrow code defect was found for an automated repair lane.

Security
Cleared: The diff changes local WinUI rendering and shared markdown equality tests; it does not add dependency, workflow, secret, network, or supply-chain surface.

Review details

Best possible solution:

Land the focused tray chat fix after interactive proof is added and maintainers either accept the accessibility follow-up or require accessibility wiring before merge.

Do we have a high-confidence way to reproduce the issue?

Source-reproducible, but not runtime-reproduced here: current master still wraps the tool body in the toggle Button and renders tool output through direct TextBlock text with a fresh FontFamily. The supplied screenshots corroborate the empty-output symptom but do not exercise both interactions after the fix.

Is this the best way to solve the issue?

Mostly yes: the patch follows the existing inline-cache pattern and narrows the collapse toggle to the header, which is the right ownership boundary. It is not merge-ready until the interaction proof and accessibility deferral decision are handled.

AGENTS.md: found, but no applicable review policy affected this item.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 99efc50cbc22.

Label changes

Label justifications:

  • P2: The PR addresses a normal tray chat UI bug affecting tool-output readability and selection with limited blast radius.
  • merge-risk: 🚨 other: The header-only toggle restructuring may alter screen-reader traversal for expanded tool output, which CI does not settle.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The screenshots partially prove before/after text restoration, but merge still needs a short recording or screenshots showing hover-out persistence and click-to-select without collapse; redact private details, update the PR body to trigger re-review, or ask a maintainer to comment @clawsweeper re-review.
  • proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. The screenshots partially prove before/after text restoration, but merge still needs a short recording or screenshots showing hover-out persistence and click-to-select without collapse; redact private details, update the PR body to trigger re-review, or ask a maintainer to comment @clawsweeper re-review.
Evidence reviewed

What I checked:

  • Repository policy read: AGENTS.md was read fully; its validation guidance applies after code changes, but this was a read-only review and no connection/pairing architecture rules were touched. (AGENTS.md:1, 99efc50cbc22)
  • Current master still wraps expanded tool content in the toggle button: The current tray timeline builds rowContent from headerRow plus body and returns Button(rowWithSeparator, toggle), so clicks from the expanded body can still route to the collapse toggle on master. (src/OpenClaw.Tray.WinUI/Chat/OpenClawChatTimeline.cs:1575, 99efc50cbc22)
  • Current master still renders tool output as direct TextBlock text with per-render FontFamily: The current tool-output body uses TextBlock(displayText) and assigns a new Cascadia FontFamily in the render setter, so the tool-output path has not yet adopted the stable inline/cache path proposed by this PR. (src/OpenClaw.Tray.WinUI/Chat/OpenClawChatTimeline.cs:1510, 99efc50cbc22)
  • PR diff targets the observed code paths: The PR diff changes tool output to TextBlock("") plus ApplyPlainSelectableInlines, introduces a per-dispatcher monospace FontFamily cache, and scopes the toggle Button to the header row only. (src/OpenClaw.Tray.WinUI/Chat/OpenClawChatTimeline.cs:1543, 5c72f943bcb9)
  • Screenshot proof is partial: Downloaded and inspected the before/after screenshots: they show an empty tool-output panel before and visible output text after, but they do not show the hover-out persistence or click-to-select interaction that the PR also changes. (5c72f943bcb9)
  • Related history: The prior native chat UI fix commit introduced the existing plain inline cache that this PR extends to the tool-output path. (src/OpenClaw.Tray.WinUI/Chat/OpenClawChatTimeline.cs:249, bf21e7b216aa)

Likely related people:

  • RBrid: Current-main history includes the prior native chat UI inline-cache fix and recent ChatMarkdownRenderer/OpenClawChatTimeline work that this PR extends. (role: recent area contributor and prior related fix author; confidence: high; commits: bf21e7b216aa, 0a164fc111b2, 7d9152f427a3; files: src/OpenClaw.Tray.WinUI/Chat/OpenClawChatTimeline.cs, src/OpenClaw.Tray.WinUI/Chat/Markdown/ChatMarkdownRenderer.cs)
  • Christine Yan: Local blame on the current affected tray timeline lines points to the recent localization/current-main commit, and API history shows adjacent chat UI polish work in the same file. (role: recent area contributor; confidence: medium; commits: 85445c78066b, a482ca9e1713; files: src/OpenClaw.Tray.WinUI/Chat/OpenClawChatTimeline.cs)
  • kenehong: Recent current-main history for the tray chat timeline includes assistant-bubble rendering and alignment fixes in the same UI surface. (role: adjacent chat timeline contributor; confidence: medium; commits: 4231feab9c0e; files: src/OpenClaw.Tray.WinUI/Chat/OpenClawChatTimeline.cs)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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 keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. labels May 29, 2026
@RBrid
RBrid marked this pull request as ready for review May 29, 2026 21:40
@RBrid
RBrid marked this pull request as draft June 4, 2026 23:28
@clawsweeper clawsweeper Bot added proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jun 5, 2026
@RBrid
RBrid marked this pull request as ready for review June 5, 2026 17:47
@kmahone
kmahone merged commit 16929b6 into openclaw:master Jun 5, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. P2 Normal priority bug or improvement with limited blast radius. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants