refactor whiteboard by alexmc2 · Pull Request #1 · alexmc2/code-share

alexmc2 · 2026-02-06T15:21:56Z

No description provided.

Copilot

Pull request overview

Refactors the whiteboard implementation by splitting the previously monolithic src/components/Whiteboard.tsx into a dedicated src/components/whiteboard/ module with focused hooks/utilities, and updates the session layout to use the new module and tab key.

Changes:

Extracted whiteboard rendering, viewport (pan/zoom), pointer handling, drawing, and undo/redo into separate hooks/modules under src/components/whiteboard/.
Introduced layered-canvas rendering utilities (drawing.ts, flood-fill.ts) and shared whiteboard types/constants (types.ts).
Rewired SessionLayout to import the new whiteboard module and renamed the tab from diagram to whiteboard.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/components/whiteboard/useWhiteboardCanvas.ts	Implements layered/offscreen canvas rebuild + viewport rendering + resize handling.
src/components/whiteboard/useViewport.ts	Encapsulates viewport transform, clamping, recentering, and touch geometry helpers.
src/components/whiteboard/useUndoRedo.ts	Provides local undo/redo stacks and keyboard shortcuts tied to the shared Yjs ops array.
src/components/whiteboard/usePointerHandlers.ts	Centralizes pointer event routing for drawing vs touch pan/zoom.
src/components/whiteboard/useDrawing.ts	Handles tool-based op creation, live preview state, and committing ops to Yjs.
src/components/whiteboard/types.ts	Defines whiteboard op schema, tools, constants (sizes, world dimensions, zoom limits).
src/components/whiteboard/index.ts	Barrel export for `Whiteboard`.
src/components/whiteboard/flood-fill.ts	Implements boundary-aware flood fill to avoid anti-aliasing artifacts.
src/components/whiteboard/drawing.ts	Extracted stroke/eraser drawing primitives used across rendering paths.
src/components/whiteboard/Whiteboard.tsx	New top-level whiteboard component composing the new hooks and toolbar.
src/components/whiteboard/Toolbar.tsx	Extracted UI toolbar for tools/colors/sizes/undo/redo/clear.
src/components/Whiteboard.tsx	Removes the prior monolithic whiteboard implementation.
src/components/SessionLayout.tsx	Updates import to `./whiteboard` and renames the tab key to `whiteboard`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-06T15:27:09Z

src/components/whiteboard/useUndoRedo.ts

+  const [canUndo, setCanUndo] = useState(false);
+  const [canRedo, setCanRedo] = useState(false);
+  const undoStack = useRef<DrawOp[]>([]);
+  const redoStack = useRef<DrawOp[]>([]);
+


Undo/redo state is purely local and never reconciled with remote Yjs changes. For example, if another participant clears the board (ops array emptied), undoStack/canUndo on this client can remain enabled even though undo can no longer find those ops in opsArray. Consider observing opsArray inside this hook and resetting local undo/redo state when the array is cleared or otherwise diverges significantly from the local stacks.

Copilot · 2026-02-06T15:27:09Z

src/components/whiteboard/Whiteboard.tsx

+  // Generate custom round cursor for pen and eraser tools
+  const brushCursor = useMemo(() => {
+    if (tool !== 'eraser' && tool !== 'pen') return 'crosshair';
+
+    const screenSize = Math.max(
+      8,
+      Math.min(128, size * viewport.transformRef.current.scale),
+    );
+    const halfSize = screenSize / 2;
+
+    const svg = `
+      <svg xmlns="http://www.w3.org/2000/svg" width="${screenSize}" height="${screenSize}" viewBox="0 0 ${screenSize} ${screenSize}">
+        <circle cx="${halfSize}" cy="${halfSize}" r="${halfSize - 1}" fill="none" stroke="rgba(128,128,128,0.8)" stroke-width="2"/>
+        <circle cx="${halfSize}" cy="${halfSize}" r="1" fill="rgba(128,128,128,0.8)"/>
+      </svg>
+    `.trim();
+
+    const dataUrl = `data:image/svg+xml;base64,${btoa(svg)}`;
+    return `url(${dataUrl}) ${halfSize} ${halfSize}, crosshair`;
+  }, [tool, size, viewport.transformRef]);


brushCursor is derived from viewport.transformRef.current.scale, but transformRef is mutated via refs (no React state), so zooming/panning won’t trigger a re-render and this memo won’t recompute. As a result the cursor size won’t actually scale with zoom as intended. Consider either computing the cursor string without useMemo, or introducing a piece of state that updates when scale changes (e.g., in the pinch/zoom handler) and using that state as a dependency.

