diff --git a/CLAUDE.md b/CLAUDE.md index c97387c..6770bec 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -2,6 +2,13 @@ This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. +**Top-level rules — these override defaults:** + +- **Zero comments** unless explaining a non-obvious *why*. See [Code conventions](#code-conventions). +- **Simple, readable code over clever code.** Prefer 3 obvious lines over 1 dense line. +- **Atomic commits** after each logical unit of work. See [Commits](#commits--atomic-always). +- **Before creating** a new hook, util, or component — grep for an existing one. Follow existing patterns. + ## Project Overview Thinkix is an AI-native infinite canvas whiteboard built with Next.js 16, React 19, and the Plait board library. The AI agent can read the current board, create new structures, and edit existing elements via a Unix-shell-style command set and a custom DSL. The project uses Bun workspaces for shared code and Liveblocks + Yjs for live collaboration. @@ -195,8 +202,91 @@ NEXT_PUBLIC_POSTHOG_HOST=... ## Code conventions -- Self-documenting code; no ceremonial comments. Only comment non-obvious *why*. +### Comments — default is zero + +Write code that does not need comments. Before adding any comment, apply this test: **does it explain WHY something is done this way, in a way the code itself cannot?** If it describes WHAT the code does, delete it and rename variables/functions until the code speaks for itself. + +**Never write comments like these:** + +```ts +// Loop through elements and filter selected ones +const selected = elements.filter(isSelected); + +// Set the active tool to pen +setActiveTool('pen'); + +// Check if the board exists +if (!board) return null; + +/** Handles the click event */ +function handleClick() { ... } + +// Initialize state +const [count, setCount] = useState(0); +``` + +**These are acceptable** (non-obvious why, workaround context, gotcha warnings): + +```ts +// Plait re-orders jsonb keys on write; serialize canonically or prompt +// caching breaks downstream. See #142. +const payload = canonicalize(element); + +// updatePointerType has no native hook — patched in with-tool-sync.ts +BoardTransforms.updatePointerType = patchedUpdatePointerType; + +// Liveblocks presence uses screen coords; we convert to canvas/world +// coords here so cursors render correctly across different viewports. +const worldPos = screenToCanvas(screenPos, viewport); +``` + +JSDoc is allowed only on exported package APIs (`packages/*`), never on internal functions whose name and signature already tell the story. + +### Simplicity over cleverness + +Optimize for the next reader, not for elegance or impressiveness. + +**Do:** +```ts +const activeUsers = users.filter(user => user.isActive); +const names = activeUsers.map(user => user.name); +``` + +**Don't:** +```ts +const names = users.reduce((acc, u) => (u.isActive ? [...acc, u.name] : acc), [] as string[]); +``` + +Rules: +- Prefer a plain `for`/`filter`/`map` chain over a dense one-liner with nested ternaries, `reduce`-to-build-objects, or point-free composition. +- Prefer 3 obvious lines over 1 dense line. Always. +- Prefer duplication over the wrong abstraction — do not invent a helper or generic until the third usage. +- Early returns over nested conditionals: `if (!board) return null` at the top, not an `else` branch at the bottom. +- No nested ternaries. Extract to a variable or use `if`/`else`. +- If you feel the urge to add a comment explaining clever logic — that is the signal to rewrite the logic, not add the comment. + +### Naming + +- Functions: verbs (`createBoard`, `handleClick`, `resolveProvider`). +- Variables/constants: nouns (`activeUsers`, `boardId`, `GRID_SIZE`). +- Booleans: adjectives/state (`isActive`, `hasPermission`, `shouldRender`). +- Names should be specific enough to be grep-able. Avoid single letters except in tight lambda args (`items.map(x => x.id)` is fine; `function f(x, y, z)` is not). + +### Commits — atomic, always + +Commit after each logical unit of work. Do not batch unrelated changes. + +- One logical change per commit. Refactors, behavior changes, and test additions are separate commits even within the same task. +- If a fix requires a preparatory refactor, commit the refactor first (with no behavior change), then commit the fix. +- Conventional commit format: `fix(collab): …`, `feat(agent): …`, `refactor(board): …`, `test: …`, `chore: …`. +- Never mix generated files (`parser.js`) with hand-written changes unless the generation is caused by that change — and say so in the commit body. +- Commit messages: subject says WHAT changed, body (when needed) says WHY. + +### General + - Feature modules expose a small `index.ts` barrel. Follow existing patterns instead of introducing new ones. +- Before creating a new hook, util, or component, grep the codebase for an existing one that does the same thing. - Types flow from `@thinkix/shared` (no JSX) and `shared/constants` (JSX allowed). -- Prefer early returns (`if (!board) return null`). -- Keep PRs focused; if you change the DSL grammar, regenerate the parser before opening the PR (see `CONTRIBUTING.md`). +- `const`-first. Only use `let` when reassignment is genuinely needed. +- Single-responsibility functions. If a function does two things, split it. +- Keep PRs focused; if you change the DSL grammar, regenerate the parser before opening the PR (see `CONTRIBUTING.md`). \ No newline at end of file diff --git a/features/board/components/BoardCanvas.tsx b/features/board/components/BoardCanvas.tsx index 6102a12..1934b0d 100644 --- a/features/board/components/BoardCanvas.tsx +++ b/features/board/components/BoardCanvas.tsx @@ -93,10 +93,11 @@ function RemoteSyncHandler({ useEffect(() => { if (!syncBusContext) return; - + const unsubscribe = syncBusContext.syncBus.subscribeToRemoteChanges((elements: BoardElement[]) => { const normalized = normalizeElements(elements); onElementsChange(normalized); + board.children = normalized; listRender.update(normalized, { board: board, parent: board, @@ -110,7 +111,11 @@ function RemoteSyncHandler({ return null; } -export function BoardCanvas({ +export function BoardCanvas(props: BoardCanvasProps) { + return ; +} + +function BoardCanvasInner({ initialValue, className, children, @@ -131,14 +136,12 @@ export function BoardCanvas({ [boardThemeMode], ); - const initialElements = useMemo(() => { - return syncElementsForBoardTheme( + const [value, setValue] = useState(() => + syncElementsForBoardTheme( boardData?.elements ?? resolvedInitialValue, getBoardThemeMode(boardData?.theme ?? DEFAULT_THEME), - ); - }, [boardData?.elements, boardData?.theme, resolvedInitialValue]); - - const [value, setValue] = useState(initialElements); + ), + ); useEffect(() => { if (boardData) { @@ -146,10 +149,6 @@ export function BoardCanvas({ } }, [boardData, setCurrentBoardId]); - useEffect(() => { - setValue(initialElements); - }, [initialElements]); - useAutoSave({ board: board, enabled: !!boardData, @@ -195,7 +194,6 @@ export function BoardCanvas({ return (
; @@ -123,14 +124,12 @@ export function CursorOverlay({ const viewport = useViewport(board); const [containerRect, setContainerRect] = useState(null); - useEffect(() => { + useEffect(() => { const getContainer = (): Element | null => { if (containerRef?.current) { return containerRef.current; } - // Fallback to DOM query for backward compatibility - const container = document.querySelector('.plait-board-container') as HTMLElement | null; - return container?.querySelector('svg') || container; + return getViewportContainerElement(board); }; const update = () => { @@ -153,7 +152,7 @@ export function CursorOverlay({ window.removeEventListener('resize', update); observer.disconnect(); }; - }, [containerRef]); + }, [containerRef, board]); const renderableCursors = useMemo((): RenderableCursor[] => { if (!containerRect) return []; @@ -172,10 +171,11 @@ export function CursorOverlay({ visible.forEach((cursor, id) => { if (count >= maxCursors) return; + const { x, y } = documentToScreen(cursor.documentX, cursor.documentY, viewport); result.push({ id, - screenX: cursor.documentX * viewport.zoom + viewport.offsetX + containerRect.left, - screenY: cursor.documentY * viewport.zoom + viewport.offsetY + containerRect.top, + screenX: x + containerRect.left, + screenY: y + containerRect.top, userName: cursor.userName, userColor: cursor.userColor, userAvatar: cursor.userAvatar, diff --git a/packages/collaboration/src/cursor-manager.ts b/packages/collaboration/src/cursor-manager.ts index 054760a..c374d7f 100644 --- a/packages/collaboration/src/cursor-manager.ts +++ b/packages/collaboration/src/cursor-manager.ts @@ -1,5 +1,5 @@ import type { Cursor, CollaborationUser } from './types'; -import type { Viewport } from './utils/viewport'; +import { documentToScreen, screenToDocument, type Viewport } from './utils/viewport'; export interface CursorState { userId: string; @@ -57,9 +57,8 @@ export class CursorManager { viewport: Viewport, pointerType: 'mouse' | 'pen' | 'touch' = 'mouse' ): void { - const documentX = (clientX - containerRect.left - viewport.offsetX) / viewport.zoom; - const documentY = (clientY - containerRect.top - viewport.offsetY) / viewport.zoom; - + const { x: documentX, y: documentY } = screenToDocument(clientX, clientY, containerRect, viewport); + this.pendingUpdate = { x: documentX, y: documentY }; this.pendingPointer = pointerType; @@ -150,8 +149,7 @@ export class CursorManager { } getCursorScreenState(cursor: CursorState, viewport: Viewport): CursorState & { screenX: number; screenY: number } { - const screenX = cursor.documentX * viewport.zoom + viewport.offsetX; - const screenY = cursor.documentY * viewport.zoom + viewport.offsetY; + const { x: screenX, y: screenY } = documentToScreen(cursor.documentX, cursor.documentY, viewport); return { ...cursor, screenX, screenY }; } @@ -242,9 +240,8 @@ export function getVisibleCursors( const margin = 100; for (const [id, cursor] of cursors) { - const screenX = cursor.documentX * viewport.zoom + viewport.offsetX; - const screenY = cursor.documentY * viewport.zoom + viewport.offsetY; - + const { x: screenX, y: screenY } = documentToScreen(cursor.documentX, cursor.documentY, viewport); + if (screenX < -margin || screenX > screenWidth + margin) continue; if (screenY < -margin || screenY > screenHeight + margin) continue; diff --git a/packages/collaboration/src/hooks/use-cursor-tracking.ts b/packages/collaboration/src/hooks/use-cursor-tracking.ts index 4a9c4a8..a27af15 100644 --- a/packages/collaboration/src/hooks/use-cursor-tracking.ts +++ b/packages/collaboration/src/hooks/use-cursor-tracking.ts @@ -9,7 +9,12 @@ import { createCursorManager, } from '../cursor-manager'; import type { Cursor, CollaborationUser } from '../types'; -import { getViewport, type Viewport } from '../utils'; +import { + getViewport, + getViewportContainerElement, + documentToScreen, + type Viewport, +} from '../utils'; export interface UseCursorTrackingOptions { board: PlaitBoard | null; @@ -72,11 +77,13 @@ export function useCursorTracking({ const handlePointerMove = (e: PointerEvent) => { const target = e.target; if (!(target instanceof Element)) return; - - const svg = target.closest('svg'); - if (!svg || !boardRef.current) return; - const rect = svg.getBoundingClientRect(); + if (!target.closest('.plait-board-container') || !boardRef.current) return; + + const container = getViewportContainerElement(boardRef.current); + if (!container) return; + + const rect = container.getBoundingClientRect(); const viewport = getViewport(boardRef.current); if (rafId) { @@ -156,9 +163,6 @@ export function useCursorScreenState( cursor: CursorState, viewport: Viewport ): CursorState & { screenX: number; screenY: number } { - return { - ...cursor, - screenX: cursor.documentX * viewport.zoom + viewport.offsetX, - screenY: cursor.documentY * viewport.zoom + viewport.offsetY, - }; + const { x: screenX, y: screenY } = documentToScreen(cursor.documentX, cursor.documentY, viewport); + return { ...cursor, screenX, screenY }; } diff --git a/packages/collaboration/src/hooks/use-sync.ts b/packages/collaboration/src/hooks/use-sync.ts index 708dd2f..64f386d 100644 --- a/packages/collaboration/src/hooks/use-sync.ts +++ b/packages/collaboration/src/hooks/use-sync.ts @@ -4,6 +4,7 @@ import { useEffect, useRef, useCallback } from 'react'; import { useStorage, useMutation, useSelf } from '@liveblocks/react/suspense'; import type { PlaitElement, PlaitBoard } from '@plait/core'; import { usePresence } from '../providers/liveblocks/hooks'; +import { getViewport, getViewportContainerElement, screenToDocument } from '../utils'; interface UseBoardSyncOptions { board: PlaitBoard | null; @@ -70,12 +71,17 @@ export function useBoardCursorTracking(board: PlaitBoard | null, enabled: boolea const handlePointerMove = (e: Event) => { const pointerEvent = e as PointerEvent; const target = pointerEvent.target as SVGElement | HTMLElement; - const svg = target.closest('svg'); - if (!svg) return; + if (!target.closest('.plait-board-container')) return; - const rect = svg.getBoundingClientRect(); - const x = (pointerEvent.clientX - rect.left - board.viewport.offsetX) / board.viewport.zoom; - const y = (pointerEvent.clientY - rect.top - board.viewport.offsetY) / board.viewport.zoom; + const container = getViewportContainerElement(board); + if (!container) return; + + const { x, y } = screenToDocument( + pointerEvent.clientX, + pointerEvent.clientY, + container.getBoundingClientRect(), + getViewport(board) + ); updateCursor({ x, y, pointer: pointerEvent.pointerType as 'mouse' | 'pen' | 'touch' }); }; diff --git a/packages/collaboration/src/hooks/use-viewport.ts b/packages/collaboration/src/hooks/use-viewport.ts index 6095f01..38b372a 100644 --- a/packages/collaboration/src/hooks/use-viewport.ts +++ b/packages/collaboration/src/hooks/use-viewport.ts @@ -4,10 +4,14 @@ import { useEffect, useState, useRef } from 'react'; import type { PlaitBoard } from '@plait/core'; import { getViewport, type Viewport } from '../utils'; -const DEFAULT_VIEWPORT: Viewport = { zoom: 1, offsetX: 0, offsetY: 0 }; +const DEFAULT_VIEWPORT: Viewport = { zoom: 1, originationX: 0, originationY: 0 }; function viewportsEqual(a: Viewport, b: Viewport): boolean { - return a.zoom === b.zoom && a.offsetX === b.offsetX && a.offsetY === b.offsetY; + return ( + a.zoom === b.zoom && + a.originationX === b.originationX && + a.originationY === b.originationY + ); } export function useViewport(board: PlaitBoard | null): Viewport { diff --git a/packages/collaboration/src/utils/index.ts b/packages/collaboration/src/utils/index.ts index b2a8f20..5e198de 100644 --- a/packages/collaboration/src/utils/index.ts +++ b/packages/collaboration/src/utils/index.ts @@ -1,2 +1,8 @@ -export { getViewport, screenToDocument, documentToScreen, type Viewport } from './viewport'; +export { + getViewport, + screenToDocument, + documentToScreen, + getViewportContainerElement, + type Viewport, +} from './viewport'; export { debounce, throttle } from './timing'; diff --git a/packages/collaboration/src/utils/viewport.ts b/packages/collaboration/src/utils/viewport.ts index 1a9532a..ca2a22a 100644 --- a/packages/collaboration/src/utils/viewport.ts +++ b/packages/collaboration/src/utils/viewport.ts @@ -1,16 +1,22 @@ -import type { PlaitBoard } from '@plait/core'; +import { PlaitBoard, getViewportOrigination } from '@plait/core'; export interface Viewport { zoom: number; - offsetX: number; - offsetY: number; + originationX: number; + originationY: number; } export function getViewport(board: PlaitBoard): Viewport { + let origination: [number, number] | undefined; + try { + origination = getViewportOrigination(board); + } catch { + origination = board.viewport?.origination; + } return { zoom: board.viewport?.zoom ?? 1, - offsetX: board.viewport?.offsetX ?? 0, - offsetY: board.viewport?.offsetY ?? 0, + originationX: origination?.[0] ?? 0, + originationY: origination?.[1] ?? 0, }; } @@ -20,8 +26,8 @@ export function screenToDocument( containerRect: DOMRect, viewport: Viewport ): { x: number; y: number } { - const x = (clientX - containerRect.left - viewport.offsetX) / viewport.zoom; - const y = (clientY - containerRect.top - viewport.offsetY) / viewport.zoom; + const x = (clientX - containerRect.left) / viewport.zoom + viewport.originationX; + const y = (clientY - containerRect.top) / viewport.zoom + viewport.originationY; return { x, y }; } @@ -30,7 +36,21 @@ export function documentToScreen( documentY: number, viewport: Viewport ): { x: number; y: number } { - const x = documentX * viewport.zoom + viewport.offsetX; - const y = documentY * viewport.zoom + viewport.offsetY; + const x = (documentX - viewport.originationX) * viewport.zoom; + const y = (documentY - viewport.originationY) * viewport.zoom; return { x, y }; } + +export function getViewportContainerElement(board: PlaitBoard | null): Element | null { + if (board) { + try { + const container = PlaitBoard.getViewportContainer(board); + if (container) return container; + } catch { + } + } + return ( + document.querySelector('.plait-board-container .viewport-container') ?? + document.querySelector('.plait-board-container') + ); +} diff --git a/tests/e2e/shapes.spec.ts b/tests/e2e/shapes.spec.ts index 7d332c6..397f232 100644 --- a/tests/e2e/shapes.spec.ts +++ b/tests/e2e/shapes.spec.ts @@ -205,9 +205,31 @@ test.describe('Shape Drawing E2E Tests', () => { test('should draw an arrow on canvas', async ({ page }) => { await selectTool(page, 'arrow'); await drawShape(page, 100, 100, 300, 200); - + const hasElement = await hasElementOnCanvas(page); expect(hasElement).toBe(true); }); }); + + test.describe('Shape Position Stability', () => { + test('shape stays where drawn after auto-save settles', async ({ page }) => { + await selectTool(page, 'rectangle'); + await drawShape(page, 100, 100, 300, 250); + await clearSelection(page); + + const element = page.locator('.board-wrapper [plait-data-id]').first(); + await element.waitFor({ state: 'attached' }); + const before = await element.boundingBox(); + expect(before).not.toBeNull(); + + await page.waitForTimeout(1500); + + const after = await element.boundingBox(); + expect(after).not.toBeNull(); + expect(Math.abs(after!.x - before!.x)).toBeLessThan(2); + expect(Math.abs(after!.y - before!.y)).toBeLessThan(2); + expect(Math.abs(after!.width - before!.width)).toBeLessThan(2); + expect(Math.abs(after!.height - before!.height)).toBeLessThan(2); + }); + }); }); diff --git a/tests/integration/cursor-collaboration.test.ts b/tests/integration/cursor-collaboration.test.ts index cc2eb94..8ce75ac 100644 --- a/tests/integration/cursor-collaboration.test.ts +++ b/tests/integration/cursor-collaboration.test.ts @@ -13,7 +13,7 @@ describe('Multi-User Cursor Scenarios', () => { let onCursorUpdate: ReturnType; let onCursorsChange: ReturnType; - const viewport: Viewport = { zoom: 1, offsetX: 0, offsetY: 0 }; + const viewport: Viewport = { zoom: 1, originationX: 0, originationY: 0 }; const rect = new DOMRect(0, 0, 800, 600); const users: CollaborationUser[] = [ @@ -119,11 +119,11 @@ describe('Multi-User Cursor Scenarios', () => { describe('Zoom/Pan Correctness', () => { const testViewports: { name: string; viewport: Viewport }[] = [ - { name: 'default', viewport: { zoom: 1, offsetX: 0, offsetY: 0 } }, - { name: 'zoomed in', viewport: { zoom: 2, offsetX: 0, offsetY: 0 } }, - { name: 'zoomed out', viewport: { zoom: 0.5, offsetX: 0, offsetY: 0 } }, - { name: 'panned', viewport: { zoom: 1, offsetX: -500, offsetY: -300 } }, - { name: 'zoomed and panned', viewport: { zoom: 1.5, offsetX: -200, offsetY: -100 } }, + { name: 'default', viewport: { zoom: 1, originationX: 0, originationY: 0 } }, + { name: 'zoomed in', viewport: { zoom: 2, originationX: 0, originationY: 0 } }, + { name: 'zoomed out', viewport: { zoom: 0.5, originationX: 0, originationY: 0 } }, + { name: 'panned', viewport: { zoom: 1, originationX: 500, originationY: 300 } }, + { name: 'zoomed and panned', viewport: { zoom: 1.5, originationX: -200, originationY: -100 } }, ]; testViewports.forEach(({ name, viewport }) => { @@ -140,8 +140,8 @@ describe('Multi-User Cursor Scenarios', () => { expect(cursor?.documentY).toBe(documentY); const screen = manager.getCursorScreenState(cursor!, viewport); - const expectedScreenX = documentX * viewport.zoom + viewport.offsetX; - const expectedScreenY = documentY * viewport.zoom + viewport.offsetY; + const expectedScreenX = (documentX - viewport.originationX) * viewport.zoom; + const expectedScreenY = (documentY - viewport.originationY) * viewport.zoom; expect(screen.screenX).toBe(expectedScreenX); expect(screen.screenY).toBe(expectedScreenY); @@ -151,16 +151,40 @@ describe('Multi-User Cursor Scenarios', () => { it('cursor follows correct transformation path', () => { const clientX = 600; const clientY = 400; - const testViewport: Viewport = { zoom: 2, offsetX: 100, offsetY: 50 }; + const testViewport: Viewport = { zoom: 2, originationX: 100, originationY: 50 }; const doc = screenToDocument(clientX, clientY, rect, testViewport); - expect(doc.x).toBe((600 - 0 - 100) / 2); - expect(doc.y).toBe((400 - 0 - 50) / 2); + expect(doc.x).toBe(600 / 2 + 100); + expect(doc.y).toBe(400 / 2 + 50); const screen = documentToScreen(doc.x, doc.y, testViewport); expect(screen.x).toBeCloseTo(clientX, 5); expect(screen.y).toBeCloseTo(clientY, 5); }); + + it('shows a cursor at the same board point for clients with different zoom, pan, and window size', () => { + const boardPoint = { x: 500, y: 300 }; + + const viewportA: Viewport = { zoom: 0.5, originationX: -400, originationY: -100 }; + const rectA = new DOMRect(0, 0, 1400, 900); + + const viewportB: Viewport = { zoom: 1.5, originationX: 350, originationY: 220 }; + + const clientAX = (boardPoint.x - viewportA.originationX) * viewportA.zoom + rectA.left; + const clientAY = (boardPoint.y - viewportA.originationY) * viewportA.zoom + rectA.top; + + manager.handlePointerMove(clientAX, clientAY, rectA, viewportA); + const wireCursor = onCursorUpdate.mock.lastCall?.[0]; + expect(wireCursor.x).toBeCloseTo(boardPoint.x, 10); + expect(wireCursor.y).toBeCloseTo(boardPoint.y, 10); + + manager.updateRemoteCursor('connA', users[0], wireCursor); + const remote = manager.getAllCursorStates().get('connA'); + const screenB = manager.getCursorScreenState(remote!, viewportB); + + expect(screenB.screenX).toBeCloseTo((boardPoint.x - viewportB.originationX) * viewportB.zoom, 10); + expect(screenB.screenY).toBeCloseTo((boardPoint.y - viewportB.originationY) * viewportB.zoom, 10); + }); }); describe('Idle Timeout', () => { diff --git a/tests/unit/cursor-manager.test.ts b/tests/unit/cursor-manager.test.ts index 8661297..b6242e6 100644 --- a/tests/unit/cursor-manager.test.ts +++ b/tests/unit/cursor-manager.test.ts @@ -30,44 +30,44 @@ globalThis.DOMRect ??= class DOMRect { } as typeof DOMRect; describe('Coordinate Conversion', () => { - const defaultViewport: Viewport = { zoom: 1, offsetX: 0, offsetY: 0 }; + const defaultViewport: Viewport = { zoom: 1, originationX: 0, originationY: 0 }; const defaultRect = new DOMRect(0, 0, 800, 600); - + describe('screenToDocument', () => { it('converts screen coordinates to document coordinates with default viewport', () => { const result = screenToDocument(100, 200, defaultRect, defaultViewport); expect(result.x).toBe(100); expect(result.y).toBe(200); }); - + it('converts screen coordinates with zoom > 1', () => { - const viewport: Viewport = { zoom: 2, offsetX: 0, offsetY: 0 }; + const viewport: Viewport = { zoom: 2, originationX: 0, originationY: 0 }; const result = screenToDocument(200, 400, defaultRect, viewport); expect(result.x).toBe(100); expect(result.y).toBe(200); }); - + it('converts screen coordinates with zoom < 1', () => { - const viewport: Viewport = { zoom: 0.5, offsetX: 0, offsetY: 0 }; + const viewport: Viewport = { zoom: 0.5, originationX: 0, originationY: 0 }; const result = screenToDocument(100, 200, defaultRect, viewport); expect(result.x).toBe(200); expect(result.y).toBe(400); }); - - it('accounts for pan offset', () => { - const viewport: Viewport = { zoom: 1, offsetX: -500, offsetY: -300 }; + + it('accounts for pan (origination)', () => { + const viewport: Viewport = { zoom: 1, originationX: 500, originationY: 300 }; const result = screenToDocument(600, 400, defaultRect, viewport); expect(result.x).toBe(1100); expect(result.y).toBe(700); }); - + it('accounts for both zoom and pan', () => { - const viewport: Viewport = { zoom: 2, offsetX: -200, offsetY: -100 }; + const viewport: Viewport = { zoom: 2, originationX: 100, originationY: 50 }; const result = screenToDocument(400, 300, defaultRect, viewport); expect(result.x).toBe(300); expect(result.y).toBe(200); }); - + it('accounts for container rect offset', () => { const rect = new DOMRect(100, 50, 800, 600); const result = screenToDocument(200, 150, rect, defaultViewport); @@ -75,46 +75,46 @@ describe('Coordinate Conversion', () => { expect(result.y).toBe(100); }); }); - + describe('documentToScreen', () => { it('converts document coordinates to screen coordinates with default viewport', () => { const result = documentToScreen(100, 200, defaultViewport); expect(result.x).toBe(100); expect(result.y).toBe(200); }); - + it('converts document coordinates with zoom > 1', () => { - const viewport: Viewport = { zoom: 2, offsetX: 0, offsetY: 0 }; + const viewport: Viewport = { zoom: 2, originationX: 0, originationY: 0 }; const result = documentToScreen(100, 200, viewport); expect(result.x).toBe(200); expect(result.y).toBe(400); }); - + it('converts document coordinates with zoom < 1', () => { - const viewport: Viewport = { zoom: 0.5, offsetX: 0, offsetY: 0 }; + const viewport: Viewport = { zoom: 0.5, originationX: 0, originationY: 0 }; const result = documentToScreen(200, 400, viewport); expect(result.x).toBe(100); expect(result.y).toBe(200); }); - - it('accounts for pan offset', () => { - const viewport: Viewport = { zoom: 1, offsetX: 100, offsetY: 50 }; + + it('accounts for pan (origination)', () => { + const viewport: Viewport = { zoom: 1, originationX: 100, originationY: 50 }; const result = documentToScreen(500, 300, viewport); - expect(result.x).toBe(600); - expect(result.y).toBe(350); + expect(result.x).toBe(400); + expect(result.y).toBe(250); }); - + it('accounts for both zoom and pan', () => { - const viewport: Viewport = { zoom: 2, offsetX: 100, offsetY: 50 }; + const viewport: Viewport = { zoom: 2, originationX: 100, originationY: 50 }; const result = documentToScreen(200, 150, viewport); - expect(result.x).toBe(500); - expect(result.y).toBe(350); + expect(result.x).toBe(200); + expect(result.y).toBe(200); }); }); - + describe('round-trip conversion', () => { it('maintains coordinates through round-trip conversion', () => { - const viewport: Viewport = { zoom: 1.5, offsetX: 200, offsetY: 100 }; + const viewport: Viewport = { zoom: 1.5, originationX: 200, originationY: 100 }; const originalDocX = 500; const originalDocY = 350; @@ -183,7 +183,7 @@ describe('CursorManager', () => { describe('throttling', () => { const throttleMs = 50; const rect = new DOMRect(0, 0, 800, 600); - const viewport: Viewport = { zoom: 1, offsetX: 0, offsetY: 0 }; + const viewport: Viewport = { zoom: 1, originationX: 0, originationY: 0 }; beforeEach(() => { manager = createCursorManager(onCursorUpdate, onCursorsChange, { @@ -337,7 +337,7 @@ describe('CursorManager', () => { describe('cursor screen state conversion', () => { const user1: CollaborationUser = { id: 'user1', name: 'Alice', color: '#FF0000' }; - const viewport: Viewport = { zoom: 2, offsetX: 100, offsetY: 50 }; + const viewport: Viewport = { zoom: 2, originationX: 100, originationY: 50 }; beforeEach(() => { manager = createCursorManager(onCursorUpdate, onCursorsChange); @@ -352,14 +352,14 @@ describe('CursorManager', () => { expect(cursor).toBeDefined(); const screenState = manager.getCursorScreenState(cursor!, viewport); - expect(screenState.screenX).toBe(500); - expect(screenState.screenY).toBe(350); + expect(screenState.screenX).toBe(200); + expect(screenState.screenY).toBe(200); }); }); }); describe('getVisibleCursors', () => { - const viewport: Viewport = { zoom: 1, offsetX: 0, offsetY: 0 }; + const viewport: Viewport = { zoom: 1, originationX: 0, originationY: 0 }; const screenWidth = 800; const screenHeight = 600; @@ -422,7 +422,7 @@ describe('getVisibleCursors', () => { }); it('accounts for zoom in visibility calculation', () => { - const zoomedViewport: Viewport = { zoom: 2, offsetX: 0, offsetY: 0 }; + const zoomedViewport: Viewport = { zoom: 2, originationX: 0, originationY: 0 }; const cursors = new Map(); cursors.set('1', createCursorState('1', 200, 150)); @@ -431,7 +431,7 @@ describe('getVisibleCursors', () => { }); it('accounts for pan offset in visibility calculation', () => { - const pannedViewport: Viewport = { zoom: 1, offsetX: -500, offsetY: -300 }; + const pannedViewport: Viewport = { zoom: 1, originationX: 500, originationY: 300 }; const cursors = new Map(); cursors.set('visible', createCursorState('visible', 600, 400)); cursors.set('hidden', createCursorState('hidden', 100, 100)); diff --git a/tests/unit/cursor-overlay.test.tsx b/tests/unit/cursor-overlay.test.tsx index 2366e76..0bd2636 100644 --- a/tests/unit/cursor-overlay.test.tsx +++ b/tests/unit/cursor-overlay.test.tsx @@ -7,8 +7,7 @@ import type { CursorState } from '@thinkix/collaboration'; interface MockBoard { viewport: { zoom: number; - offsetX: number; - offsetY: number; + origination?: [number, number]; }; } @@ -16,8 +15,7 @@ describe('CursorOverlay', () => { const mockBoard: MockBoard = { viewport: { zoom: 1, - offsetX: 0, - offsetY: 0, + origination: [0, 0], }, }; @@ -36,15 +34,18 @@ describe('CursorOverlay', () => { vi.clearAllMocks(); const svg = document.createElementNS('http://www.w3.org/2000/svg', 'svg'); + const viewportContainer = document.createElement('div'); + viewportContainer.className = 'viewport-container'; const container = document.createElement('div'); container.className = 'plait-board-container'; - container.appendChild(svg); + viewportContainer.appendChild(svg); + container.appendChild(viewportContainer); document.body.appendChild(container); - - svg.getBoundingClientRect = () => new DOMRect(0, 0, 800, 600); - + + viewportContainer.getBoundingClientRect = () => new DOMRect(0, 0, 800, 600); + vi.spyOn(document, 'querySelector').mockImplementation((selector) => { - if (selector.includes('svg')) return svg; + if (selector.includes('viewport-container')) return viewportContainer; if (selector.includes('plait-board-container')) return container; return null; }); diff --git a/tests/unit/use-cursor-tracking.test.tsx b/tests/unit/use-cursor-tracking.test.tsx index 5b0fd0f..da510d9 100644 --- a/tests/unit/use-cursor-tracking.test.tsx +++ b/tests/unit/use-cursor-tracking.test.tsx @@ -5,8 +5,7 @@ import type { PlaitBoard } from '@plait/core'; interface MockBoard { viewport: { zoom: number; - offsetX: number; - offsetY: number; + origination?: [number, number]; }; } @@ -15,9 +14,9 @@ vi.mock('@thinkix/collaboration/hooks/use-cursor-tracking', () => ({ cursors: enabled ? new Map() : new Map(), updateMyCursor: vi.fn(), }), - useCursorScreenState: (cursor: { documentX: number; documentY: number }, viewport: { zoom: number; offsetX: number; offsetY: number }) => ({ - screenX: cursor.documentX * viewport.zoom + viewport.offsetX, - screenY: cursor.documentY * viewport.zoom + viewport.offsetY, + useCursorScreenState: (cursor: { documentX: number; documentY: number }, viewport: { zoom: number; originationX: number; originationY: number }) => ({ + screenX: (cursor.documentX - viewport.originationX) * viewport.zoom, + screenY: (cursor.documentY - viewport.originationY) * viewport.zoom, }), })); @@ -27,8 +26,7 @@ describe('useCursorTracking', () => { const mockBoard: MockBoard = { viewport: { zoom: 1, - offsetX: 0, - offsetY: 0, + origination: [0, 0], }, }; @@ -124,14 +122,14 @@ describe('useCursorScreenState', () => { lastUpdated: Date.now(), pointer: 'mouse', }; - const viewport = { zoom: 2, offsetX: 50, offsetY: 25 }; - - const { result } = renderHook(() => + const viewport = { zoom: 2, originationX: 50, originationY: 25 }; + + const { result } = renderHook(() => useCursorScreenState(cursor as MockCursor, viewport) ); - - expect(result.current.screenX).toBe(250); - expect(result.current.screenY).toBe(425); + + expect(result.current.screenX).toBe(100); + expect(result.current.screenY).toBe(350); }); it('handles zoom < 1', () => { @@ -144,7 +142,7 @@ describe('useCursorScreenState', () => { lastUpdated: Date.now(), pointer: 'mouse', }; - const viewport = { zoom: 0.5, offsetX: 0, offsetY: 0 }; + const viewport = { zoom: 0.5, originationX: 0, originationY: 0 }; const { result } = renderHook(() => useCursorScreenState(cursor as MockCursor, viewport) diff --git a/tests/unit/viewport.test.ts b/tests/unit/viewport.test.ts index 7b93682..2f2d08a 100644 --- a/tests/unit/viewport.test.ts +++ b/tests/unit/viewport.test.ts @@ -4,20 +4,19 @@ import type { PlaitBoard } from '@plait/core'; describe('Viewport Utilities', () => { describe('getViewport', () => { - it('returns viewport from board', () => { + it('returns zoom and origination from board viewport', () => { const mockBoard = { viewport: { zoom: 1.5, - offsetX: 100, - offsetY: 200, + origination: [100, 200], }, } as unknown as PlaitBoard; const result = getViewport(mockBoard); expect(result.zoom).toBe(1.5); - expect(result.offsetX).toBe(100); - expect(result.offsetY).toBe(200); + expect(result.originationX).toBe(100); + expect(result.originationY).toBe(200); }); it('returns default values when viewport is undefined', () => { @@ -26,11 +25,11 @@ describe('Viewport Utilities', () => { const result = getViewport(mockBoard); expect(result.zoom).toBe(1); - expect(result.offsetX).toBe(0); - expect(result.offsetY).toBe(0); + expect(result.originationX).toBe(0); + expect(result.originationY).toBe(0); }); - it('handles partial viewport object', () => { + it('defaults origination to zero when board has not initialized it', () => { const mockBoard = { viewport: { zoom: 2, @@ -40,16 +39,15 @@ describe('Viewport Utilities', () => { const result = getViewport(mockBoard); expect(result.zoom).toBe(2); - expect(result.offsetX).toBe(0); - expect(result.offsetY).toBe(0); + expect(result.originationX).toBe(0); + expect(result.originationY).toBe(0); }); it('handles extreme zoom values', () => { const mockBoard = { viewport: { zoom: 0.001, - offsetX: 0, - offsetY: 0, + origination: [0, 0], }, } as unknown as PlaitBoard; @@ -58,24 +56,23 @@ describe('Viewport Utilities', () => { expect(result.zoom).toBe(0.001); }); - it('handles large offset values', () => { + it('handles large origination values', () => { const mockBoard = { viewport: { zoom: 1, - offsetX: -100000, - offsetY: 100000, + origination: [-100000, 100000], }, } as unknown as PlaitBoard; const result = getViewport(mockBoard); - expect(result.offsetX).toBe(-100000); - expect(result.offsetY).toBe(100000); + expect(result.originationX).toBe(-100000); + expect(result.originationY).toBe(100000); }); }); describe('screenToDocument', () => { - const defaultViewport: Viewport = { zoom: 1, offsetX: 0, offsetY: 0 }; + const defaultViewport: Viewport = { zoom: 1, originationX: 0, originationY: 0 }; const defaultRect = new DOMRect(0, 0, 800, 600); it('converts screen coordinates with default viewport', () => { @@ -86,7 +83,7 @@ describe('Viewport Utilities', () => { }); it('converts with zoom > 1', () => { - const viewport: Viewport = { zoom: 2, offsetX: 0, offsetY: 0 }; + const viewport: Viewport = { zoom: 2, originationX: 0, originationY: 0 }; const result = screenToDocument(200, 400, defaultRect, viewport); expect(result.x).toBe(100); @@ -94,27 +91,27 @@ describe('Viewport Utilities', () => { }); it('converts with zoom < 1', () => { - const viewport: Viewport = { zoom: 0.5, offsetX: 0, offsetY: 0 }; + const viewport: Viewport = { zoom: 0.5, originationX: 0, originationY: 0 }; const result = screenToDocument(100, 200, defaultRect, viewport); expect(result.x).toBe(200); expect(result.y).toBe(400); }); - it('accounts for positive offset', () => { - const viewport: Viewport = { zoom: 1, offsetX: 100, offsetY: 50 }; + it('accounts for positive origination (panned right/down)', () => { + const viewport: Viewport = { zoom: 1, originationX: 100, originationY: 50 }; const result = screenToDocument(200, 150, defaultRect, viewport); - expect(result.x).toBe(100); - expect(result.y).toBe(100); + expect(result.x).toBe(300); + expect(result.y).toBe(200); }); - it('accounts for negative offset', () => { - const viewport: Viewport = { zoom: 1, offsetX: -100, offsetY: -50 }; + it('accounts for negative origination (panned left/up)', () => { + const viewport: Viewport = { zoom: 1, originationX: -100, originationY: -50 }; const result = screenToDocument(100, 100, defaultRect, viewport); - expect(result.x).toBe(200); - expect(result.y).toBe(150); + expect(result.x).toBe(0); + expect(result.y).toBe(50); }); it('accounts for container rect offset', () => { @@ -125,12 +122,12 @@ describe('Viewport Utilities', () => { expect(result.y).toBe(100); }); - it('handles combined zoom and offset', () => { - const viewport: Viewport = { zoom: 2, offsetX: -200, offsetY: -100 }; + it('handles combined zoom and origination', () => { + const viewport: Viewport = { zoom: 2, originationX: -200, originationY: -100 }; const result = screenToDocument(400, 300, defaultRect, viewport); - expect(result.x).toBe(300); - expect(result.y).toBe(200); + expect(result.x).toBe(0); + expect(result.y).toBe(50); }); it('handles zero screen coordinates', () => { @@ -148,11 +145,11 @@ describe('Viewport Utilities', () => { }); it('handles floating point coordinates', () => { - const viewport: Viewport = { zoom: 1.5, offsetX: 10.5, offsetY: 20.5 }; + const viewport: Viewport = { zoom: 1.5, originationX: 10.5, originationY: 20.5 }; const result = screenToDocument(100.25, 200.75, defaultRect, viewport); - expect(result.x).toBeCloseTo((100.25 - 10.5) / 1.5, 10); - expect(result.y).toBeCloseTo((200.75 - 20.5) / 1.5, 10); + expect(result.x).toBeCloseTo(100.25 / 1.5 + 10.5, 10); + expect(result.y).toBeCloseTo(200.75 / 1.5 + 20.5, 10); }); it('handles very large screen coordinates', () => { @@ -164,7 +161,7 @@ describe('Viewport Utilities', () => { }); describe('documentToScreen', () => { - const defaultViewport: Viewport = { zoom: 1, offsetX: 0, offsetY: 0 }; + const defaultViewport: Viewport = { zoom: 1, originationX: 0, originationY: 0 }; it('converts document coordinates with default viewport', () => { const result = documentToScreen(100, 200, defaultViewport); @@ -174,7 +171,7 @@ describe('Viewport Utilities', () => { }); it('converts with zoom > 1', () => { - const viewport: Viewport = { zoom: 2, offsetX: 0, offsetY: 0 }; + const viewport: Viewport = { zoom: 2, originationX: 0, originationY: 0 }; const result = documentToScreen(100, 200, viewport); expect(result.x).toBe(200); @@ -182,35 +179,35 @@ describe('Viewport Utilities', () => { }); it('converts with zoom < 1', () => { - const viewport: Viewport = { zoom: 0.5, offsetX: 0, offsetY: 0 }; + const viewport: Viewport = { zoom: 0.5, originationX: 0, originationY: 0 }; const result = documentToScreen(200, 400, viewport); expect(result.x).toBe(100); expect(result.y).toBe(200); }); - it('accounts for positive offset', () => { - const viewport: Viewport = { zoom: 1, offsetX: 100, offsetY: 50 }; + it('accounts for positive origination', () => { + const viewport: Viewport = { zoom: 1, originationX: 100, originationY: 50 }; const result = documentToScreen(100, 200, viewport); - expect(result.x).toBe(200); - expect(result.y).toBe(250); + expect(result.x).toBe(0); + expect(result.y).toBe(150); }); - it('accounts for negative offset', () => { - const viewport: Viewport = { zoom: 1, offsetX: -100, offsetY: -50 }; + it('accounts for negative origination', () => { + const viewport: Viewport = { zoom: 1, originationX: -100, originationY: -50 }; const result = documentToScreen(100, 200, viewport); - expect(result.x).toBe(0); - expect(result.y).toBe(150); + expect(result.x).toBe(200); + expect(result.y).toBe(250); }); - it('handles combined zoom and offset', () => { - const viewport: Viewport = { zoom: 2, offsetX: 100, offsetY: 50 }; + it('handles combined zoom and origination', () => { + const viewport: Viewport = { zoom: 2, originationX: 100, originationY: 50 }; const result = documentToScreen(200, 150, viewport); - expect(result.x).toBe(500); - expect(result.y).toBe(350); + expect(result.x).toBe(200); + expect(result.y).toBe(200); }); it('handles zero document coordinates', () => { @@ -228,21 +225,21 @@ describe('Viewport Utilities', () => { }); it('handles floating point coordinates', () => { - const viewport: Viewport = { zoom: 1.5, offsetX: 10.5, offsetY: 20.5 }; + const viewport: Viewport = { zoom: 1.5, originationX: 10.5, originationY: 20.5 }; const result = documentToScreen(100.25, 200.75, viewport); - expect(result.x).toBe(100.25 * 1.5 + 10.5); - expect(result.y).toBe(200.75 * 1.5 + 20.5); + expect(result.x).toBeCloseTo((100.25 - 10.5) * 1.5, 10); + expect(result.y).toBeCloseTo((200.75 - 20.5) * 1.5, 10); }); }); describe('round-trip conversion', () => { it('maintains coordinates through screen -> document -> screen round-trip', () => { const viewports: Viewport[] = [ - { zoom: 1, offsetX: 0, offsetY: 0 }, - { zoom: 2, offsetX: 100, offsetY: 50 }, - { zoom: 0.5, offsetX: -200, offsetY: -100 }, - { zoom: 1.5, offsetX: 0, offsetY: 0 }, + { zoom: 1, originationX: 0, originationY: 0 }, + { zoom: 2, originationX: 100, originationY: 50 }, + { zoom: 0.5, originationX: -200, originationY: -100 }, + { zoom: 1.5, originationX: 0, originationY: 0 }, ]; const rect = new DOMRect(0, 0, 800, 600); @@ -261,9 +258,9 @@ describe('Viewport Utilities', () => { it('maintains coordinates through document -> screen -> document round-trip', () => { const viewports: Viewport[] = [ - { zoom: 1, offsetX: 0, offsetY: 0 }, - { zoom: 2, offsetX: 100, offsetY: 50 }, - { zoom: 0.5, offsetX: -200, offsetY: -100 }, + { zoom: 1, originationX: 0, originationY: 0 }, + { zoom: 2, originationX: 100, originationY: 50 }, + { zoom: 0.5, originationX: -200, originationY: -100 }, ]; const rect = new DOMRect(0, 0, 800, 600); @@ -280,4 +277,35 @@ describe('Viewport Utilities', () => { }); }); }); + + describe('cross-client consistency', () => { + it('a point captured in one client projects onto the same board point in a client with different zoom, pan, and window size', () => { + const boardPoint = { x: 500, y: 300 }; + + const viewportA: Viewport = { zoom: 0.5, originationX: -400, originationY: -100 }; + const rectA = new DOMRect(0, 0, 1400, 900); + + const viewportB: Viewport = { zoom: 1.5, originationX: 350, originationY: 220 }; + const rectB = new DOMRect(60, 40, 900, 700); + + const clientA = { + x: (boardPoint.x - viewportA.originationX) * viewportA.zoom + rectA.left, + y: (boardPoint.y - viewportA.originationY) * viewportA.zoom + rectA.top, + }; + + const wire = screenToDocument(clientA.x, clientA.y, rectA, viewportA); + expect(wire.x).toBeCloseTo(boardPoint.x, 10); + expect(wire.y).toBeCloseTo(boardPoint.y, 10); + + const screenB = documentToScreen(wire.x, wire.y, viewportB); + expect(screenB.x + rectB.left).toBeCloseTo( + (boardPoint.x - viewportB.originationX) * viewportB.zoom + rectB.left, + 10 + ); + expect(screenB.y + rectB.top).toBeCloseTo( + (boardPoint.y - viewportB.originationY) * viewportB.zoom + rectB.top, + 10 + ); + }); + }); });