diff --git a/src/components/RemoteCursors.test.jsx b/src/components/RemoteCursors.test.jsx new file mode 100644 index 0000000..bc95f21 --- /dev/null +++ b/src/components/RemoteCursors.test.jsx @@ -0,0 +1,40 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { render } from "@testing-library/react"; +import { RemoteCursors } from "./RemoteCursors"; + +function makeCursor(id, overrides = {}) { + return { + id, + displayName: `User-${id}`, + color: "#ff0000", + x: 100, + y: 200, + lastUpdate: Date.now(), + ...overrides, + }; +} + +describe("RemoteCursors", () => { + it("renders nothing when cursors is empty", () => { + const { container } = render(); + expect(container.innerHTML).toBe(""); + }); + + it("renders nothing when cursors is null", () => { + const { container } = render(); + expect(container.innerHTML).toBe(""); + }); + + it("renders one element per cursor", () => { + const cursors = [makeCursor("a"), makeCursor("b")]; + const { container } = render(); + const svgs = container.querySelectorAll("svg"); + expect(svgs).toHaveLength(2); + }); + + it("displays the cursor display name", () => { + const cursors = [makeCursor("a", { displayName: "Alice" })]; + const { container } = render(); + expect(container.textContent).toContain("Alice"); + }); +}); diff --git a/src/constants.js b/src/constants.js index 41f2537..5841756 100644 --- a/src/constants.js +++ b/src/constants.js @@ -63,7 +63,8 @@ export const DESCRIPTION_MAX_LENGTH = 500; // ── Collaboration ─────────────────────────── export const COLLAB_DEBOUNCE_MS = 500; -export const CURSOR_THROTTLE_MS = 50; +export const CURSOR_THROTTLE_MS = 150; +export const CURSOR_STALE_MS = 4000; export const COLLAB_ROOM_CODE_LENGTH = 6; export const COLLAB_CURSOR_FADE_MS = 3000; diff --git a/src/hooks/useCollaboration.js b/src/hooks/useCollaboration.js index ffcf643..8e0f5fb 100644 --- a/src/hooks/useCollaboration.js +++ b/src/hooks/useCollaboration.js @@ -1,6 +1,6 @@ import { useState, useRef, useCallback, useEffect } from "react"; import { supabase } from "../collab/supabaseClient"; -import { COLLAB_DEBOUNCE_MS, CURSOR_THROTTLE_MS, COLLAB_ROOM_CODE_LENGTH } from "../constants"; +import { COLLAB_DEBOUNCE_MS, CURSOR_THROTTLE_MS, CURSOR_STALE_MS, COLLAB_ROOM_CODE_LENGTH } from "../constants"; function generateRoomCode() { const chars = "ABCDEFGHJKLMNPQRSTUVWXYZ23456789"; // no I/O/0/1 for clarity @@ -48,6 +48,7 @@ export function useCollaboration({ const applyRemoteStateRef = useRef(null); const applyRemoteImageRef = useRef(null); const initializedPeersRef = useRef(new Set()); + const remoteCursorsRef = useRef([]); // Keep refs in sync useEffect(() => { screensRef.current = screens; }, [screens]); @@ -102,11 +103,30 @@ export function useCollaboration({ } }); + // Broadcast: cursor-update + channel.on("broadcast", { event: "cursor-update" }, ({ payload }) => { + if (!payload?.peerId) return; + const entry = { + id: payload.peerId, + displayName: payload.displayName, + color: payload.color, + x: payload.x, + y: payload.y, + lastUpdate: payload.ts, + }; + const prev = remoteCursorsRef.current; + const idx = prev.findIndex((c) => c.id === payload.peerId); + const next = idx >= 0 + ? prev.map((c, i) => (i === idx ? entry : c)) + : [...prev, entry]; + remoteCursorsRef.current = next; + setRemoteCursors(next); + }); + // Presence: sync channel.on("presence", { event: "sync" }, () => { const state = channel.presenceState(); const allPeers = []; - const cursors = []; for (const [, presences] of Object.entries(state)) { for (const p of presences) { if (p.peerId === peerIdRef.current) continue; @@ -116,26 +136,22 @@ export function useCollaboration({ color: p.color, role: p.role, }); - if (p.cursorX != null && p.cursorY != null) { - cursors.push({ - id: p.peerId, - displayName: p.displayName, - color: p.color, - x: p.cursorX, - y: p.cursorY, - lastUpdate: p.cursorTs || Date.now(), - }); - } } } setPeers(allPeers); - setRemoteCursors(cursors); + + // Remove cursors for peers that left via Presence + const activePeerIds = new Set(allPeers.map((p) => p.id)); + const filtered = remoteCursorsRef.current.filter((c) => activePeerIds.has(c.id)); + if (filtered.length !== remoteCursorsRef.current.length) { + remoteCursorsRef.current = filtered; + setRemoteCursors(filtered); + } // Host departure detection if (myRole !== "host") { const hasHost = allPeers.some((p) => p.role === "host"); if (!hasHost && allPeers.length === 0 && channelRef.current) { - // Check if we ever had a host (we did since we joined) setHostLeft(true); } } @@ -193,9 +209,6 @@ export function useCollaboration({ displayName, color, role: "host", - cursorX: null, - cursorY: null, - cursorTs: null, }); channelRef.current = channel; setRoomCode(code); @@ -226,9 +239,6 @@ export function useCollaboration({ displayName, color, role: "editor", - cursorX: null, - cursorY: null, - cursorTs: null, }); channelRef.current = channel; setRoomCode(normalizedCode); @@ -253,6 +263,7 @@ export function useCollaboration({ debounceTimerRef.current = null; } initializedPeersRef.current.clear(); + remoteCursorsRef.current = []; setIsConnected(false); setRoomCode(null); setRole(null); @@ -336,14 +347,17 @@ export function useCollaboration({ const worldX = (e.clientX - rect.left - panRef.current.x) / zoomRef.current; const worldY = (e.clientY - rect.top - panRef.current.y) / zoomRef.current; - channel.track({ - peerId: peerIdRef.current, - displayName: selfDisplayNameRef.current || "", - color: selfColorRef.current || "#61afef", - role: roleRef.current, - cursorX: worldX, - cursorY: worldY, - cursorTs: now, + channel.send({ + type: "broadcast", + event: "cursor-update", + payload: { + peerId: peerIdRef.current, + displayName: selfDisplayNameRef.current || "", + color: selfColorRef.current || "#61afef", + x: worldX, + y: worldY, + ts: now, + }, }); }; @@ -351,6 +365,23 @@ export function useCollaboration({ return () => canvas.removeEventListener("mousemove", onMouseMove); }, [isConnected, canvasRef]); + // Stale cursor cleanup: remove cursors not updated within CURSOR_STALE_MS. + // Handles unclean disconnects (browser crash, network drop) where Presence + // leave event never fires. + useEffect(() => { + if (!isConnected) return; + const id = setInterval(() => { + const now = Date.now(); + const prev = remoteCursorsRef.current; + const filtered = prev.filter((c) => now - c.lastUpdate < CURSOR_STALE_MS); + if (filtered.length !== prev.length) { + remoteCursorsRef.current = filtered; + setRemoteCursors(filtered); + } + }, 1000); + return () => clearInterval(id); + }, [isConnected]); + // Cleanup on unmount useEffect(() => { return () => { diff --git a/src/hooks/useCollaboration.test.js b/src/hooks/useCollaboration.test.js new file mode 100644 index 0000000..664b962 --- /dev/null +++ b/src/hooks/useCollaboration.test.js @@ -0,0 +1,333 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { renderHook, act } from "@testing-library/react"; +import { useCollaboration } from "./useCollaboration"; + +// ── Mock supabaseClient ───────────────────────────────────────────── +// We capture handlers registered via channel.on() so tests can fire them. +let channelHandlers; +let mockChannel; +let mockTrackPayload; + +function createMockChannel() { + channelHandlers = {}; + mockTrackPayload = null; + + mockChannel = { + on(type, filter, cb) { + const key = type === "broadcast" + ? `broadcast:${filter.event}` + : `presence:${filter.event}`; + channelHandlers[key] = cb; + return mockChannel; // chainable + }, + subscribe(cb) { + // Immediately invoke with "SUBSCRIBED" + if (cb) Promise.resolve().then(() => cb("SUBSCRIBED")); + return mockChannel; + }, + track: vi.fn(async (payload) => { mockTrackPayload = payload; }), + send: vi.fn(), + unsubscribe: vi.fn(), + presenceState: vi.fn(() => ({})), + }; + + return mockChannel; +} + +vi.mock("../collab/supabaseClient", () => ({ + supabase: { + channel: () => createMockChannel(), + }, +})); + +// ── Helpers ───────────────────────────────────────────────────────── +const noop = () => {}; +const defaultProps = () => ({ + screens: [], + connections: [], + documents: [], + featureBrief: "", + taskLink: "", + techStack: "", + dataModels: [], + stickyNotes: [], + screenGroups: [], + applyRemoteState: noop, + applyRemoteImage: noop, + canvasRef: { current: null }, + pan: { x: 0, y: 0 }, + zoom: 1, +}); + +async function setupAndCreate(overrides = {}) { + const props = { ...defaultProps(), ...overrides }; + const hook = renderHook(() => useCollaboration(props)); + + // Create a room to wire up channel handlers + await act(async () => { + hook.result.current.createRoom("Alice", "#ff0000"); + // Let the microtask (subscribe callback) resolve + await Promise.resolve(); + }); + return hook; +} + +// ── Tests ─────────────────────────────────────────────────────────── + +describe("useCollaboration — cursor broadcast", () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + afterEach(() => { + vi.useRealTimers(); + vi.restoreAllMocks(); + }); + + // ── track() calls no longer include cursor fields ───────────── + it("track() in createRoom does not include cursor fields", async () => { + await setupAndCreate(); + expect(mockTrackPayload).toBeDefined(); + expect(mockTrackPayload).not.toHaveProperty("cursorX"); + expect(mockTrackPayload).not.toHaveProperty("cursorY"); + expect(mockTrackPayload).not.toHaveProperty("cursorTs"); + expect(mockTrackPayload).toHaveProperty("peerId"); + expect(mockTrackPayload).toHaveProperty("displayName", "Alice"); + expect(mockTrackPayload).toHaveProperty("role", "host"); + }); + + // ── cursor-update broadcast adds a new cursor ───────────────── + it("cursor-update broadcast adds a new remote cursor", async () => { + const { result } = await setupAndCreate(); + const handler = channelHandlers["broadcast:cursor-update"]; + expect(handler).toBeDefined(); + + act(() => { + handler({ + payload: { + peerId: "peer-1", + displayName: "Bob", + color: "#00ff00", + x: 100, + y: 200, + ts: 1000, + }, + }); + }); + + expect(result.current.remoteCursors).toHaveLength(1); + expect(result.current.remoteCursors[0]).toMatchObject({ + id: "peer-1", + displayName: "Bob", + x: 100, + y: 200, + lastUpdate: 1000, + }); + }); + + // ── cursor-update broadcast updates an existing cursor ──────── + it("cursor-update broadcast updates existing cursor instead of duplicating", async () => { + const { result } = await setupAndCreate(); + const handler = channelHandlers["broadcast:cursor-update"]; + + act(() => { + handler({ + payload: { peerId: "peer-1", displayName: "Bob", color: "#00ff00", x: 100, y: 200, ts: 1000 }, + }); + }); + expect(result.current.remoteCursors).toHaveLength(1); + + act(() => { + handler({ + payload: { peerId: "peer-1", displayName: "Bob", color: "#00ff00", x: 300, y: 400, ts: 2000 }, + }); + }); + expect(result.current.remoteCursors).toHaveLength(1); + expect(result.current.remoteCursors[0].x).toBe(300); + expect(result.current.remoteCursors[0].y).toBe(400); + expect(result.current.remoteCursors[0].lastUpdate).toBe(2000); + }); + + // ── multiple peers tracked independently ────────────────────── + it("tracks cursors from multiple peers independently", async () => { + const { result } = await setupAndCreate(); + const handler = channelHandlers["broadcast:cursor-update"]; + + act(() => { + handler({ payload: { peerId: "peer-1", displayName: "Bob", color: "#ff0000", x: 10, y: 20, ts: 100 } }); + handler({ payload: { peerId: "peer-2", displayName: "Eve", color: "#0000ff", x: 50, y: 60, ts: 200 } }); + }); + + expect(result.current.remoteCursors).toHaveLength(2); + expect(result.current.remoteCursors.map((c) => c.id)).toEqual(["peer-1", "peer-2"]); + }); + + // ── ignores payloads without peerId ─────────────────────────── + it("cursor-update broadcast ignores payload without peerId", async () => { + const { result } = await setupAndCreate(); + const handler = channelHandlers["broadcast:cursor-update"]; + + act(() => { + handler({ payload: { displayName: "Ghost", x: 0, y: 0, ts: 1 } }); + }); + expect(result.current.remoteCursors).toHaveLength(0); + + act(() => { + handler({ payload: null }); + }); + expect(result.current.remoteCursors).toHaveLength(0); + }); + + // ── presence:sync removes cursors for disconnected peers ────── + it("presence:sync removes cursors when peer leaves", async () => { + const { result } = await setupAndCreate(); + const cursorHandler = channelHandlers["broadcast:cursor-update"]; + const syncHandler = channelHandlers["presence:sync"]; + + // Add two cursors + act(() => { + cursorHandler({ payload: { peerId: "peer-1", displayName: "Bob", color: "#ff0000", x: 10, y: 20, ts: 100 } }); + cursorHandler({ payload: { peerId: "peer-2", displayName: "Eve", color: "#0000ff", x: 50, y: 60, ts: 200 } }); + }); + expect(result.current.remoteCursors).toHaveLength(2); + + // Simulate peer-2 leaving — only peer-1 remains in Presence + mockChannel.presenceState.mockReturnValue({ + "key1": [{ peerId: "peer-1", displayName: "Bob", color: "#ff0000", role: "editor" }], + }); + + act(() => syncHandler()); + + expect(result.current.remoteCursors).toHaveLength(1); + expect(result.current.remoteCursors[0].id).toBe("peer-1"); + }); + + // ── presence:sync keeps cursors for active peers ────────────── + it("presence:sync keeps cursors when all peers are still present", async () => { + const { result } = await setupAndCreate(); + const cursorHandler = channelHandlers["broadcast:cursor-update"]; + const syncHandler = channelHandlers["presence:sync"]; + + act(() => { + cursorHandler({ payload: { peerId: "peer-1", displayName: "Bob", color: "#ff0000", x: 10, y: 20, ts: 100 } }); + }); + + mockChannel.presenceState.mockReturnValue({ + "key1": [{ peerId: "peer-1", displayName: "Bob", color: "#ff0000", role: "editor" }], + }); + + act(() => syncHandler()); + + expect(result.current.remoteCursors).toHaveLength(1); + expect(result.current.remoteCursors[0].id).toBe("peer-1"); + }); + + // ── presence:sync still builds peer list without cursor fields ─ + it("presence:sync builds peer list without relying on cursor fields", async () => { + const { result } = await setupAndCreate(); + const syncHandler = channelHandlers["presence:sync"]; + + mockChannel.presenceState.mockReturnValue({ + "key1": [{ peerId: "peer-1", displayName: "Bob", color: "#ff0000", role: "editor" }], + "key2": [{ peerId: "peer-2", displayName: "Eve", color: "#0000ff", role: "viewer" }], + }); + + act(() => syncHandler()); + + expect(result.current.peers).toHaveLength(2); + expect(result.current.peers[0]).toMatchObject({ id: "peer-1", displayName: "Bob", role: "editor" }); + expect(result.current.peers[1]).toMatchObject({ id: "peer-2", displayName: "Eve", role: "viewer" }); + }); + + // ── leaveRoom clears remote cursors ─────────────────────────── + it("leaveRoom clears remoteCursors", async () => { + const { result } = await setupAndCreate(); + const handler = channelHandlers["broadcast:cursor-update"]; + + act(() => { + handler({ payload: { peerId: "peer-1", displayName: "Bob", color: "#ff0000", x: 10, y: 20, ts: 100 } }); + }); + expect(result.current.remoteCursors).toHaveLength(1); + + act(() => result.current.leaveRoom()); + + expect(result.current.remoteCursors).toHaveLength(0); + expect(result.current.isConnected).toBe(false); + }); + +}); + +describe("useCollaboration — stale cursor cleanup", () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + afterEach(() => { + vi.useRealTimers(); + vi.restoreAllMocks(); + }); + + it("removes stale cursors after CURSOR_STALE_MS", async () => { + const { result } = await setupAndCreate(); + const handler = channelHandlers["broadcast:cursor-update"]; + const now = Date.now(); + + act(() => { + handler({ payload: { peerId: "fresh", displayName: "Fresh", color: "#ff0000", x: 10, y: 20, ts: now } }); + handler({ payload: { peerId: "stale", displayName: "Stale", color: "#0000ff", x: 30, y: 40, ts: now - 5000 } }); + }); + expect(result.current.remoteCursors).toHaveLength(2); + + // Advance past the 1s interval tick + act(() => vi.advanceTimersByTime(1100)); + + expect(result.current.remoteCursors).toHaveLength(1); + expect(result.current.remoteCursors[0].id).toBe("fresh"); + }); + + it("does not filter when all cursors are fresh", async () => { + const { result } = await setupAndCreate(); + const handler = channelHandlers["broadcast:cursor-update"]; + const now = Date.now(); + + act(() => { + handler({ payload: { peerId: "a", displayName: "A", color: "#ff0000", x: 10, y: 20, ts: now } }); + handler({ payload: { peerId: "b", displayName: "B", color: "#0000ff", x: 30, y: 40, ts: now - 1000 } }); + }); + const before = result.current.remoteCursors; + + act(() => vi.advanceTimersByTime(1100)); + + // All fresh — should retain both + expect(result.current.remoteCursors).toHaveLength(2); + // Same reference (no unnecessary state update) + expect(result.current.remoteCursors).toBe(before); + }); + + it("cleanup does not run when not connected", () => { + const props = defaultProps(); + const { result } = renderHook(() => useCollaboration(props)); + + // Not connected — no interval should fire, no error + act(() => vi.advanceTimersByTime(5000)); + expect(result.current.remoteCursors).toHaveLength(0); + }); + + it("cleanup fires repeatedly on each interval tick", async () => { + const { result } = await setupAndCreate(); + const handler = channelHandlers["broadcast:cursor-update"]; + const now = Date.now(); + + // Add a cursor that will go stale after first tick + act(() => { + handler({ payload: { peerId: "soon-stale", displayName: "X", color: "#ff0000", x: 10, y: 20, ts: now } }); + }); + expect(result.current.remoteCursors).toHaveLength(1); + + // First tick — still fresh (only 1.1s old vs 4s threshold) + act(() => vi.advanceTimersByTime(1100)); + expect(result.current.remoteCursors).toHaveLength(1); + + // Advance enough for it to go stale (total ~5.1s elapsed) + act(() => vi.advanceTimersByTime(4000)); + expect(result.current.remoteCursors).toHaveLength(0); + }); +}); diff --git a/src/hooks/useHotspotInteraction.js b/src/hooks/useHotspotInteraction.js index 5951bc0..2556d54 100644 --- a/src/hooks/useHotspotInteraction.js +++ b/src/hooks/useHotspotInteraction.js @@ -69,7 +69,7 @@ export function useHotspotInteraction({ } else { setHotspotInteraction({ mode: "selected", screenId, hotspotId }); } - }, [hotspotInteraction, canvasRef, screens, captureDragSnapshot, pan, zoom, selectedHotspots, setSelectedConnection, activeTool]); + }, [hotspotInteraction, canvasRef, screens, pan, zoom, selectedHotspots, setSelectedConnection, activeTool]); const onImageAreaMouseDown = useCallback((e, screenId) => { // Pan tool blocks drawing hotspots