Copilot · 2026-02-06T15:27:10Z

src/components/whiteboard/usePointerHandlers.ts

+  // Drawing state
+  isDrawingRef: React.RefObject<boolean>,
+  currentOpRef: React.RefObject<unknown>,
+  // Drawing handlers
+  handleStart: (e: React.PointerEvent) => void,
+  handleMove: (e: React.PointerEvent) => void,
+  handleEnd: () => void,


currentOpRef is typed as React.RefObject<unknown>, but this hook assigns null and the canvas renderer expects a DrawOp | null. Using unknown here loses type safety and can mask mistakes. Consider changing this parameter type to React.RefObject<DrawOp | null> (and importing the DrawOp type) to keep the drawing pipeline consistent.

Copilot · 2026-02-06T15:27:10Z

src/components/whiteboard/useWhiteboardCanvas.ts

+  // Track if world canvas needs rebuild
+  const worldNeedsRebuildRef = useRef(true);
+


worldNeedsRebuildRef is only ever written to (set true/false) and never read. This adds noise and suggests there’s missing logic (e.g., skipping rebuilds when not needed). Either remove it, or use it to guard expensive rebuilds so repeated calls (theme + ops updates) don’t always replay the full op log.

Copilot · 2026-02-06T15:27:10Z

src/components/whiteboard/useWhiteboardCanvas.ts

+  // Rebuild world canvas on data changes
+  useEffect(() => {
+    worldNeedsRebuildRef.current = true;
+    rebuildWorldCanvas();
+    scheduleViewportRender();
+  }, [opsArray, rebuildWorldCanvas, scheduleViewportRender]);
+
+  // Re-render on theme change
+  useEffect(() => {
+    worldNeedsRebuildRef.current = true;
+    rebuildWorldCanvas();
+    scheduleViewportRender();
+  }, [isDark, rebuildWorldCanvas, scheduleViewportRender]);
+
+  // Handle resize


On initial mount, both the “data changes” effect and the “theme change” effect run, causing rebuildWorldCanvas() + scheduleViewportRender() to execute twice back-to-back. Consider merging these into a single effect (e.g., keyed on [opsArray, isDark]) or adding a small guard so the expensive canvas replay happens only once per commit.

Suggested change

// Rebuild world canvas on data changes

useEffect(() => {

worldNeedsRebuildRef.current = true;

rebuildWorldCanvas();

scheduleViewportRender();

}, [opsArray, rebuildWorldCanvas, scheduleViewportRender]);

// Re-render on theme change

useEffect(() => {

worldNeedsRebuildRef.current = true;

rebuildWorldCanvas();

scheduleViewportRender();

}, [isDark, rebuildWorldCanvas, scheduleViewportRender]);

// Handle resize

// Rebuild world canvas when data or theme changes

useEffect(() => {

worldNeedsRebuildRef.current = true;

rebuildWorldCanvas();

scheduleViewportRender();

}, [opsArray, isDark, rebuildWorldCanvas, scheduleViewportRender]);

// Handle resize

useLayoutEffect(() => {

resizeCanvas();

updateViewportForResize();

scheduleViewportRender();

const container = containerRef.current;

if (!container) return;

const resizeObserver = new ResizeObserver(() => {

resizeCanvas();

updateViewportForResize();

scheduleViewportRender();

});

resizeObserver.observe(container);

return () => {

resizeObserver.disconnect();

if (rafIdRef.current !== null) {

cancelAnimationFrame(rafIdRef.current);

}

};

}, [

resizeCanvas,

updateViewportForResize,

scheduleViewportRender,

containerRef,

]);

Copilot · 2026-02-06T15:27:11Z

src/components/whiteboard/flood-fill.ts

+  // Convert fill color to RGBA
+  const tempCanvas = document.createElement('canvas');
+  tempCanvas.width = 1;
+  tempCanvas.height = 1;
+  const tempCtx = tempCanvas.getContext('2d')!;
+  tempCtx.fillStyle = fillColor;
+  tempCtx.fillRect(0, 0, 1, 1);
+  const fillRGBA = tempCtx.getImageData(0, 0, 1, 1).data;
+


floodFillWithBoundary creates a new 1x1 <canvas> on every fill to parse the color. Creating DOM nodes per fill can become a hotspot. Consider caching a single tiny canvas/context (module-level or via closure) or using a lightweight color parser so repeated fills don’t allocate DOM resources.

cloudflare-workers-and-pages · 2026-02-06T15:44:31Z

