From 081d91982a1ed6d7dc26fe6a5e65c441aaf7f29f Mon Sep 17 00:00:00 2001 From: Hugo Dutka Date: Thu, 26 Mar 2026 17:20:16 +0100 Subject: [PATCH] fix(site): fix desktop visibility glitch (#23678) If the desktop viewer component was hidden, for example after collapsing the sidebar, the next time it was shown the viewer would be blank. This PR fixes that. --- .../hooks/useDesktopConnection.test.ts | 159 ++++++++++++++++++ .../AgentsPage/hooks/useDesktopConnection.ts | 30 ++++ 2 files changed, 189 insertions(+) diff --git a/site/src/pages/AgentsPage/hooks/useDesktopConnection.test.ts b/site/src/pages/AgentsPage/hooks/useDesktopConnection.test.ts index 5c6ac3907b..1185eaec54 100644 --- a/site/src/pages/AgentsPage/hooks/useDesktopConnection.test.ts +++ b/site/src/pages/AgentsPage/hooks/useDesktopConnection.test.ts @@ -67,6 +67,40 @@ import { watchChatDesktop } from "#/api/api"; const mockWatchChatDesktop = vi.mocked(watchChatDesktop); +// ---- Mock ResizeObserver ---------------------------------------------------- + +interface FakeResizeObserverInstance { + disconnect: ReturnType; + simulateResize: (width: number, height: number) => void; +} + +let resizeObserverInstances: FakeResizeObserverInstance[] = []; + +class MockResizeObserver { + private _callback: ResizeObserverCallback; + private _disconnect = vi.fn(); + + constructor(callback: ResizeObserverCallback) { + this._callback = callback; + const self = this; + resizeObserverInstances.push({ + disconnect: this._disconnect, + simulateResize(width: number, height: number) { + self._callback( + [{ contentRect: { width, height } } as ResizeObserverEntry], + self as unknown as ResizeObserver, + ); + }, + }); + } + + observe(_target: Element) {} + unobserve(_target: Element) {} + disconnect() { + this._disconnect(); + } +} + // ---- helpers --------------------------------------------------------------- function getLastRFBInstance(): MockRFBInstance { @@ -76,6 +110,14 @@ function getLastRFBInstance(): MockRFBInstance { return lastInstance.current; } +function getLastResizeObserver(): FakeResizeObserverInstance { + const instance = resizeObserverInstances[resizeObserverInstances.length - 1]; + if (!instance) { + throw new Error("No ResizeObserver was constructed"); + } + return instance; +} + function createMockSocket(): WebSocket { const socket = new WebSocket("ws://localhost"); vi.spyOn(socket, "close").mockImplementation(() => {}); @@ -90,6 +132,9 @@ describe("useDesktopConnection", () => { mockWatchChatDesktop.mockReturnValue(createMockSocket()); lastInstance.current = null; FakeRFB.throwOnConstruct = false; + resizeObserverInstances = []; + globalThis.ResizeObserver = + MockResizeObserver as unknown as typeof ResizeObserver; }); afterEach(() => { @@ -820,4 +865,118 @@ describe("useDesktopConnection", () => { vi.useRealTimers(); } }); + + // -- Visibility observer (ResizeObserver) --------------------------------- + + it("forces scaleViewport on hidden→visible transition", () => { + renderHook(() => useDesktopConnection({ chatId: "chat-1" })); + const rfb = getLastRFBInstance(); + act(() => rfb.simulateEvent("connect")); + + const observer = getLastResizeObserver(); + + // First observation with nonzero size (initial attach). + act(() => observer.simulateResize(800, 600)); + + // Container hidden (ancestor applies display: none). + act(() => observer.simulateResize(0, 0)); + + // Reset so we can detect re-assignment. + rfb.scaleViewport = false; + + // Container visible again — should force rescale. + act(() => observer.simulateResize(800, 600)); + + expect(rfb.scaleViewport).toBe(true); + }); + + it("does not force scaleViewport on normal nonzero→nonzero resize", () => { + renderHook(() => useDesktopConnection({ chatId: "chat-1" })); + const rfb = getLastRFBInstance(); + act(() => rfb.simulateEvent("connect")); + + const observer = getLastResizeObserver(); + + // Initial nonzero observation. + act(() => observer.simulateResize(800, 600)); + + // Reset so we can detect re-assignment. + rfb.scaleViewport = false; + + // Normal resize — not a hidden→visible transition. + act(() => observer.simulateResize(1024, 768)); + + expect(rfb.scaleViewport).toBe(false); + }); + + it("disconnects visibility observer on unmount", () => { + const { unmount } = renderHook(() => + useDesktopConnection({ chatId: "chat-1" }), + ); + + getLastRFBInstance(); + const observer = getLastResizeObserver(); + + unmount(); + + expect(observer.disconnect).toHaveBeenCalled(); + }); + + it("disconnects visibility observer before reconnect", () => { + vi.useFakeTimers(); + + try { + renderHook(() => useDesktopConnection({ chatId: "chat-1" })); + const rfb1 = getLastRFBInstance(); + act(() => rfb1.simulateEvent("connect")); + + expect(resizeObserverInstances).toHaveLength(1); + const observer1 = resizeObserverInstances[0]; + + // Trigger reconnect. + act(() => rfb1.simulateEvent("disconnect", { clean: false })); + act(() => vi.advanceTimersByTime(1000)); + + // Old observer should be disconnected. + expect(observer1.disconnect).toHaveBeenCalled(); + + // New observer created for the new connection. + expect(resizeObserverInstances).toHaveLength(2); + } finally { + vi.useRealTimers(); + } + }); + + it("ignores stale visibility observer callback after chatId change", () => { + const { rerender } = renderHook( + ({ chatId }: { chatId: string | undefined }) => + useDesktopConnection({ chatId }), + { initialProps: { chatId: "chat-aaa" as string | undefined } }, + ); + + const rfb1 = getLastRFBInstance(); + act(() => rfb1.simulateEvent("connect")); + + const observer1 = resizeObserverInstances[0]; + + // Set nonzero previous dimensions on old observer. + act(() => observer1.simulateResize(800, 600)); + + // Change chatId — triggers teardown + new connection. + rerender({ chatId: "chat-bbb" }); + + const rfb2 = getLastRFBInstance(); + act(() => rfb2.simulateEvent("connect")); + + // Reset so we can detect re-assignment. + rfb2.scaleViewport = false; + + // Fire a stale hidden→visible transition on the OLD + // observer. The generation mismatch should prevent it + // from writing to the current RFB instance. + act(() => observer1.simulateResize(0, 0)); + act(() => observer1.simulateResize(800, 600)); + + expect(rfb2.scaleViewport).toBe(false); + }); }); diff --git a/site/src/pages/AgentsPage/hooks/useDesktopConnection.ts b/site/src/pages/AgentsPage/hooks/useDesktopConnection.ts index be90fd629b..5264de7cc4 100644 --- a/site/src/pages/AgentsPage/hooks/useDesktopConnection.ts +++ b/site/src/pages/AgentsPage/hooks/useDesktopConnection.ts @@ -91,6 +91,8 @@ export function useDesktopConnection({ // contains primitives. This avoids any reliance on the React // Compiler for function identity stability. useEffect(() => { + let visibilityObserver: ResizeObserver | null = null; + const cleanupRfb = () => { if (rfbRef.current) { try { @@ -127,6 +129,8 @@ export function useDesktopConnection({ } offscreenContainerRef.current = null; reconnectAttemptRef.current = 0; + visibilityObserver?.disconnect(); + visibilityObserver = null; }; const doConnect = () => { @@ -141,6 +145,8 @@ export function useDesktopConnection({ clearAllTimers(); cleanupRfb(); + visibilityObserver?.disconnect(); + visibilityObserver = null; setStatus("connecting"); // Remove the previous offscreen container from the DOM @@ -261,6 +267,30 @@ export function useDesktopConnection({ rfbRef.current = rfb; setRfbInstance(rfb); + + // Work around a noVNC rendering bug: when an ancestor + // hides this container (display: none), the canvas + // shrinks to 0×0. When the container becomes visible + // again, noVNC may skip rescaling because it believes + // the viewport size hasn't changed. Re-assigning + // scaleViewport = true forces a fresh scale pass + // regardless. + let prevContainerW = 0; + let prevContainerH = 0; + visibilityObserver = new ResizeObserver((entries) => { + const entry = entries[0]; + if (!entry) return; + if (gen !== generationRef.current) return; + const { width, height } = entry.contentRect; + const wasHidden = prevContainerW === 0 && prevContainerH === 0; + const isVisible = width > 0 && height > 0; + prevContainerW = width; + prevContainerH = height; + if (wasHidden && isVisible && rfbRef.current) { + rfbRef.current.scaleViewport = true; + } + }); + visibilityObserver.observe(offscreenContainerRef.current); } catch { socket.close(); setStatus("error");