Refactor App.tsx from god component to context-based architecture

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-19 14:19:58 +01:00
parent 6584d8d064
commit 86768842ff
35 changed files with 1065 additions and 795 deletions
--- a/apps/web/src/components/tests/action-bar.test.tsx
+++ b/apps/web/src/components/tests/action-bar.test.tsx
@@ -3,21 +3,59 @@ import "@testing-library/jest-dom/vitest";

 import { cleanup, render, screen } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
-import { afterEach, describe, expect, it, vi } from "vitest";
-import { ActionBar } from "../action-bar";
+import { afterEach, beforeAll, describe, expect, it, vi } from "vitest";
+import { AllProviders } from "../../__tests__/test-providers.js";
+import { ActionBar } from "../action-bar.js";
+
+// Mock persistence — no localStorage interaction
+vi.mock("../../persistence/encounter-storage.js", () => ({
+	loadEncounter: () => null,
+	saveEncounter: () => {},
+}));
+
+vi.mock("../../persistence/player-character-storage.js", () => ({
+	loadPlayerCharacters: () => [],
+	savePlayerCharacters: () => {},
+}));
+
+// Mock bestiary — no IndexedDB or JSON index
+vi.mock("../../adapters/bestiary-cache.js", () => ({
+	loadAllCachedCreatures: () => Promise.resolve(new Map()),
+	isSourceCached: () => Promise.resolve(false),
+	cacheSource: () => Promise.resolve(),
+	getCachedSources: () => Promise.resolve([]),
+	clearSource: () => Promise.resolve(),
+	clearAll: () => Promise.resolve(),
+}));
+
+vi.mock("../../adapters/bestiary-index-adapter.js", () => ({
+	loadBestiaryIndex: () => ({ sources: {}, creatures: [] }),
+	getAllSourceCodes: () => [],
+	getDefaultFetchUrl: () => "",
+	getSourceDisplayName: (code: string) => code,
+}));
+
+// DOM API stubs — jsdom doesn't implement these
+beforeAll(() => {
+	Object.defineProperty(globalThis, "matchMedia", {
+		writable: true,
+		value: vi.fn().mockImplementation((query: string) => ({
+			matches: false,
+			media: query,
+			onchange: null,
+			addListener: vi.fn(),
+			removeListener: vi.fn(),
+			addEventListener: vi.fn(),
+			removeEventListener: vi.fn(),
+			dispatchEvent: vi.fn(),
+		})),
+	});
+});

 afterEach(cleanup);