Deploying code-share with Cloudflare Pages

Latest commit:	`9a43b94`
Status:	✅ Deploy successful!
Preview URL:	https://550fb34f.code-share-44l.pages.dev
Branch Preview URL:	https://whiteboard-refactor.code-share-44l.pages.dev

View logs

Copilot

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-06T19:27:14Z

src/components/whiteboard/Toolbar.tsx

+        <button
+          className={toolButtonClass(tool === 'pen')}
+          onClick={() => setTool('pen')}
+          title="Pen"
+        >
+          ✏️
+        </button>
+        <button
+          className={toolButtonClass(tool === 'line')}
+          onClick={() => setTool('line')}
+          title="Line"
+        >
+          ╱
+        </button>
+        <button
+          className={toolButtonClass(tool === 'rect')}
+          onClick={() => setTool('rect')}
+          title="Rectangle"
+        >
+          ▢
+        </button>
+        <button
+          className={toolButtonClass(tool === 'circle')}
+          onClick={() => setTool('circle')}
+          title="Circle"
+        >
+          ◯
+        </button>
+        <button
+          className={toolButtonClass(tool === 'eraser')}
+          onClick={() => setTool('eraser')}
+          title="Eraser (select and delete)"
+        >
+          🧹
+        </button>
+        <button
+          className={toolButtonClass(tool === 'fill')}
+          onClick={() => setTool('fill')}
+          title="Fill Bucket"
+        >
+          🪣
+        </button>


Toolbar icon-only controls rely on title but do not provide aria-label/aria-pressed, which makes the tool picker hard to use with screen readers. Please add accessible names (and consider aria-pressed for the active tool) to these buttons.

Copilot · 2026-02-06T19:27:14Z

src/components/whiteboard/Toolbar.tsx

+              focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-primary`}
+            style={{ backgroundColor: c }}
+            onClick={() => setColour(c)}
+            title={c}


The colour swatch buttons are purely visual; they should have an accessible name (e.g., aria-label) and ideally communicate selection state (e.g., aria-pressed/aria-selected) rather than relying on border styling alone.

Suggested change

title={c}

title={c}

aria-label={c}

aria-pressed={colour === c}

Copilot · 2026-02-06T19:27:14Z

src/components/whiteboard/Toolbar.tsx

+        <button
+          className={toolButtonClass(tool === 'eraser')}
+          onClick={() => setTool('eraser')}
+          title="Eraser (select and delete)"


The eraser tooltip says "Eraser (select and delete)", but the current implementation is a brush-style stroke eraser (eraseStroke). Updating the tooltip text will avoid confusing users about what the tool does.

Suggested change

title="Eraser (select and delete)"

title="Eraser (brush-style)"

Copilot · 2026-02-06T19:27:15Z

src/components/whiteboard/useDrawing.ts

+  currentOpRef: React.RefObject<DrawOp | null>,
+): DrawingState {
+  const isDrawing = useRef(false);
+  const startPoint = useRef<Point>({ x: 0, y: 0 });


startPoint is written in handleStart but never read anywhere in this hook. If it’s not needed anymore, removing it will reduce state/complexity; if it is needed for shapes later, consider implementing the dependent logic now or adding a comment explaining the planned usage.

Suggested change

const startPoint = useRef<Point>({ x: 0, y: 0 });

refactor whiteboard

61802ac

alexmc2 requested a review from Copilot February 6, 2026 15:22

Copilot started reviewing on behalf of alexmc2 February 6, 2026 15:22 View session

Copilot AI reviewed Feb 6, 2026

View reviewed changes

add copilot code review instructions

815ef74

implement code review recommendations

e21e2bd

alexmc2 requested a review from Copilot February 6, 2026 19:23

Copilot started reviewing on behalf of alexmc2 February 6, 2026 19:23 View session

Copilot AI reviewed Feb 6, 2026

View reviewed changes

remove dead code in useDrawing.ts

9a43b94

alexmc2 merged commit fff2ee6 into main Feb 6, 2026
2 checks passed

alexmc2 deleted the whiteboard-refactor branch February 6, 2026 19:36

		// Track if world canvas needs rebuild
		const worldNeedsRebuildRef = useRef(true);

	title="Eraser (select and delete)"
	title="Eraser (brush-style)"

Conversation

alexmc2 commented Feb 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

cloudflare-workers-and-pages bot commented Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deploying code-share with Cloudflare Pages

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

cloudflare-workers-and-pages bot commented Feb 6, 2026 •

edited

Loading