mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
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.
This commit is contained in:
@@ -67,6 +67,40 @@ import { watchChatDesktop } from "#/api/api";
|
||||
|
||||
const mockWatchChatDesktop = vi.mocked(watchChatDesktop);
|
||||
|
||||
// ---- Mock ResizeObserver ----------------------------------------------------
|
||||
|
||||
interface FakeResizeObserverInstance {
|
||||
disconnect: ReturnType<typeof vi.fn>;
|
||||
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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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");
|
||||
|
||||
Reference in New Issue
Block a user