From 86f17ea72647f1c49be97342f9a6ac3dc29142ec Mon Sep 17 00:00:00 2001 From: Daniele Cammareri Date: Tue, 10 Mar 2026 23:17:33 +0100 Subject: [PATCH 1/3] fix(react-chess-game): sound and keyboard control improvements - Fix sound override merging (missing spread operator) - Add check sound support with proper priority (checkmate > capture > check > move) - Sounds only trigger on new moves, not on initial FEN load - Scope keyboard controls to focused board by default - Add containerRef option for custom keyboard scoping - Ignore keyboard shortcuts when focus is on input/textarea/select/contenteditable - Use ResizeObserver for responsive promotion menu positioning - Fix promotion menu positioning for black orientation - Fix deepMergeChessboardOptions using mergeWith instead of merge - Add extensive test coverage for all changes Co-Authored-By: Claude Opus 4.6 --- packages/react-chess-game/README.md | 11 +- packages/react-chess-game/package.json | 3 +- .../src/components/ChessGame/parts/Board.tsx | 94 +++++++- .../ChessGame/parts/KeyboardControls.tsx | 24 +- .../src/components/ChessGame/parts/Root.tsx | 16 +- .../src/components/ChessGame/parts/Sounds.tsx | 2 +- .../parts/__tests__/KeyboardControls.test.tsx | 46 +++- .../ChessGame/parts/__tests__/Sounds.test.tsx | 56 +++++ .../src/hooks/useBoardSounds.test.ts | 150 ++++++++++-- .../src/hooks/useBoardSounds.ts | 68 +++++- .../useChessGameBoardContainerContext.ts | 20 ++ .../src/hooks/useKeyboardControls.test.tsx | 225 +++++++++++++++++- .../src/hooks/useKeyboardControls.ts | 54 ++++- .../src/utils/__tests__/board.test.ts | 43 ++++ packages/react-chess-game/src/utils/board.ts | 37 ++- 15 files changed, 766 insertions(+), 83 deletions(-) create mode 100644 packages/react-chess-game/src/hooks/useChessGameBoardContainerContext.ts diff --git a/packages/react-chess-game/README.md b/packages/react-chess-game/README.md index 61f8bbe..8e2dd92 100644 --- a/packages/react-chess-game/README.md +++ b/packages/react-chess-game/README.md @@ -206,7 +206,7 @@ Supports **ref forwarding** and all standard **HTML div attributes** (className, ### ChessGame.Sounds -Provides sound effects for the chess game. Uses built-in sounds by default, but custom sounds can be provided as base64-encoded strings. +Provides sound effects for the chess game. Uses built-in sounds by default, but custom sounds can be provided as base64-encoded strings. Sounds are emitted for new moves only, so loading a static FEN does not trigger audio. **Note:** This is a logic-only component that returns `null`. It sets up audio functionality via hooks. @@ -234,13 +234,14 @@ Provides sound effects for the chess game. Uses built-in sounds by default, but Enables keyboard navigation through the game history. -**Note:** This is a logic-only component that returns `null`. It sets up keyboard event listeners via hooks. +**Note:** This is a logic-only component that returns `null`. It sets up keyboard event listeners via hooks. When used alongside `ChessGame.Board` in the same `ChessGame.Root`, shortcuts automatically scope to the focused board. #### Props -| Name | Type | Default | Description | -| ---------- | ------------------ | ------------------------- | --------------------------------------------- | -| `controls` | `KeyboardControls` | `defaultKeyboardControls` | Object mapping key names to handler functions | +| Name | Type | Default | Description | +| -------------- | ------------------------ | ------------------------- | ------------------------------------------------------------ | +| `controls` | `KeyboardControls` | `defaultKeyboardControls` | Object mapping key names to handler functions | +| `containerRef` | `RefObject` | Board in same root | Override the default focus scope used for keyboard shortcuts | **Default Controls:** diff --git a/packages/react-chess-game/package.json b/packages/react-chess-game/package.json index 05aac3b..bda8117 100644 --- a/packages/react-chess-game/package.json +++ b/packages/react-chess-game/package.json @@ -18,8 +18,7 @@ "dist" ], "scripts": { - "build": "tsup src/index.ts", - "test": "echo \"Error: no test specified\" && exit 1" + "build": "tsup src/index.ts" }, "keywords": [ "chess", diff --git a/packages/react-chess-game/src/components/ChessGame/parts/Board.tsx b/packages/react-chess-game/src/components/ChessGame/parts/Board.tsx index b7ea5f7..06df161 100644 --- a/packages/react-chess-game/src/components/ChessGame/parts/Board.tsx +++ b/packages/react-chess-game/src/components/ChessGame/parts/Board.tsx @@ -11,6 +11,7 @@ import { deepMergeChessboardOptions, } from "../../../utils/board"; import { isLegalMove, requiresPromotion } from "../../../utils/chess"; +import { useChessGameBoardContainerContext } from "../../../hooks/useChessGameBoardContainerContext"; import { useChessGameContext } from "../../../hooks/useChessGameContext"; import { useChessGameTheme } from "../../../theme/context"; @@ -19,8 +20,20 @@ export interface ChessGameProps extends React.HTMLAttributes { } export const Board = React.forwardRef( - ({ options = {}, className, style: userStyle, ...rest }, ref) => { + ( + { + options = {}, + className, + style: userStyle, + onPointerDownCapture, + tabIndex = 0, + ...rest + }, + ref, + ) => { const gameContext = useChessGameContext(); + const { boardContainerRef, setBoardContainerElement } = + useChessGameBoardContainerContext(); const theme = useChessGameTheme(); if (!gameContext) { @@ -43,6 +56,37 @@ export const Board = React.forwardRef( const [promotionMove, setPromotionMove] = React.useState | null>(null); + // Track square width for responsive updates + const [squareWidth, setSquareWidth] = React.useState(80); + + const setBoardContainerRef = React.useCallback( + (node: HTMLDivElement | null) => { + boardContainerRef.current = node; + setBoardContainerElement(node); + + if (typeof ref === "function") { + ref(node); + return; + } + + if (ref) { + ref.current = node; + } + }, + [boardContainerRef, ref, setBoardContainerElement], + ); + + const handlePointerDownCapture = React.useCallback( + (event: React.PointerEvent) => { + onPointerDownCapture?.(event); + + if (!event.defaultPrevented) { + boardContainerRef.current?.focus({ preventScroll: true }); + } + }, + [boardContainerRef, onPointerDownCapture], + ); + const onSquareClick = (square: Square) => { if (isGameOver) { return; @@ -102,12 +146,28 @@ export const Board = React.forwardRef( setPromotionMove(null); }; - // Calculate square width for precise positioning - const squareWidth = React.useMemo(() => { - if (typeof document === "undefined") return 80; - const squareElement = document.querySelector(`[data-square]`); - return squareElement?.getBoundingClientRect()?.width ?? 80; - }, [promotionMove]); + // Use ResizeObserver for responsive square width updates + React.useEffect(() => { + if (typeof window === "undefined" || !boardContainerRef.current) return; + + const updateSquareWidth = () => { + const squareElement = + boardContainerRef.current?.querySelector("[data-square]"); + if (squareElement) { + setSquareWidth(squareElement.getBoundingClientRect().width); + } + }; + + // Initial measurement + updateSquareWidth(); + + // Only use ResizeObserver if available (not in all test environments) + if (typeof ResizeObserver !== "undefined" && boardContainerRef.current) { + const observer = new ResizeObserver(updateSquareWidth); + observer.observe(boardContainerRef.current); + return () => observer.disconnect(); + } + }, []); // Calculate promotion square position const promotionSquareLeft = React.useMemo(() => { @@ -173,8 +233,22 @@ export const Board = React.forwardRef( position: "relative" as const, }; + // Calculate promotion menu vertical position based on orientation + const isBlackOrientation = orientation === "b"; + const promotionRank = promotionMove?.to?.[1]; + const isTopRank = isBlackOrientation + ? promotionRank === "1" + : promotionRank === "8"; + return ( -
+
{promotionMove && ( <> @@ -199,8 +273,8 @@ export const Board = React.forwardRef(
; }; -export const KeyboardControls: FC = ({ controls }) => { - const keyboardControls = { ...defaultKeyboardControls, ...controls }; - useKeyboardControls(keyboardControls); +export const KeyboardControls: FC = ({ + controls, + containerRef, +}) => { + const { boardContainerElement, boardContainerRef } = + useChessGameBoardContainerContext(); + + useKeyboardControls({ + controls, + containerRef: + containerRef ?? (boardContainerElement ? boardContainerRef : undefined), + }); return null; }; diff --git a/packages/react-chess-game/src/components/ChessGame/parts/Root.tsx b/packages/react-chess-game/src/components/ChessGame/parts/Root.tsx index 4670f07..b3927b3 100644 --- a/packages/react-chess-game/src/components/ChessGame/parts/Root.tsx +++ b/packages/react-chess-game/src/components/ChessGame/parts/Root.tsx @@ -1,6 +1,7 @@ import React from "react"; import { Color } from "chess.js"; import { useChessGame } from "../../../hooks/useChessGame"; +import { ChessGameBoardContainerContext } from "../../../hooks/useChessGameBoardContainerContext"; import { ChessGameContext } from "../../../hooks/useChessGameContext"; import { ThemeProvider } from "../../../theme/context"; import { mergeTheme } from "../../../theme/utils"; @@ -32,13 +33,26 @@ export const Root: React.FC> = ({ timeControl, autoSwitchOnMove, }); + const boardContainerRef = React.useRef(null); + const [boardContainerElement, setBoardContainerElement] = + React.useState(null); // Merge partial theme with defaults const mergedTheme = React.useMemo(() => mergeTheme(theme), [theme]); + const boardContainerContext = React.useMemo( + () => ({ + boardContainerRef, + boardContainerElement, + setBoardContainerElement, + }), + [boardContainerElement], + ); return ( - {children} + + {children} + ); }; diff --git a/packages/react-chess-game/src/components/ChessGame/parts/Sounds.tsx b/packages/react-chess-game/src/components/ChessGame/parts/Sounds.tsx index f97208f..4bf151d 100644 --- a/packages/react-chess-game/src/components/ChessGame/parts/Sounds.tsx +++ b/packages/react-chess-game/src/components/ChessGame/parts/Sounds.tsx @@ -19,7 +19,7 @@ export const Sounds: React.FC = ({ sounds }) => { return {} as Record; } - return Object.entries({ ...defaultSounds, sounds }).reduce( + return Object.entries({ ...defaultSounds, ...sounds }).reduce( (acc, [name, base64]) => { acc[name as Sound] = new Audio(`data:audio/wav;base64,${base64}`); return acc; diff --git a/packages/react-chess-game/src/components/ChessGame/parts/__tests__/KeyboardControls.test.tsx b/packages/react-chess-game/src/components/ChessGame/parts/__tests__/KeyboardControls.test.tsx index 1634061..e3dfacc 100644 --- a/packages/react-chess-game/src/components/ChessGame/parts/__tests__/KeyboardControls.test.tsx +++ b/packages/react-chess-game/src/components/ChessGame/parts/__tests__/KeyboardControls.test.tsx @@ -1,5 +1,5 @@ import React from "react"; -import { render } from "@testing-library/react"; +import { act, fireEvent, render } from "@testing-library/react"; import "@testing-library/jest-dom"; import { ChessGame } from "../.."; import { KeyboardControls } from "../KeyboardControls"; @@ -20,6 +20,50 @@ describe("ChessGame.KeyboardControls", () => { expect(container.querySelector("*")).toBeNull(); }); + it("should scope shortcuts to the focused board by default", () => { + const customHandler = jest.fn(); + + const { container } = render( + + + + , + ); + + act(() => { + window.dispatchEvent(new KeyboardEvent("keydown", { key: "z" })); + }); + + expect(customHandler).not.toHaveBeenCalled(); + + const boardContainer = container.firstElementChild as HTMLDivElement | null; + expect(boardContainer).toHaveAttribute("tabindex", "0"); + + fireEvent.pointerDown(boardContainer as HTMLDivElement); + + act(() => { + window.dispatchEvent(new KeyboardEvent("keydown", { key: "z" })); + }); + + expect(customHandler).toHaveBeenCalledTimes(1); + }); + + it("should remain global when no board is registered", () => { + const customHandler = jest.fn(); + + render( + + + , + ); + + act(() => { + window.dispatchEvent(new KeyboardEvent("keydown", { key: "z" })); + }); + + expect(customHandler).toHaveBeenCalledTimes(1); + }); + it("should throw error when used outside ChessGame.Root", () => { const consoleError = jest .spyOn(console, "error") diff --git a/packages/react-chess-game/src/components/ChessGame/parts/__tests__/Sounds.test.tsx b/packages/react-chess-game/src/components/ChessGame/parts/__tests__/Sounds.test.tsx index 56fe7c6..0542295 100644 --- a/packages/react-chess-game/src/components/ChessGame/parts/__tests__/Sounds.test.tsx +++ b/packages/react-chess-game/src/components/ChessGame/parts/__tests__/Sounds.test.tsx @@ -4,7 +4,22 @@ import "@testing-library/jest-dom"; import { ChessGame } from "../.."; import { Sounds } from "../Sounds"; +// Mock Audio constructor +const mockAudioInstances: HTMLAudioElement[] = []; +class MockAudio { + src: string; + play = jest.fn().mockResolvedValue(undefined); + constructor(src: string) { + this.src = src; + mockAudioInstances.push(this as unknown as HTMLAudioElement); + } +} + describe("ChessGame.Sounds", () => { + beforeEach(() => { + mockAudioInstances.length = 0; + }); + it("should have correct displayName", () => { expect(Sounds.displayName).toBe("ChessGame.Sounds"); }); @@ -19,4 +34,45 @@ describe("ChessGame.Sounds", () => { // Sounds should not render any DOM elements expect(container.querySelector("*")).toBeNull(); }); + + describe("sound override merging", () => { + let originalAudio: typeof window.Audio; + + beforeAll(() => { + originalAudio = window.Audio; + (window as unknown as Record).Audio = MockAudio; + }); + + afterAll(() => { + window.Audio = originalAudio; + }); + + it("should properly merge custom sounds with default sounds", () => { + const customMoveSound = "customMoveBase64"; + render( + + + , + ); + + // Find the audio instance for the move sound + const moveAudio = mockAudioInstances.find( + (audio) => audio.src === `data:audio/wav;base64,${customMoveSound}`, + ); + + // Should have created an audio element with the custom sound + expect(moveAudio).toBeDefined(); + }); + + it("should include default sounds not overridden", () => { + render( + + + , + ); + + // All default sounds should be created + expect(mockAudioInstances.length).toBe(4); // move, capture, gameOver, check + }); + }); }); diff --git a/packages/react-chess-game/src/hooks/useBoardSounds.test.ts b/packages/react-chess-game/src/hooks/useBoardSounds.test.ts index f0364e2..387c9ff 100644 --- a/packages/react-chess-game/src/hooks/useBoardSounds.test.ts +++ b/packages/react-chess-game/src/hooks/useBoardSounds.test.ts @@ -24,6 +24,7 @@ describe("useBoardSounds", () => { info: { lastMove?: Partial | null; isCheckmate?: boolean; + isCheck?: boolean; }; }; @@ -47,15 +48,34 @@ describe("useBoardSounds", () => { expect(mockSounds.move.play).not.toHaveBeenCalled(); expect(mockSounds.capture.play).not.toHaveBeenCalled(); expect(mockSounds.gameOver.play).not.toHaveBeenCalled(); + expect(mockSounds.check.play).not.toHaveBeenCalled(); }); - it("should play move sound when lastMove is present", () => { + it("should not play sound when mounted with an existing checkmated position", () => { + mockContextValue.info.lastMove = { + from: "f3", + to: "g5", + piece: "Q" as PieceSymbol, + } as unknown as Partial; + mockContextValue.info.isCheck = true; + mockContextValue.info.isCheckmate = true; + + renderHook(() => useBoardSounds(mockSounds)); + + expect(mockSounds.move.play).not.toHaveBeenCalled(); + expect(mockSounds.capture.play).not.toHaveBeenCalled(); + expect(mockSounds.gameOver.play).not.toHaveBeenCalled(); + expect(mockSounds.check.play).not.toHaveBeenCalled(); + }); + + it("should play move sound when lastMove changes", () => { + const { rerender } = renderHook(() => useBoardSounds(mockSounds)); + mockContextValue.info.lastMove = { from: "e2", to: "e4", piece: "P" as PieceSymbol, } as unknown as Partial; - const { rerender } = renderHook(() => useBoardSounds(mockSounds)); mockedUseChessGameContext.mockReturnValue( mockContextValue as unknown as ReturnType, @@ -67,7 +87,9 @@ describe("useBoardSounds", () => { expect(mockSounds.gameOver.play).not.toHaveBeenCalled(); }); - it("should play capture sound when lastMove includes a capture", () => { + it("should play capture sound when lastMove changes to a capture", () => { + const { rerender } = renderHook(() => useBoardSounds(mockSounds)); + mockContextValue.info.lastMove = { from: "e4", to: "d5", @@ -78,7 +100,6 @@ describe("useBoardSounds", () => { mockContextValue as unknown as ReturnType, ); - const { rerender } = renderHook(() => useBoardSounds(mockSounds)); rerender(); expect(mockSounds.capture.play).toHaveBeenCalledTimes(1); @@ -86,7 +107,9 @@ describe("useBoardSounds", () => { expect(mockSounds.gameOver.play).not.toHaveBeenCalled(); }); - it("should play gameOver sound when isCheckmate is true", () => { + it("should play gameOver sound when a new move ends in checkmate", () => { + const { rerender } = renderHook(() => useBoardSounds(mockSounds)); + mockContextValue.info.isCheckmate = true; mockContextValue.info.lastMove = { from: "f3", @@ -97,7 +120,6 @@ describe("useBoardSounds", () => { mockContextValue as unknown as ReturnType, ); - const { rerender } = renderHook(() => useBoardSounds(mockSounds)); rerender(); expect(mockSounds.gameOver.play).toHaveBeenCalledTimes(1); @@ -105,7 +127,9 @@ describe("useBoardSounds", () => { expect(mockSounds.capture.play).not.toHaveBeenCalled(); }); - it("should play gameOver sound even if last move was a capture", () => { + it("should play gameOver sound even if the checkmating move was a capture", () => { + const { rerender } = renderHook(() => useBoardSounds(mockSounds)); + mockContextValue.info.isCheckmate = true; mockContextValue.info.lastMove = { from: "f3", @@ -117,7 +141,6 @@ describe("useBoardSounds", () => { mockContextValue as unknown as ReturnType, ); - const { rerender } = renderHook(() => useBoardSounds(mockSounds)); rerender(); expect(mockSounds.gameOver.play).toHaveBeenCalledTimes(1); @@ -126,6 +149,8 @@ describe("useBoardSounds", () => { }); it("should not play sound if lastMove becomes null", () => { + const { rerender } = renderHook(() => useBoardSounds(mockSounds)); + mockContextValue.info.lastMove = { from: "e2", to: "e4", @@ -134,8 +159,9 @@ describe("useBoardSounds", () => { mockedUseChessGameContext.mockReturnValue( mockContextValue as unknown as ReturnType, ); - const { rerender } = renderHook(() => useBoardSounds(mockSounds)); + rerender(); + expect(mockSounds.move.play).toHaveBeenCalledTimes(1); mockContextValue.info.lastMove = null; @@ -149,31 +175,118 @@ describe("useBoardSounds", () => { expect(mockSounds.gameOver.play).not.toHaveBeenCalled(); }); - it("should use updated sounds when they change", () => { - // Setup initial sounds and render hook + it("should play check sound when a new move gives check", () => { + const { rerender } = renderHook(() => useBoardSounds(mockSounds)); + mockContextValue.info.lastMove = { - from: "e2", - to: "e4", + from: "f3", + to: "g5", + piece: "Q" as PieceSymbol, + } as unknown as Partial; + mockContextValue.info.isCheck = true; + mockedUseChessGameContext.mockReturnValue( + mockContextValue as unknown as ReturnType, + ); + + rerender(); + + expect(mockSounds.check.play).toHaveBeenCalledTimes(1); + expect(mockSounds.move.play).not.toHaveBeenCalled(); + expect(mockSounds.capture.play).not.toHaveBeenCalled(); + expect(mockSounds.gameOver.play).not.toHaveBeenCalled(); + }); + + it("should play checkmate sound over check sound (checkmate takes precedence)", () => { + const { rerender } = renderHook(() => useBoardSounds(mockSounds)); + + mockContextValue.info.lastMove = { + from: "f3", + to: "g5", + piece: "Q" as PieceSymbol, + } as unknown as Partial; + mockContextValue.info.isCheck = true; + mockContextValue.info.isCheckmate = true; + mockedUseChessGameContext.mockReturnValue( + mockContextValue as unknown as ReturnType, + ); + + rerender(); + + expect(mockSounds.gameOver.play).toHaveBeenCalledTimes(1); + expect(mockSounds.check.play).not.toHaveBeenCalled(); + expect(mockSounds.move.play).not.toHaveBeenCalled(); + expect(mockSounds.capture.play).not.toHaveBeenCalled(); + }); + + it("should not react to check state changes without a new lastMove", () => { + const { rerender } = renderHook(() => useBoardSounds(mockSounds)); + + mockContextValue.info.isCheck = true; + mockedUseChessGameContext.mockReturnValue( + mockContextValue as unknown as ReturnType, + ); + rerender(); + + expect(mockSounds.check.play).not.toHaveBeenCalled(); + expect(mockSounds.gameOver.play).not.toHaveBeenCalled(); + + mockContextValue.info.isCheckmate = true; + mockedUseChessGameContext.mockReturnValue( + mockContextValue as unknown as ReturnType, + ); + + rerender(); + + expect(mockSounds.check.play).not.toHaveBeenCalled(); + expect(mockSounds.gameOver.play).not.toHaveBeenCalled(); + }); + + it("should play capture sound over check sound (capture takes precedence)", () => { + const { rerender } = renderHook(() => useBoardSounds(mockSounds)); + + mockContextValue.info.lastMove = { + from: "e4", + to: "d5", piece: "P" as PieceSymbol, + captured: "p" as PieceSymbol, } as unknown as Partial; + mockContextValue.info.isCheck = true; mockedUseChessGameContext.mockReturnValue( mockContextValue as unknown as ReturnType, ); + rerender(); + + expect(mockSounds.capture.play).toHaveBeenCalledTimes(1); + expect(mockSounds.check.play).not.toHaveBeenCalled(); + expect(mockSounds.move.play).not.toHaveBeenCalled(); + expect(mockSounds.gameOver.play).not.toHaveBeenCalled(); + }); + + it("should use updated sounds when they change", () => { const { rerender } = renderHook((props) => useBoardSounds(props), { initialProps: mockSounds, }); - // Verify initial sound played + mockContextValue.info.lastMove = { + from: "e2", + to: "e4", + piece: "P" as PieceSymbol, + } as unknown as Partial; + mockedUseChessGameContext.mockReturnValue( + mockContextValue as unknown as ReturnType, + ); + + rerender(mockSounds); + expect(mockSounds.move.play).toHaveBeenCalledTimes(1); - // Create new set of mock sounds const newMockSounds = createMockSounds(); - // Re-render with new sounds and trigger a move rerender(newMockSounds); - // Update lastMove to trigger sound effect with new sounds + expect(newMockSounds.move.play).not.toHaveBeenCalled(); + mockContextValue.info.lastMove = { from: "e7", to: "e5", @@ -185,10 +298,7 @@ describe("useBoardSounds", () => { rerender(newMockSounds); - // Original sounds should not be called again expect(mockSounds.move.play).toHaveBeenCalledTimes(1); - - // New sounds should be called expect(newMockSounds.move.play).toHaveBeenCalledTimes(1); expect(newMockSounds.capture.play).not.toHaveBeenCalled(); expect(newMockSounds.gameOver.play).not.toHaveBeenCalled(); diff --git a/packages/react-chess-game/src/hooks/useBoardSounds.ts b/packages/react-chess-game/src/hooks/useBoardSounds.ts index 44dfbfc..c1e26ed 100644 --- a/packages/react-chess-game/src/hooks/useBoardSounds.ts +++ b/packages/react-chess-game/src/hooks/useBoardSounds.ts @@ -1,4 +1,5 @@ -import { useEffect } from "react"; +import { type Move } from "chess.js"; +import { useEffect, useRef } from "react"; import { useChessGameContext } from "./useChessGameContext"; import { type Sound } from "../assets/sounds"; @@ -10,28 +11,75 @@ const playSound = async (audioElement: HTMLAudioElement) => { } }; +const getMoveSignature = (move: Partial | null | undefined) => { + if (!move?.from || !move?.to) { + return null; + } + + return [ + move.color ?? "", + move.piece ?? "", + move.from, + move.to, + move.captured ?? "", + move.promotion ?? "", + move.san ?? "", + ].join(":"); +}; + export const useBoardSounds = (sounds: Record) => { const { - info: { lastMove, isCheckmate }, + info: { lastMove, isCheckmate, isCheck }, } = useChessGameContext(); + // Use ref to store sounds to avoid triggering effect on every render + const soundsRef = useRef(sounds); + soundsRef.current = sounds; + const previousMoveSignatureRef = useRef(null); + const isFirstRenderRef = useRef(true); + useEffect(() => { - if (Object.keys(sounds).length === 0) { + const currentSounds = soundsRef.current; + const currentMoveSignature = getMoveSignature(lastMove); + + if (isFirstRenderRef.current) { + isFirstRenderRef.current = false; + previousMoveSignatureRef.current = currentMoveSignature; + return; + } + + if (!currentMoveSignature) { + previousMoveSignatureRef.current = null; + return; + } + + if (currentMoveSignature === previousMoveSignatureRef.current) { + return; + } + + previousMoveSignatureRef.current = currentMoveSignature; + + if (Object.keys(currentSounds).length === 0) { + return; + } + + if (isCheckmate && currentSounds.gameOver) { + playSound(currentSounds.gameOver); return; } - if (isCheckmate && sounds.gameOver) { - playSound(sounds.gameOver); + if (lastMove?.captured && currentSounds.capture) { + playSound(currentSounds.capture); return; } - if (lastMove?.captured && sounds.capture) { - playSound(sounds.capture); + if (isCheck && currentSounds.check) { + playSound(currentSounds.check); return; } - if (lastMove && sounds.move) { - playSound(sounds.move); + if (lastMove && currentSounds.move) { + playSound(currentSounds.move); } - }, [lastMove]); + }, [lastMove, isCheck, isCheckmate]); }; diff --git a/packages/react-chess-game/src/hooks/useChessGameBoardContainerContext.ts b/packages/react-chess-game/src/hooks/useChessGameBoardContainerContext.ts new file mode 100644 index 0000000..7deb623 --- /dev/null +++ b/packages/react-chess-game/src/hooks/useChessGameBoardContainerContext.ts @@ -0,0 +1,20 @@ +import React from "react"; + +export type ChessGameBoardContainerContextType = { + boardContainerRef: React.RefObject; + boardContainerElement: HTMLDivElement | null; + setBoardContainerElement: (node: HTMLDivElement | null) => void; +}; + +export const ChessGameBoardContainerContext = + React.createContext(null); + +export const useChessGameBoardContainerContext = () => { + const context = React.useContext(ChessGameBoardContainerContext); + if (!context) { + throw new Error( + "useChessGameBoardContainerContext must be used within a ChessGame.Root component.", + ); + } + return context; +}; diff --git a/packages/react-chess-game/src/hooks/useKeyboardControls.test.tsx b/packages/react-chess-game/src/hooks/useKeyboardControls.test.tsx index 0bbd344..373dd33 100644 --- a/packages/react-chess-game/src/hooks/useKeyboardControls.test.tsx +++ b/packages/react-chess-game/src/hooks/useKeyboardControls.test.tsx @@ -1,4 +1,5 @@ import { renderHook, act } from "@testing-library/react"; +import { createRef } from "react"; import { useKeyboardControls } from "./useKeyboardControls"; import { useChessGameContext, @@ -97,7 +98,7 @@ describe("useKeyboardControls", () => { const customHandler = jest.fn(); const customControls = { z: customHandler }; - renderHook(() => useKeyboardControls(customControls)); + renderHook(() => useKeyboardControls({ controls: customControls })); const event = new KeyboardEvent("keydown", { key: "z" }); const preventDefaultSpy = jest.spyOn(event, "preventDefault"); @@ -115,7 +116,7 @@ describe("useKeyboardControls", () => { const customHandler = jest.fn(); const customControls = { z: customHandler }; - renderHook(() => useKeyboardControls(customControls)); + renderHook(() => useKeyboardControls({ controls: customControls })); const event = new KeyboardEvent("keydown", { key: "ArrowRight" }); const preventDefaultSpy = jest.spyOn(event, "preventDefault"); @@ -136,7 +137,7 @@ describe("useKeyboardControls", () => { const customArrowLeftHandler = jest.fn(); const customControls = { ArrowLeft: customArrowLeftHandler }; - renderHook(() => useKeyboardControls(customControls)); + renderHook(() => useKeyboardControls({ controls: customControls })); const event = new KeyboardEvent("keydown", { key: "ArrowLeft" }); const preventDefaultSpy = jest.spyOn(event, "preventDefault"); @@ -168,4 +169,222 @@ describe("useKeyboardControls", () => { }); expect(preventDefaultSpy).not.toHaveBeenCalled(); }); + + it("should not trigger when focus is on an input element", () => { + renderHook(() => useKeyboardControls()); + + // Create and focus an input element + const input = document.createElement("input"); + document.body.appendChild(input); + input.focus(); + + const event = new KeyboardEvent("keydown", { key: "ArrowLeft" }); + const preventDefaultSpy = jest.spyOn(event, "preventDefault"); + + act(() => { + window.dispatchEvent(event); + }); + + expect(mockGameContext.methods.goToPreviousMove).not.toHaveBeenCalled(); + expect(preventDefaultSpy).not.toHaveBeenCalled(); + + document.body.removeChild(input); + }); + + it("should not trigger when focus is on a textarea element", () => { + renderHook(() => useKeyboardControls()); + + // Create and focus a textarea element + const textarea = document.createElement("textarea"); + document.body.appendChild(textarea); + textarea.focus(); + + const event = new KeyboardEvent("keydown", { key: "ArrowLeft" }); + const preventDefaultSpy = jest.spyOn(event, "preventDefault"); + + act(() => { + window.dispatchEvent(event); + }); + + expect(mockGameContext.methods.goToPreviousMove).not.toHaveBeenCalled(); + expect(preventDefaultSpy).not.toHaveBeenCalled(); + + document.body.removeChild(textarea); + }); + + it("should not trigger when focus is on a select element", () => { + renderHook(() => useKeyboardControls()); + + const select = document.createElement("select"); + document.body.appendChild(select); + select.focus(); + + const event = new KeyboardEvent("keydown", { key: "ArrowDown" }); + const preventDefaultSpy = jest.spyOn(event, "preventDefault"); + + act(() => { + window.dispatchEvent(event); + }); + + expect(mockGameContext.methods.goToEnd).not.toHaveBeenCalled(); + expect(preventDefaultSpy).not.toHaveBeenCalled(); + + document.body.removeChild(select); + }); + + it("should not trigger when focus is on an element with isContentEditable=true", () => { + renderHook(() => useKeyboardControls()); + + // Create and focus a contenteditable element + const div = document.createElement("div"); + div.setAttribute("contenteditable", "true"); + document.body.appendChild(div); + div.focus(); + + // Mock isContentEditable since jsdom doesn't compute it automatically + Object.defineProperty(div, "isContentEditable", { + value: true, + writable: true, + }); + + const event = new KeyboardEvent("keydown", { key: "ArrowLeft" }); + const preventDefaultSpy = jest.spyOn(event, "preventDefault"); + + act(() => { + window.dispatchEvent(event); + }); + + expect(mockGameContext.methods.goToPreviousMove).not.toHaveBeenCalled(); + expect(preventDefaultSpy).not.toHaveBeenCalled(); + + document.body.removeChild(div); + }); + + it("should not trigger when focus is on an element with contenteditable='plaintext-only'", () => { + renderHook(() => useKeyboardControls()); + + // Create and focus an element with plaintext-only editing + const div = document.createElement("div"); + div.setAttribute("contenteditable", "plaintext-only"); + document.body.appendChild(div); + div.focus(); + + // Mock isContentEditable since jsdom doesn't compute it automatically + Object.defineProperty(div, "isContentEditable", { + value: true, + writable: true, + }); + + const event = new KeyboardEvent("keydown", { key: "ArrowLeft" }); + const preventDefaultSpy = jest.spyOn(event, "preventDefault"); + + act(() => { + window.dispatchEvent(event); + }); + + // isContentEditable should be true for plaintext-only as well + expect(mockGameContext.methods.goToPreviousMove).not.toHaveBeenCalled(); + expect(preventDefaultSpy).not.toHaveBeenCalled(); + + document.body.removeChild(div); + }); + + it("should use updated controls after rerender", () => { + const initialHandler = jest.fn(); + const updatedHandler = jest.fn(); + + const { rerender } = renderHook((options) => useKeyboardControls(options), { + initialProps: { controls: { ArrowLeft: initialHandler } }, + }); + + // Trigger with initial handler + const event1 = new KeyboardEvent("keydown", { key: "ArrowLeft" }); + act(() => { + window.dispatchEvent(event1); + }); + + expect(initialHandler).toHaveBeenCalledTimes(1); + expect(updatedHandler).not.toHaveBeenCalled(); + + // Rerender with updated handler + rerender({ controls: { ArrowLeft: updatedHandler } }); + + // Trigger with updated handler + const event2 = new KeyboardEvent("keydown", { key: "ArrowLeft" }); + act(() => { + window.dispatchEvent(event2); + }); + + // Initial handler should still be called only once + expect(initialHandler).toHaveBeenCalledTimes(1); + // Updated handler should be called + expect(updatedHandler).toHaveBeenCalledTimes(1); + }); + + describe("containerRef scoping", () => { + it("should only trigger when focus is within the containerRef element", () => { + const containerRef = createRef(); + const container = document.createElement("div"); + document.body.appendChild(container); + containerRef.current = container; + + renderHook(() => useKeyboardControls({ containerRef })); + + // Focus outside the container + const outsideElement = document.createElement("input"); + document.body.appendChild(outsideElement); + outsideElement.focus(); + + const event1 = new KeyboardEvent("keydown", { key: "ArrowLeft" }); + act(() => { + window.dispatchEvent(event1); + }); + + // Should not trigger because focus is outside container + expect(mockGameContext.methods.goToPreviousMove).not.toHaveBeenCalled(); + + // Focus inside the container + const insideElement = document.createElement("button"); + container.appendChild(insideElement); + insideElement.focus(); + + const event2 = new KeyboardEvent("keydown", { key: "ArrowLeft" }); + act(() => { + window.dispatchEvent(event2); + }); + + // Should trigger because focus is inside container + expect(mockGameContext.methods.goToPreviousMove).toHaveBeenCalledTimes(1); + + document.body.removeChild(container); + document.body.removeChild(outsideElement); + }); + + it("should work globally when no containerRef is provided", () => { + renderHook(() => useKeyboardControls()); + + // Even with no specific focus, should work globally + const event = new KeyboardEvent("keydown", { key: "ArrowLeft" }); + act(() => { + window.dispatchEvent(event); + }); + + expect(mockGameContext.methods.goToPreviousMove).toHaveBeenCalledTimes(1); + }); + + it("should not trigger when containerRef.current is null", () => { + const containerRef = createRef(); + // containerRef.current is null by default + + renderHook(() => useKeyboardControls({ containerRef })); + + const event = new KeyboardEvent("keydown", { key: "ArrowLeft" }); + act(() => { + window.dispatchEvent(event); + }); + + // Scoped controls should stay inactive until the container ref is attached + expect(mockGameContext.methods.goToPreviousMove).not.toHaveBeenCalled(); + }); + }); }); diff --git a/packages/react-chess-game/src/hooks/useKeyboardControls.ts b/packages/react-chess-game/src/hooks/useKeyboardControls.ts index 05bdb06..a8f524c 100644 --- a/packages/react-chess-game/src/hooks/useKeyboardControls.ts +++ b/packages/react-chess-game/src/hooks/useKeyboardControls.ts @@ -1,19 +1,65 @@ -import { useEffect } from "react"; +import { useEffect, useRef, type RefObject } from "react"; import { defaultKeyboardControls, KeyboardControls, } from "../components/ChessGame/parts/KeyboardControls"; import { useChessGameContext } from "./useChessGameContext"; -export const useKeyboardControls = (controls?: KeyboardControls) => { +export type UseKeyboardControlsOptions = { + controls?: KeyboardControls; + /** + * Optional container ref to scope keyboard handling to a specific board. + * Keyboard events only trigger when focus is within this container. + */ + containerRef?: RefObject; +}; + +export const useKeyboardControls = (options?: UseKeyboardControlsOptions) => { + const controls = options?.controls; + const containerRef = options?.containerRef; + const gameContext = useChessGameContext(); if (!gameContext) { throw new Error("ChessGameContext not found"); } const keyboardControls = { ...defaultKeyboardControls, ...controls }; + + // Use ref to store controls to avoid stale closure issues + const controlsRef = useRef(keyboardControls); + controlsRef.current = keyboardControls; + useEffect(() => { const handleKeyDown = (event: KeyboardEvent) => { - const handler = keyboardControls[event.key]; + const activeElement = document.activeElement as HTMLElement | null; + + // If containerRef is provided, only respond when focus is within that container + if (containerRef) { + const containerElement = containerRef.current; + const isWithinContainer = + !!containerElement && + !!activeElement && + containerElement.contains(activeElement); + if (!isWithinContainer) { + return; + } + } + + // Ignore events from editable elements using the proper isContentEditable property + // This handles contenteditable="true", "plaintext-only", and inherited editability + if (activeElement?.isContentEditable) { + return; + } + + // Also ignore input and textarea elements + if ( + activeElement instanceof HTMLInputElement || + activeElement instanceof HTMLTextAreaElement || + activeElement instanceof HTMLSelectElement + ) { + return; + } + + const handler = controlsRef.current[event.key]; if (handler) { event.preventDefault(); handler(gameContext); @@ -23,6 +69,6 @@ export const useKeyboardControls = (controls?: KeyboardControls) => { return () => { window.removeEventListener("keydown", handleKeyDown); }; - }, [gameContext]); + }, [gameContext, containerRef]); return null; }; diff --git a/packages/react-chess-game/src/utils/__tests__/board.test.ts b/packages/react-chess-game/src/utils/__tests__/board.test.ts index d78e6a7..b3278d5 100644 --- a/packages/react-chess-game/src/utils/__tests__/board.test.ts +++ b/packages/react-chess-game/src/utils/__tests__/board.test.ts @@ -1,4 +1,5 @@ import { Chess } from "chess.js"; +import type { ChessboardOptions } from "react-chessboard"; import { getCustomSquareStyles, deepMergeChessboardOptions } from "../board"; import { getGameInfo } from "../chess"; import { defaultGameTheme } from "../../theme/defaults"; @@ -512,5 +513,47 @@ describe("Board Utilities", () => { expect(result.onSquareClick).toBe(userCustomOptions.onSquareClick); expect(result.onSquareClick).not.toBe(baseOptions.onSquareClick); }); + + it("should overwrite arrays instead of merging them", () => { + // Test the mergeWith behavior directly with array-valued options + // ChessboardOptions may not have array properties, but this tests the merge logic + const baseOptions = { + squareStyles: { e4: { backgroundColor: "yellow" } }, + showNotation: true, + // Simulate an array property that might be added in future + customArray: ["a", "b", "c"], + } as Record; + + const customOptions = { + customArray: ["x", "y"], + } as Record; + + const result = deepMergeChessboardOptions( + baseOptions as unknown as ChessboardOptions, + customOptions as unknown as Partial, + ) as Record; + + // Arrays should be overwritten, NOT merged (i.e., ["x", "y"], not ["a", "b", "c", "x", "y"]) + expect(result.customArray).toEqual(["x", "y"]); + expect(result.customArray).not.toEqual(["a", "b", "c", "x", "y"]); + expect(result.customArray).not.toEqual(["x", "y", "c"]); + }); + + it("should merge objects while preserving non-overridden properties", () => { + const baseOptions = { + squareStyles: { e4: { backgroundColor: "yellow" } }, + showNotation: true, + }; + + const customOptions = { + squareStyles: { d5: { backgroundColor: "blue" } }, + }; + + const result = deepMergeChessboardOptions(baseOptions, customOptions); + + // Objects should be merged, not overwritten + expect(result.squareStyles?.e4).toEqual({ backgroundColor: "yellow" }); + expect(result.squareStyles?.d5).toEqual({ backgroundColor: "blue" }); + }); }); }); diff --git a/packages/react-chess-game/src/utils/board.ts b/packages/react-chess-game/src/utils/board.ts index 96afb36..8deb2f4 100644 --- a/packages/react-chess-game/src/utils/board.ts +++ b/packages/react-chess-game/src/utils/board.ts @@ -1,6 +1,6 @@ import { type Chess, type Square } from "chess.js"; import { type CSSProperties } from "react"; -import { merge } from "lodash"; +import { mergeWith } from "lodash"; import type { ChessboardOptions } from "react-chessboard"; import { getDestinationSquares, type GameInfo } from "./chess"; import type { ChessGameTheme } from "../theme/types"; @@ -87,28 +87,21 @@ export const deepMergeChessboardOptions = ( return { ...baseOptions }; // Return a new object even when no custom options } - const result = merge({}, baseOptions, customOptions, { - customizer: (_objValue: unknown, srcValue: unknown) => { - // Functions should always overwrite (not merge) - // This is important for event handlers like onSquareClick, onPieceDrop, etc. - if (typeof srcValue === "function") { - return srcValue; - } + return mergeWith({}, baseOptions, customOptions, (_objValue, srcValue) => { + // Functions should always overwrite (not merge) + // This is important for event handlers like onSquareClick, onPieceDrop, etc. + if (typeof srcValue === "function") { + return srcValue; + } - // For arrays, we typically want to overwrite rather than merge - // This avoids unexpected behavior with array concatenation - if (Array.isArray(srcValue)) { - return srcValue; - } + // For arrays, we typically want to overwrite rather than merge + // This avoids unexpected behavior with array concatenation + if (Array.isArray(srcValue)) { + return srcValue; + } - // Let lodash handle objects with default deep merge behavior - // This will properly merge nested objects like squareStyles, dropSquareStyle, etc. - return undefined; // Use default merge behavior - }, + // Let lodash handle objects with default deep merge behavior + // This will properly merge nested objects like squareStyles, dropSquareStyle, etc. + return undefined; // Use default merge behavior }); - - // Clean up any unwanted properties that lodash might add - delete (result as Record).customizer; - - return result; }; From 0729ec73ec3e8f16f6ce45ae8e53c54403028d77 Mon Sep 17 00:00:00 2001 From: Daniele Cammareri Date: Tue, 10 Mar 2026 23:51:53 +0100 Subject: [PATCH 2/3] fix(react-chess-game, react-chess-puzzle): enhance focus handling for board components --- .../components/ChessGame/ChessGame.stories.tsx | 14 ++++++++++++-- .../src/components/ChessGame/parts/Board.tsx | 15 +++++++++++++-- .../ChessGame/parts/__tests__/Board.test.tsx | 18 +++++++++++++++++- .../ChessPuzzle/ChessPuzzle.stories.tsx | 12 +++++++++++- 4 files changed, 53 insertions(+), 6 deletions(-) diff --git a/packages/react-chess-game/src/components/ChessGame/ChessGame.stories.tsx b/packages/react-chess-game/src/components/ChessGame/ChessGame.stories.tsx index 2625cd3..9467d33 100644 --- a/packages/react-chess-game/src/components/ChessGame/ChessGame.stories.tsx +++ b/packages/react-chess-game/src/components/ChessGame/ChessGame.stories.tsx @@ -30,6 +30,16 @@ const meta = { export default meta; +const AutoFocusedBoard = () => { + const boardRef = React.useRef(null); + + React.useEffect(() => { + boardRef.current?.focus({ preventScroll: true }); + }, []); + + return ; +}; + export const Default = () => ( ( - +

@@ -83,7 +93,7 @@ export const WithKeyboardControls = () => ( d: (ctx) => ctx.methods.goToNextMove(), }} /> - +

diff --git a/packages/react-chess-game/src/components/ChessGame/parts/Board.tsx b/packages/react-chess-game/src/components/ChessGame/parts/Board.tsx index 06df161..dcd136d 100644 --- a/packages/react-chess-game/src/components/ChessGame/parts/Board.tsx +++ b/packages/react-chess-game/src/components/ChessGame/parts/Board.tsx @@ -59,6 +59,10 @@ export const Board = React.forwardRef( // Track square width for responsive updates const [squareWidth, setSquareWidth] = React.useState(80); + const focusBoardContainer = React.useCallback(() => { + boardContainerRef.current?.focus({ preventScroll: true }); + }, [boardContainerRef]); + const setBoardContainerRef = React.useCallback( (node: HTMLDivElement | null) => { boardContainerRef.current = node; @@ -81,13 +85,15 @@ export const Board = React.forwardRef( onPointerDownCapture?.(event); if (!event.defaultPrevented) { - boardContainerRef.current?.focus({ preventScroll: true }); + focusBoardContainer(); } }, - [boardContainerRef, onPointerDownCapture], + [focusBoardContainer, onPointerDownCapture], ); const onSquareClick = (square: Square) => { + focusBoardContainer(); + if (isGameOver) { return; } @@ -131,6 +137,8 @@ export const Board = React.forwardRef( }; const onPromotionPieceSelect = (piece: string): void => { + focusBoardContainer(); + if (promotionMove?.from && promotionMove?.to) { makeMove({ from: promotionMove.from, @@ -142,6 +150,7 @@ export const Board = React.forwardRef( }; const onSquareRightClick = () => { + focusBoardContainer(); setActiveSquare(null); setPromotionMove(null); }; @@ -197,11 +206,13 @@ export const Board = React.forwardRef( }, dropSquareStyle: theme.state.dropSquare, onPieceDrag: ({ piece, square }) => { + focusBoardContainer(); if (piece.pieceType[0] === turn) { setActiveSquare(square as Square); } }, onPieceDrop: ({ sourceSquare, targetSquare }) => { + focusBoardContainer(); setActiveSquare(null); const moveData = { from: sourceSquare as Square, diff --git a/packages/react-chess-game/src/components/ChessGame/parts/__tests__/Board.test.tsx b/packages/react-chess-game/src/components/ChessGame/parts/__tests__/Board.test.tsx index fe37a5b..2c2f08b 100644 --- a/packages/react-chess-game/src/components/ChessGame/parts/__tests__/Board.test.tsx +++ b/packages/react-chess-game/src/components/ChessGame/parts/__tests__/Board.test.tsx @@ -1,5 +1,5 @@ import React from "react"; -import { render } from "@testing-library/react"; +import { fireEvent, render } from "@testing-library/react"; import "@testing-library/jest-dom"; import { ChessGame } from "../.."; import { Board } from "../Board"; @@ -107,6 +107,22 @@ describe("ChessGame.Board", () => { expect(handleClick).toHaveBeenCalledTimes(1); }); + it("should focus the board container on pointer interaction", () => { + const { container } = render( + + + , + ); + + const board = container.firstElementChild as HTMLDivElement; + + expect(document.activeElement).not.toBe(board); + + fireEvent.pointerDown(board); + + expect(document.activeElement).toBe(board); + }); + it("should throw error when used outside ChessGame.Root", () => { // Suppress console.error for this test const consoleError = jest diff --git a/packages/react-chess-puzzle/src/components/ChessPuzzle/ChessPuzzle.stories.tsx b/packages/react-chess-puzzle/src/components/ChessPuzzle/ChessPuzzle.stories.tsx index 4e620b5..3ae3685 100644 --- a/packages/react-chess-puzzle/src/components/ChessPuzzle/ChessPuzzle.stories.tsx +++ b/packages/react-chess-puzzle/src/components/ChessPuzzle/ChessPuzzle.stories.tsx @@ -40,6 +40,16 @@ const meta = { export default meta; +const AutoFocusedPuzzleBoard = () => { + const boardRef = React.useRef(null); + + React.useEffect(() => { + boardRef.current?.focus({ preventScroll: true }); + }, []); + + return ; +}; + export const Example = (args: RootProps) => { const [puzzleIndex, setPuzzleIndex] = React.useState(0); const puzzle = puzzles[puzzleIndex]; @@ -172,7 +182,7 @@ export const WithKeyboardControls = (args: RootProps) => { subtitle="Use keyboard shortcuts to navigate" /> - +
From 62b9a9393865f2b7e8a362adac99fddb0e3211a2 Mon Sep 17 00:00:00 2001 From: Daniele Cammareri Date: Tue, 10 Mar 2026 23:54:01 +0100 Subject: [PATCH 3/3] fix: add changeset for sound and keyboard control improvements --- .changeset/upset-cobras-smell.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/upset-cobras-smell.md diff --git a/.changeset/upset-cobras-smell.md b/.changeset/upset-cobras-smell.md new file mode 100644 index 0000000..2d8557b --- /dev/null +++ b/.changeset/upset-cobras-smell.md @@ -0,0 +1,6 @@ +--- +"@react-chess-tools/react-chess-puzzle": patch +"@react-chess-tools/react-chess-game": patch +--- + +fix: sound and keyboard control improvements