-const defaultProps = {
-	onAddCombatant: vi.fn(),
-	onAddFromBestiary: vi.fn(),
-	bestiarySearch: () => [],
-	bestiaryLoaded: false,
-};
-
-function renderBar(overrides: Partial<Parameters<typeof ActionBar>[0]> = {}) {
-	const props = { ...defaultProps, ...overrides };
-	return render(<ActionBar {...props} />);
+function renderBar(props: Partial<Parameters<typeof ActionBar>[0]> = {}) {
+	return render(<ActionBar {...props} />, { wrapper: AllProviders });
 }

 describe("ActionBar", () => {
@@ -26,26 +64,26 @@ describe("ActionBar", () => {
 		expect(screen.getByPlaceholderText("+ Add combatants")).toBeInTheDocument();
 	});

-	it("submitting with a name calls onAddCombatant", async () => {
+	it("submitting with a name adds a combatant", async () => {
 		const user = userEvent.setup();
-		const onAddCombatant = vi.fn();
-		renderBar({ onAddCombatant });
+		renderBar();
 		const input = screen.getByPlaceholderText("+ Add combatants");
 		await user.type(input, "Goblin");
 		// The Add button appears when name >= 2 chars and no suggestions
 		const addButton = screen.getByRole("button", { name: "Add" });
 		await user.click(addButton);
-		expect(onAddCombatant).toHaveBeenCalledWith("Goblin", undefined);
+		// Input is cleared after adding (context handles the state)
+		expect(input).toHaveValue("");
 	});

 	it("submitting with empty name does nothing", async () => {
 		const user = userEvent.setup();
-		const onAddCombatant = vi.fn();
-		renderBar({ onAddCombatant });
+		renderBar();
 		// Submit the form directly (Enter on empty input)
 		const input = screen.getByPlaceholderText("+ Add combatants");
 		await user.type(input, "{Enter}");
-		expect(onAddCombatant).not.toHaveBeenCalled();
+		// Input stays empty, no error
+		expect(input).toHaveValue("");
 	});

 	it("shows custom fields (Init, AC, MaxHP) when name >= 2 chars and no bestiary suggestions", async () => {
@@ -66,23 +104,18 @@ describe("ActionBar", () => {
 		expect(screen.getByRole("button", { name: "Add" })).toBeInTheDocument();
 	});

-	it("shows roll all initiative button when showRollAllInitiative is true", () => {
-		const onRollAllInitiative = vi.fn();
-		renderBar({ showRollAllInitiative: true, onRollAllInitiative });
+	it("does not show roll all initiative button when no creature combatants", () => {
+		renderBar();
 		expect(
-			screen.getByRole("button", { name: "Roll all initiative" }),
-		).toBeInTheDocument();
+			screen.queryByRole("button", { name: "Roll all initiative" }),
+		).not.toBeInTheDocument();
 	});

-	it("roll all initiative button is disabled when rollAllInitiativeDisabled is true", () => {
-		const onRollAllInitiative = vi.fn();
-		renderBar({
-			showRollAllInitiative: true,
-			onRollAllInitiative,
-			rollAllInitiativeDisabled: true,
-		});
+	it("shows overflow menu items", () => {
+		renderBar({ onManagePlayers: vi.fn() });
+		// The overflow menu should be present (it contains Player Characters etc.)
 		expect(
-			screen.getByRole("button", { name: "Roll all initiative" }),
-		).toBeDisabled();
+			screen.getByRole("button", { name: "More actions" }),
+		).toBeInTheDocument();
 	});
 });
--- a/apps/web/src/components/tests/combatant-row.test.tsx
+++ b/apps/web/src/components/tests/combatant-row.test.tsx
@@ -1,33 +1,65 @@
 // @vitest-environment jsdom
 import "@testing-library/jest-dom/vitest";

-import { combatantId } from "@initiative/domain";
+import { type CreatureId, combatantId } from "@initiative/domain";
 import { cleanup, render, screen } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
-import { afterEach, describe, expect, it, vi } from "vitest";
-import { CombatantRow } from "../combatant-row";
-import { PLAYER_COLOR_HEX } from "../player-icon-map";
+import { afterEach, beforeAll, describe, expect, it, vi } from "vitest";
+import { AllProviders } from "../../__tests__/test-providers.js";
+import { CombatantRow } from "../combatant-row.js";
+import { PLAYER_COLOR_HEX } from "../player-icon-map.js";
+
+// Mock persistence — no localStorage interaction
+vi.mock("../../persistence/encounter-storage.js", () => ({
+	loadEncounter: () => null,
+	saveEncounter: () => {},
+}));
+
+vi.mock("../../persistence/player-character-storage.js", () => ({
+	loadPlayerCharacters: () => [],
+	savePlayerCharacters: () => {},
+}));
+
+// Mock bestiary — no IndexedDB or JSON index
+vi.mock("../../adapters/bestiary-cache.js", () => ({
+	loadAllCachedCreatures: () => Promise.resolve(new Map()),
+	isSourceCached: () => Promise.resolve(false),
+	cacheSource: () => Promise.resolve(),
+	getCachedSources: () => Promise.resolve([]),
+	clearSource: () => Promise.resolve(),
+	clearAll: () => Promise.resolve(),
+}));
+
+vi.mock("../../adapters/bestiary-index-adapter.js", () => ({
+	loadBestiaryIndex: () => ({ sources: {}, creatures: [] }),
+	getAllSourceCodes: () => [],
+	getDefaultFetchUrl: () => "",
+	getSourceDisplayName: (code: string) => code,
+}));
+
+// DOM API stubs
+beforeAll(() => {
+	Object.defineProperty(globalThis, "matchMedia", {
+		writable: true,
+		value: vi.fn().mockImplementation((query: string) => ({
+			matches: false,
+			media: query,
+			onchange: null,
+			addListener: vi.fn(),
+			removeListener: vi.fn(),
+			addEventListener: vi.fn(),
+			removeEventListener: vi.fn(),
+			dispatchEvent: vi.fn(),
+		})),
+	});
+});

 afterEach(cleanup);

-const defaultProps = {
-	onRename: vi.fn(),
-	onSetInitiative: vi.fn(),
-	onRemove: vi.fn(),
-	onSetHp: vi.fn(),
-	onAdjustHp: vi.fn(),
-	onSetAc: vi.fn(),
-	onToggleCondition: vi.fn(),
-	onToggleConcentration: vi.fn(),
-};
-
 function renderRow(
 	overrides: Partial<{
 		combatant: Parameters<typeof CombatantRow>[0]["combatant"];
 		isActive: boolean;
-		onRollInitiative: (id: ReturnType<typeof combatantId>) => void;
-		onRemove: (id: ReturnType<typeof combatantId>) => void;
-		onShowStatBlock: () => void;
 	}> = {},
 ) {
 	const combatant = overrides.combatant ?? {
@@ -38,15 +70,13 @@ function renderRow(
 		currentHp: 10,
 		ac: 13,
 	};
-	const props = {
-		...defaultProps,
-		combatant,
-		isActive: overrides.isActive ?? false,
-		onRollInitiative: overrides.onRollInitiative,
-		onShowStatBlock: overrides.onShowStatBlock,
-		onRemove: overrides.onRemove ?? defaultProps.onRemove,
-	};
-	return render(<CombatantRow {...props} />);
+	return render(
+		<CombatantRow
+			combatant={combatant}
+			isActive={overrides.isActive ?? false}
+		/>,
+		{ wrapper: AllProviders },
+	);
 }

 describe("CombatantRow", () => {
@@ -132,10 +162,9 @@ describe("CombatantRow", () => {
 		expect(svgIcon).toHaveStyle({ color: PLAYER_COLOR_HEX.red });
 	});

-	it("remove button calls onRemove after confirmation", async () => {
+	it("remove button removes after confirmation", async () => {
 		const user = userEvent.setup();
-		const onRemove = vi.fn();
-		renderRow({ onRemove });
+		renderRow();
 		const removeBtn = screen.getByRole("button", {
 			name: "Remove combatant",
 		});
@@ -146,16 +175,19 @@ describe("CombatantRow", () => {
 			name: "Confirm remove combatant",
 		});
 		await user.click(confirmBtn);
-		expect(onRemove).toHaveBeenCalledWith(combatantId("1"));
+		// After confirming, the button returns to its initial state
+		expect(
+			screen.queryByRole("button", { name: "Confirm remove combatant" }),
+		).not.toBeInTheDocument();
 	});

-	it("shows d20 roll button when initiative is undefined and onRollInitiative is provided", () => {
+	it("shows d20 roll button when initiative is undefined and combatant has creatureId", () => {
 		renderRow({
 			combatant: {
 				id: combatantId("1"),
 				name: "Goblin",
+				creatureId: "srd:goblin" as CreatureId,
 			},
-			onRollInitiative: vi.fn(),
 		});
 		expect(
 			screen.getByRole("button", { name: "Roll initiative" }),
--- a/apps/web/src/components/tests/source-manager.test.tsx
+++ b/apps/web/src/components/tests/source-manager.test.tsx
@@ -11,28 +11,51 @@ vi.mock("../../adapters/bestiary-cache.js", () => ({
 	clearAll: vi.fn(),
 }));

+// Mock the context module
+vi.mock("../../contexts/bestiary-context.js", () => ({
+	useBestiaryContext: vi.fn(),
+}));
+
 import * as bestiaryCache from "../../adapters/bestiary-cache.js";
-import { SourceManager } from "../source-manager";
+import { useBestiaryContext } from "../../contexts/bestiary-context.js";
+import { SourceManager } from "../source-manager.js";

 const mockGetCachedSources = vi.mocked(bestiaryCache.getCachedSources);
 const mockClearSource = vi.mocked(bestiaryCache.clearSource);
 const mockClearAll = vi.mocked(bestiaryCache.clearAll);
+const mockUseBestiaryContext = vi.mocked(useBestiaryContext);

 afterEach(() => {
 	cleanup();
 	vi.clearAllMocks();
 });

+function setupMockContext() {
+	const refreshCache = vi.fn().mockResolvedValue(undefined);
+	mockUseBestiaryContext.mockReturnValue({
+		refreshCache,
+		search: vi.fn().mockReturnValue([]),
+		getCreature: vi.fn(),
+		isLoaded: true,
+		isSourceCached: vi.fn().mockResolvedValue(false),
+		fetchAndCacheSource: vi.fn(),
+		uploadAndCacheSource: vi.fn(),
+	} as ReturnType<typeof useBestiaryContext>);
+	return { refreshCache };
+}
+
 describe("SourceManager", () => {
 	it("shows 'No cached sources' empty state when no sources", async () => {
+		setupMockContext();
 		mockGetCachedSources.mockResolvedValue([]);
-		render(<SourceManager onCacheCleared={vi.fn()} />);
+		render(<SourceManager />);
 		await waitFor(() => {
 			expect(screen.getByText("No cached sources")).toBeInTheDocument();
 		});
 	});

 	it("lists cached sources with display name and creature count", async () => {
+		setupMockContext();
 		mockGetCachedSources.mockResolvedValue([
 			{
 				sourceCode: "mm",
@@ -47,7 +70,7 @@ describe("SourceManager", () => {
 				cachedAt: Date.now(),
 			},
 		]);
-		render(<SourceManager onCacheCleared={vi.fn()} />);
+		render(<SourceManager />);
 		await waitFor(() => {
 			expect(screen.getByText("Monster Manual")).toBeInTheDocument();
 		});
@@ -56,9 +79,9 @@ describe("SourceManager", () => {
 		expect(screen.getByText("100 creatures")).toBeInTheDocument();
 	});

-	it("Clear All button calls cache clear and onCacheCleared", async () => {
+	it("Clear All button calls cache clear and refreshCache", async () => {
 		const user = userEvent.setup();
-		const onCacheCleared = vi.fn();
+		const { refreshCache } = setupMockContext();
 		mockGetCachedSources
 			.mockResolvedValueOnce([
 				{
@@ -70,7 +93,7 @@ describe("SourceManager", () => {
 			])
 			.mockResolvedValue([]);
 		mockClearAll.mockResolvedValue(undefined);
-		render(<SourceManager onCacheCleared={onCacheCleared} />);
+		render(<SourceManager />);

 		await waitFor(() => {
 			expect(screen.getByText("Monster Manual")).toBeInTheDocument();
@@ -80,12 +103,12 @@ describe("SourceManager", () => {
 		await waitFor(() => {
 			expect(mockClearAll).toHaveBeenCalled();
 		});
-		expect(onCacheCleared).toHaveBeenCalled();
+		expect(refreshCache).toHaveBeenCalled();
 	});

 	it("individual source delete button calls clear for that source", async () => {
 		const user = userEvent.setup();
-		const onCacheCleared = vi.fn();
+		const { refreshCache } = setupMockContext();
 		mockGetCachedSources
 			.mockResolvedValueOnce([
 				{
@@ -111,7 +134,7 @@ describe("SourceManager", () => {
 			]);
 		mockClearSource.mockResolvedValue(undefined);

-		render(<SourceManager onCacheCleared={onCacheCleared} />);
+		render(<SourceManager />);
 		await waitFor(() => {
 			expect(screen.getByText("Monster Manual")).toBeInTheDocument();
 		});
@@ -122,6 +145,6 @@ describe("SourceManager", () => {
 		await waitFor(() => {
 			expect(mockClearSource).toHaveBeenCalledWith("mm");
 		});
-		expect(onCacheCleared).toHaveBeenCalled();
+		expect(refreshCache).toHaveBeenCalled();
 	});
 });
--- a/apps/web/src/components/tests/turn-navigation.test.tsx
+++ b/apps/web/src/components/tests/turn-navigation.test.tsx
@@ -5,11 +5,23 @@ import type { Encounter } from "@initiative/domain";
 import { combatantId } from "@initiative/domain";
 import { cleanup, render, screen } from "@testing-library/react";
 import { afterEach, describe, expect, it, vi } from "vitest";
-import { TurnNavigation } from "../turn-navigation";

-afterEach(cleanup);
+// Mock the context module
+vi.mock("../../contexts/encounter-context.js", () => ({
+	useEncounterContext: vi.fn(),
+}));

-function renderNav(overrides: Partial<Encounter> = {}) {
+import { useEncounterContext } from "../../contexts/encounter-context.js";
+import { TurnNavigation } from "../turn-navigation.js";
+
+const mockUseEncounterContext = vi.mocked(useEncounterContext);
+
+afterEach(() => {
+	cleanup();
+	vi.clearAllMocks();
+});
+
+function mockContext(overrides: Partial<Encounter> = {}) {
 	const encounter: Encounter = {
 		combatants: [
 			{ id: combatantId("1"), name: "Goblin" },
@@ -20,14 +32,38 @@ function renderNav(overrides: Partial<Encounter> = {}) {
 		...overrides,
 	};

-	return render(
-		<TurnNavigation
-			encounter={encounter}
-			onAdvanceTurn={vi.fn()}
-			onRetreatTurn={vi.fn()}
-			onClearEncounter={vi.fn()}
-		/>,
+	const value = {
+		encounter,
+		advanceTurn: vi.fn(),
+		retreatTurn: vi.fn(),
+		clearEncounter: vi.fn(),
+		isEmpty: encounter.combatants.length === 0,
+		hasCreatureCombatants: false,
+		canRollAllInitiative: false,
+		addCombatant: vi.fn(),
+		removeCombatant: vi.fn(),
+		editCombatant: vi.fn(),
+		setInitiative: vi.fn(),
+		setHp: vi.fn(),
+		adjustHp: vi.fn(),
+		setAc: vi.fn(),
+		toggleCondition: vi.fn(),
+		toggleConcentration: vi.fn(),
+		addFromBestiary: vi.fn(),
+		addFromPlayerCharacter: vi.fn(),
+		makeStore: vi.fn(),
+		events: [],
+	};
+
+	mockUseEncounterContext.mockReturnValue(
+		value as ReturnType<typeof useEncounterContext>,
 	);
+	return value;
+}
+
+function renderNav(overrides: Partial<Encounter> = {}) {
+	mockContext(overrides);
+	return render(<TurnNavigation />);
 }

 describe("TurnNavigation", () => {
@@ -49,7 +85,7 @@ describe("TurnNavigation", () => {

 		it("does not render an em dash between round and name", () => {
 			const { container } = renderNav();
-			expect(container.textContent).not.toContain("—");
+			expect(container.textContent).not.toContain("\u2014");
 		});

 		it("round badge and combatant name are siblings in the center area", () => {
@@ -60,69 +96,27 @@ describe("TurnNavigation", () => {
 		});

 		it("updates the round badge when round changes", () => {
-			const { rerender } = render(
-				<TurnNavigation
-					encounter={{
-						combatants: [{ id: combatantId("1"), name: "Goblin" }],
-						activeIndex: 0,
-						roundNumber: 2,
-					}}
-					onAdvanceTurn={vi.fn()}
-					onRetreatTurn={vi.fn()}
-					onClearEncounter={vi.fn()}
-				/>,
-			);
+			mockContext({ roundNumber: 2 });
+			const { rerender } = render(<TurnNavigation />);
 			expect(screen.getByText("R2")).toBeInTheDocument();

-			rerender(
-				<TurnNavigation
-					encounter={{
-						combatants: [{ id: combatantId("1"), name: "Goblin" }],
-						activeIndex: 0,
-						roundNumber: 3,
-					}}
-					onAdvanceTurn={vi.fn()}
-					onRetreatTurn={vi.fn()}
-					onClearEncounter={vi.fn()}
-				/>,
-			);
+			mockContext({ roundNumber: 3 });
+			rerender(<TurnNavigation />);
 			expect(screen.getByText("R3")).toBeInTheDocument();
 			expect(screen.queryByText("R2")).not.toBeInTheDocument();
 		});

 		it("renders the next combatant name when turn advances", () => {
-			const { rerender } = render(
-				<TurnNavigation
-					encounter={{
-						combatants: [
-							{ id: combatantId("1"), name: "Goblin" },
-							{ id: combatantId("2"), name: "Conjurer" },
-						],
-						activeIndex: 0,
-						roundNumber: 1,
-					}}
-					onAdvanceTurn={vi.fn()}
-					onRetreatTurn={vi.fn()}
-					onClearEncounter={vi.fn()}
-				/>,
-			);
+			const combatants = [
+				{ id: combatantId("1"), name: "Goblin" },
+				{ id: combatantId("2"), name: "Conjurer" },
+			];
+			mockContext({ combatants, activeIndex: 0 });
+			const { rerender } = render(<TurnNavigation />);
 			expect(screen.getByText("Goblin")).toBeInTheDocument();

-			rerender(
-				<TurnNavigation
-					encounter={{
-						combatants: [
-							{ id: combatantId("1"), name: "Goblin" },
-							{ id: combatantId("2"), name: "Conjurer" },
-						],
-						activeIndex: 1,
-						roundNumber: 1,
-					}}
-					onAdvanceTurn={vi.fn()}
-					onRetreatTurn={vi.fn()}
-					onClearEncounter={vi.fn()}
-				/>,
-			);
+			mockContext({ combatants, activeIndex: 1 });
+			rerender(<TurnNavigation />);
 			expect(screen.getByText("Conjurer")).toBeInTheDocument();
 		});
 	});