From 1fdb78dbbe1855dc4db23c43386b500458a30413 Mon Sep 17 00:00:00 2001 From: Pablo Gonzalez Date: Sun, 30 Aug 2026 00:38:51 +0200 Subject: [PATCH] fix(desktop): flush close button against the window edge and theme-correct its hover (#3231) * test(ui): provide sync runtime context in issue-2903 harness edfc9779c (perf(chat): make session switching feel instant) rewired useDirectoryStore and friends from the system context to the runtime context, but this harness only rendered SyncContext.Provider, so the render phase threw 'useSyncRuntime must be used within ' and every PR run since failed this file. Mirror SyncProvider's own nesting: render the runtime context (read from its globalThis registry key) inside the system one, with a currentDirectory source matching the new CurrentDirectorySource contract. Also drop the chained globalThis type assertions in favor of one documented cast. Test-only change; no runtime behavior affected. * fix(desktop): pair close-button hover with solid error red and its foreground The classic window-control close button hovered with the --status-error-background banner wash but colored the glyph with --status-error-foreground, which each theme authors as the contrast color for the solid error red (the --destructive pairing). On the wash the glyph loses contrast in both modes - near-black on muted dark red in dark themes, white on pale red in light themes - and the dark-mode wash reads as a muddy saturated red. Hover now uses the solid --status-error with its authored foreground, matching the destructive button pairing and the Windows caption-button convention. * chore: re-run PR review bot (evidence added at HEAD) * fix(header): remove right-edge gap before close button with custom window controls The header root already applied pr-0 for frameless chrome with right-side controls, but webWindowControlsOverlayStyle set an inline padding-right on the same element, which overrides the class. In Electron (frame: false, no titleBarOverlay) the WCO right inset is always 0, so the close button sat 12px from the window edge and the top-right corner did not trigger close. Skip the inline style for the frameless + right case so the class governs; the browser window-controls-overlay path keeps its padding and inset reservation. * fix(desktop): inset right-side traffic lights from the window edge The header flush-edge fix (pr-0 for frameless + right controls) also pulled the traffic-lights cluster against the window edge, but the inset is a Windows-caption convention that only the classic style follows. macOS-style circles keep their spacing: an explicit 12px right margin on the right-side cluster, owned by the component so the mini-chat window matches. * chore: re-run PR review bot (body now documents the traffic-lights inset) --- ...ue-2903-subagent-status-line-only.test.tsx | 39 ++++++++++++++++--- .../desktop/WindowsWindowControls.tsx | 10 ++++- packages/ui/src/components/layout/Header.tsx | 10 ++++- 3 files changed, 51 insertions(+), 8 deletions(-) diff --git a/packages/ui/src/components/chat/__tests__/issue-2903-subagent-status-line-only.test.tsx b/packages/ui/src/components/chat/__tests__/issue-2903-subagent-status-line-only.test.tsx index 54ed84d2..3b314a22 100644 --- a/packages/ui/src/components/chat/__tests__/issue-2903-subagent-status-line-only.test.tsx +++ b/packages/ui/src/components/chat/__tests__/issue-2903-subagent-status-line-only.test.tsx @@ -138,14 +138,27 @@ const buildMaterializedSubagentSession = () => { return { messages, part }; }; -const syncContext = (globalThis as unknown as { +// SAFETY: sync-context.tsx publishes exactly these two keys on globalThis +// (SYNC_CONTEXT_GLOBAL_KEY / SYNC_RUNTIME_CONTEXT_GLOBAL_KEY) so every module +// instance shares one context identity; the cast only adds those two optional +// keys to the global object type, and the guards below re-check presence. +const syncGlobals = globalThis as { __openchamber_sync_context__?: React.Context; -}).__openchamber_sync_context__; + __openchamber_sync_runtime_context__?: React.Context; +}; + +const syncContext = syncGlobals.__openchamber_sync_context__; if (!syncContext) { throw new Error('sync context was not published on globalThis by @/sync/sync-context'); } +const syncRuntimeContext = syncGlobals.__openchamber_sync_runtime_context__; + +if (!syncRuntimeContext) { + throw new Error('sync runtime context was not published on globalThis by @/sync/sync-context'); +} + describe('issue #2903 busy embedded subagent status-line-only', () => { test('cold disabled reads hide a fully materialized 14-message subagent; enabled reads return all 14', async () => { const dom = installMinimalDom(); @@ -173,7 +186,16 @@ describe('issue #2903 busy embedded subagent status-line-only', () => { }); const system = { childStores, messageLoader: {}, sdk: {}, runtimeKey: 'test', directory: DIRECTORY }; - const Provider = syncContext.Provider as React.Provider; + // Mirrors SyncProvider's own nesting: system context outer, runtime inner. + // Directory-scoped hooks read the runtime context, so the harness must + // provide it with a currentDirectory source for the store lookups. + const runtime = { + childStores, + messageLoader: {}, + sdk: {}, + runtimeKey: 'test', + currentDirectory: { get: () => DIRECTORY, subscribe: () => () => undefined }, + }; let inactiveCount = -1; let activeCount = -1; let enabled = false; @@ -188,15 +210,22 @@ describe('issue #2903 busy embedded subagent status-line-only', () => { return null; }; + const renderHarness = () => + React.createElement( + syncContext.Provider, + { value: system }, + React.createElement(syncRuntimeContext.Provider, { value: runtime }, React.createElement(Harness)), + ); + try { await act(async () => { - root.render(React.createElement(Provider, { value: system }, React.createElement(Harness))); + root.render(renderHarness()); }); expect(inactiveCount).toBe(0); enabled = true; await act(async () => { - root.render(React.createElement(Provider, { value: system }, React.createElement(Harness))); + root.render(renderHarness()); }); expect(activeCount).toBe(14); } finally { diff --git a/packages/ui/src/components/desktop/WindowsWindowControls.tsx b/packages/ui/src/components/desktop/WindowsWindowControls.tsx index 4d257e30..b74fdcfb 100644 --- a/packages/ui/src/components/desktop/WindowsWindowControls.tsx +++ b/packages/ui/src/components/desktop/WindowsWindowControls.tsx @@ -142,7 +142,9 @@ export const WindowsWindowControls = React.memo(function WindowsWindowControls({
@@ -207,7 +209,11 @@ export const WindowsWindowControls = React.memo(function WindowsWindowControls({ type="button" className={cn( buttonClassName, - 'hover:bg-[var(--status-error-background)] hover:text-[var(--status-error-foreground)]', + // Hover pairs the solid error red with its authored on-red + // foreground (the --destructive pairing). The error-background wash + // is a banner surface tint, not a glyph-button hover: against it the + // on-solid foreground is unreadable in both modes. + 'hover:bg-[var(--status-error)] hover:text-[var(--status-error-foreground)]', )} onClick={() => { void invokeDesktop('desktop_close_current_window'); }} title={t('header.windowControls.close')} diff --git a/packages/ui/src/components/layout/Header.tsx b/packages/ui/src/components/layout/Header.tsx index f8a3f331..7c1f3d3e 100644 --- a/packages/ui/src/components/layout/Header.tsx +++ b/packages/ui/src/components/layout/Header.tsx @@ -1356,6 +1356,14 @@ export const Header: React.FC = () => { return undefined; } + // Custom in-window controls (frameless Electron, right side) own the right + // edge: no inline padding, so the pr-0 class applies and the close button + // sits flush with the window corner per Windows conventions. Only the + // browser's native window-controls overlay reserves padding + right inset. + if (usesFramelessChrome && windowControlsSide === 'right') { + return undefined; + } + return { // Left inset is handled by the no-drag spacer (see renderDesktop); only // the right inset / titlebar height are owned by the window-controls overlay. @@ -1363,7 +1371,7 @@ export const Header: React.FC = () => { minHeight: 'max(3rem, var(--oc-wco-titlebar-height, 0px))', height: 'max(3rem, var(--oc-wco-titlebar-height, 0px))', }; - }, [isDesktopApp, isVSCode, usesFramelessChrome]); + }, [isDesktopApp, isVSCode, usesFramelessChrome, windowControlsSide]); const updateHeaderHeight = React.useCallback(() => { if (typeof document === 'undefined') {