refactor(chat): use Reactor theme resources - #1084
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Codex review: needs real behavior proof before merge. Reviewed August 5, 2026, 2:07 AM ET / 06:07 UTC. ClawSweeper reviewWhat this changesThis PR moves chat, Connection, and Hub High Contrast presentation from manual accessibility observation to WinUI/Reactor theme resources, with localized tooltip text and focused UI tests. Merge readiness⛔ Blocked until stronger real behavior proof is added - 3 items remain Keep open. The supplied current-head patch appears to address the prior High Contrast fallback finding, but the PR still needs inspectable visual proof that the active High Contrast Hub uses legible fallback icons; local retrieval of the full PR blobs was blocked by the checkout's unreachable promisor remote. Priority: P2 Review scores
Verification
How this fits togetherThe WinUI tray renders chat, the Hub navigation rail, and Connection settings. Windows theme and High Contrast values flow through application resources into those controls and their accessibility presentation. flowchart LR
WindowsTheme[Windows theme and High Contrast]
AppResources[Application theme resources]
Chat[Chat timeline and composer]
Hub[Hub navigation]
Connection[Connection settings]
UiTests[Focused UI tests]
WindowsTheme --> AppResources
AppResources --> Chat
AppResources --> Hub
AppResources --> Connection
Chat --> UiTests
Hub --> UiTests
Connection --> UiTests
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Keep the declarative resource approach, then attach a redacted current-head High Contrast screenshot or short recording that visibly shows the Hub fallback icons and an active chat surface. Do we have a high-confidence way to reproduce the issue? No. The PR describes prior COM and High Contrast behavior, but this read-only review could not establish a local current-head reproduction because the partial checkout could not retrieve the PR blobs. Is this the best way to solve the issue? Unclear. Replacing dispatcher-affine accessibility observation with declarative theme resources is a maintainable direction, but the full branch source and visual High Contrast result still need verification. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 6e225e4758e1. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (8 earlier review cycles)
|
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
bkudiess
left a comment
There was a problem hiding this comment.
Reviewed and live-tested with >90% confidence after the maintainer follow-up. Exact-head build, 3,399 Shared tests (32 skipped), 2,040 Tray tests, 100 non-accessibility UI tests, and 19 accessibility tests passed. Windows High Contrast Black proof confirms system-foreground Hub navigation while normal themes retain SVG icons. Runtime tests cover the isolated tool and compaction style paths. Final GPT and Gemini reviews reported no actionable findings at 95% and 100% confidence. GitHub CI is green for x64, ARM64, tests, E2E, hygiene, dispatch, Socket, and the CodeQL gate.
Summary
Windows.UI.ViewManagement.AccessibilitySettingsandHighContrastChangedobservation with declarative XAMLThemeResource/ThemeDictionaryresources and Reactor theme bindings.FontIcon-targeted capability-pill style for the optional Connection status glyph.ImageIconplusSvgImageSourcenavigation rendering for normal themes.FontIcons without constructing or subscribing to the legacy accessibility API.Why
AccessibilitySettingswas removedThis is a reliability fix, not only a cosmetic refactor. Previous DebugView captures on the popout path showed an unhandled
COMExceptionin the legacy accessibility lifecycle. The replacement keeps theme resolution inside WinUI/Reactor resources.The maintainer follow-up preserves the former High Contrast navigation contract without restoring that unstable subscription. Normal themes retain the colorful SVG icons. High Contrast resolves to system-foreground Fluent glyphs.
Validation
Current head:
4b702dcdgit diff --check.\build.ps1: all 5 projects passeddotnet test .\tests\OpenClaw.Shared.Tests\OpenClaw.Shared.Tests.csproj --no-restore: 3,399 passed, 32 skippeddotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restore: 2,040 passeddotnet test .\tests\OpenClaw.Tray.UITests\OpenClaw.Tray.UITests.csproj -c Debug -r win-x64 --no-build --filter "Category!=Accessibility": 100 passeddotnet test .\tests\OpenClaw.Tray.UITests\OpenClaw.Tray.UITests.csproj -c Debug -r win-x64 --no-build --filter "Category=Accessibility": 19 passedReal behavior proof
The patched app was launched with isolated tray data and a dedicated MCP port while Windows High Contrast Black was active. The Hub remained responsive and exposed visible navigation for Connection, Voice & Audio, Permissions, Sandbox, Diagnostics, and Settings. The original custom Windows theme was restored afterward, and the patched process was closed.
{ "head": "4b702dcd", "highContrastEnabled": true, "processAlive": true, "processResponding": true, "connectionStatus": "Disconnected", "mcpPort": 8876, "visibleNavigationNames": [ "Connection", "Voice & Audio", "Permissions", "Sandbox", "Diagnostics", "Settings" ] }Focused WinUI behavior proof also mounts realistic
User -> Assistant -> ToolCalland compaction rows under the isolatedTestApp, verifies the exact typed Border styles are applied, and completes without the previous CoreMessagingXP/combase host crash.Review
Thickness, and WinUI maps semantic success/caution/critical brushes toSystemColorWindowTextColorin High Contrast.MERGE, 95% confidence, no actionable findings.MERGE, 100% confidence, no actionable findings.401), so it produced no review result.Scope
This PR starts from
mainand changes chat/tray theme-resource handling, Hub navigation presentation, Connection styling, localization, and focused tests. It does not include chat tail positioning or follow-tail behavior.