From 4b08b0c71cc0bca99bdacf85a5a6f061a227a45a Mon Sep 17 00:00:00 2001 From: Joey Yakimowich-Payne Date: Tue, 21 Apr 2026 13:28:06 -0600 Subject: [PATCH] feat(modifiers): persist bound layout on profile + drive Lobby layout from profile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously the ModifierProfileEditor's layout picker was editor-only UI state — it drove per-instance board preview and live validation, but was dropped on save. This meant per-instance modifiers (square- bound to their authoring layout) silently became orphans in the Lobby if the user picked a different layout at apply time, with no signal about the mismatch. Changes: - ModifierProfileEditor now writes the bound layout through to profile.layoutId (schema already supported the optional field) and rehydrates boundLayout from profile.layoutId on open / load / undo / redo. - Lobby rearranged so the Modifier Profile picker sits ABOVE the Layout Picker. Picking a profile with a layoutId binding snaps selectedLayout to match. Same behavior on URL ?modifierProfile deep-links and after the editor closes with a new save. - Mismatch banner (amber) surfaces when the user overrides the layout after picking a bound profile, with a one-click "Switch to " restore. Unit test added for library round-trip of layoutId; new profile-layout-binding.spec.ts covers the four scenarios: snap, manual override + mismatch banner, unbound profile leaves layout alone, and editor-save persists layoutId. --- .../chess/e2e/profile-layout-binding.spec.ts | 193 ++++++++++++++++ packages/chess/src/modifiers/library.test.ts | 16 ++ packages/chess/src/ui/Lobby.tsx | 212 ++++++++++++------ .../chess/src/ui/ModifierProfileEditor.tsx | 41 +++- 4 files changed, 387 insertions(+), 75 deletions(-) create mode 100644 packages/chess/e2e/profile-layout-binding.spec.ts diff --git a/packages/chess/e2e/profile-layout-binding.spec.ts b/packages/chess/e2e/profile-layout-binding.spec.ts new file mode 100644 index 0000000..4722761 --- /dev/null +++ b/packages/chess/e2e/profile-layout-binding.spec.ts @@ -0,0 +1,193 @@ +/** + * E2E — Modifier profile ⇄ layout binding. + * + * Per-instance modifiers are square-bound to the layout they were + * authored against. Rather than leaving this coupling implicit, the + * profile descriptor now carries an optional `layoutId` that: + * 1. Is persisted when the user sets a bound layout in the + * ModifierProfileEditor, AND + * 2. Drives the Lobby's selectedLayout when the profile is picked, + * AND + * 3. Surfaces a mismatch warning if the user manually overrides + * the layout after picking the profile. + * + * This spec exercises the full save → pick → auto-switch → mismatch + * loop. + */ +import { test, expect, type Page } from '@playwright/test'; + +const PROFILE_LIBRARY_KEY = 'houserules:modifier-profiles:v1'; + +/** + * Seed a saved profile with `layoutId` pre-bound so downstream tests + * don't have to open the editor to set it up. Mirrors the shape the + * ModifierProfileEditor would write to localStorage via saveToLibrary. + */ +async function seedBoundProfile( + page: Page, + id: string, + name: string, + layoutId: string, +): Promise { + await page.evaluate( + ({ key, id, name, layoutId }) => { + const entry = { + id, + name, + profile: { + id, + name, + description: '', + layoutId, + perType: [], + perInstance: [], + version: 1, + source: 'custom', + }, + starred: false, + updatedAt: Date.now(), + }; + localStorage.setItem(key, JSON.stringify([entry])); + }, + { key: PROFILE_LIBRARY_KEY, id, name, layoutId }, + ); +} + +async function freshLobby(page: Page): Promise { + await page.goto('/'); + await page.evaluate(() => { + localStorage.clear(); + sessionStorage.clear(); + }); + await page.reload(); + await expect(page.getByTestId('page-home')).toBeVisible(); +} + +test.describe('Modifier profile ⇄ layout binding', () => { + test('picking a profile with layoutId snaps the lobby layout', async ({ + page, + }) => { + await freshLobby(page); + await seedBoundProfile( + page, + 'lib-horde-bound', + 'Horde Profile', + 'horde', + ); + await page.reload(); + + // Pre-condition: lobby starts on Classic. + await expect(page.getByTestId('layout-picker')).toHaveValue('classic'); + + await page.getByTestId('profile-picker').selectOption('lib-horde-bound'); + + // Auto-switch: LayoutPicker snaps to the profile's authored layout. + await expect(page.getByTestId('layout-picker')).toHaveValue('horde'); + // No mismatch banner should appear because the snap matches. + await expect( + page.getByTestId('layout-profile-mismatch'), + ).toHaveCount(0); + }); + + test('mismatch banner appears when user overrides layout after picking bound profile', async ({ + page, + }) => { + await freshLobby(page); + await seedBoundProfile( + page, + 'lib-horde-bound', + 'Horde Profile', + 'horde', + ); + await page.reload(); + + await page.getByTestId('profile-picker').selectOption('lib-horde-bound'); + await expect(page.getByTestId('layout-picker')).toHaveValue('horde'); + + // User overrides to a different layout — mismatch surfaces. + await page.getByTestId('layout-picker').selectOption('classic'); + const banner = page.getByTestId('layout-profile-mismatch'); + await expect(banner).toBeVisible(); + await expect(banner).toContainText('Horde Profile'); + + // Clicking "Switch to …" restores the profile's authored layout. + await page.getByTestId('layout-profile-mismatch-switch').click(); + await expect(page.getByTestId('layout-picker')).toHaveValue('horde'); + await expect( + page.getByTestId('layout-profile-mismatch'), + ).toHaveCount(0); + }); + + test('profile without layoutId leaves the layout unchanged', async ({ + page, + }) => { + await freshLobby(page); + // Profile without a layoutId. + await page.evaluate( + ({ key }) => { + const entry = { + id: 'lib-unbound', + name: 'Unbound Profile', + profile: { + id: 'lib-unbound', + name: 'Unbound Profile', + description: '', + perType: [], + perInstance: [], + version: 1, + source: 'custom', + }, + starred: false, + updatedAt: Date.now(), + }; + localStorage.setItem(key, JSON.stringify([entry])); + }, + { key: PROFILE_LIBRARY_KEY }, + ); + await page.reload(); + + // Pick some non-default layout first. + await page.getByTestId('layout-picker').selectOption('horde'); + await expect(page.getByTestId('layout-picker')).toHaveValue('horde'); + + // Picking an unbound profile should NOT steal the layout. + await page.getByTestId('profile-picker').selectOption('lib-unbound'); + await expect(page.getByTestId('layout-picker')).toHaveValue('horde'); + await expect( + page.getByTestId('layout-profile-mismatch'), + ).toHaveCount(0); + }); + + test('editor persists the bound layout on save', async ({ page }) => { + await freshLobby(page); + + // Open the editor via the picker's "Custom…" option. + await page.getByTestId('profile-picker').selectOption('custom'); + await expect( + page.getByTestId('modifier-editor-modal'), + ).toBeVisible({ timeout: 3000 }); + + // Set a name and bind the layout. + await page.getByTestId('profile-name').fill('My Bound Test'); + await page.getByTestId('bound-layout-picker').selectOption('horde'); + + // Save + close the editor. + await page.getByTestId('save-profile').click(); + await page.keyboard.press('Escape'); + + // Confirm the library entry carries layoutId. + const stored = await page.evaluate( + ({ key }) => localStorage.getItem(key), + { key: PROFILE_LIBRARY_KEY }, + ); + expect(stored).not.toBeNull(); + const entries = JSON.parse(stored as string) as { + profile: { layoutId?: string; name: string }; + }[]; + const mine = entries.find((e) => e.profile.name === 'My Bound Test'); + expect(mine?.profile.layoutId).toBe('horde'); + + // And the auto-selected newest entry already picks the bound layout. + await expect(page.getByTestId('layout-picker')).toHaveValue('horde'); + }); +}); diff --git a/packages/chess/src/modifiers/library.test.ts b/packages/chess/src/modifiers/library.test.ts index f29bb6d..ebc66fd 100644 --- a/packages/chess/src/modifiers/library.test.ts +++ b/packages/chess/src/modifiers/library.test.ts @@ -135,6 +135,22 @@ describe("saveToLibrary()", () => { expect(library[0]?.name).toBe("Renamed"); }); + it("preserves layoutId through the save/load round-trip", () => { + const boundProfile: ModifierProfile = { + ...emptyProfile, + id: "bound-profile", + layoutId: "classic", + }; + saveToLibrary( + seed({ + id: "lib-bound", + profile: boundProfile, + }), + ); + const [reloaded] = loadLibrary(); + expect(reloaded?.profile.layoutId).toBe("classic"); + }); + it("evicts the oldest non-starred when MAX_ENTRIES is hit", () => { // Seed with MAX_ENTRIES entries, incrementing updatedAt. for (let i = 0; i < __test__.MAX_ENTRIES; i++) { diff --git a/packages/chess/src/ui/Lobby.tsx b/packages/chess/src/ui/Lobby.tsx index f7c47c7..a71f61a 100644 --- a/packages/chess/src/ui/Lobby.tsx +++ b/packages/chess/src/ui/Lobby.tsx @@ -156,6 +156,20 @@ export function Lobby({ chessState }: LobbyProps = {}) { ); setSelectedProfile(profile); setProfilePickerValue(profile.id); + // URL-carried profiles also honour their `layoutId` binding: + // if the shared profile was authored against a known layout, + // snap to it unless an explicit `?layoutId` / `?fen` was also + // given on the URL (handled earlier in this effect). + if ( + profile.layoutId !== undefined && + searchParams.get('layoutId') === null && + searchParams.get('fen') === null + ) { + const layout = LAYOUT_REGISTRY.get(profile.layoutId); + if (layout !== undefined) { + setSelectedLayout(layout); + } + } } catch { /* invalid param — ignore */ } @@ -203,6 +217,20 @@ export function Lobby({ chessState }: LobbyProps = {}) { } setProfilePickerValue(entry.id); setSelectedProfile(entry.profile); + // If the profile was authored against a specific layout, snap the + // lobby's layout picker to match. Per-instance modifiers are + // square-bound to their authoring layout — silently applying them + // to the wrong one yields orphan warnings at apply time and a + // confused user. When the id doesn't resolve (layout deleted / + // not registered), leave the current layout alone; the mismatch + // badge below will surface the situation. + const desired = entry.profile.layoutId; + if (desired !== undefined) { + const layout = LAYOUT_REGISTRY.get(desired); + if (layout !== undefined) { + setSelectedLayout(layout); + } + } } /** @@ -219,6 +247,15 @@ export function Lobby({ chessState }: LobbyProps = {}) { const newest = [...fresh].sort((a, b) => b.updatedAt - a.updatedAt)[0]!; setProfilePickerValue(newest.id); setSelectedProfile(newest.profile); + // Mirror handleProfileChange: if the just-authored profile bound + // a specific layout, snap the lobby's layout picker to match. + const desired = newest.profile.layoutId; + if (desired !== undefined) { + const layout = LAYOUT_REGISTRY.get(desired); + if (layout !== undefined) { + setSelectedLayout(layout); + } + } } /** @@ -446,67 +483,11 @@ export function Lobby({ chessState }: LobbyProps = {}) { Host Game
- setEditorOpen(true)} - disabled={loading} - /> - - {/* F1 (post-epic-deferrals) — host color preference. - `"white"` is the back-compat default; we send the - field on the wire only when the user picks something - else. `"random"` is resolved server-side at room - creation, so the server's `room.created` response - carries the concrete assigned color. */} -
- -
- {(['white', 'black', 'random'] as const).map((choice) => { - const isActive = preferredColor === choice; - const label = - choice === 'white' - ? 'White' - : choice === 'black' - ? 'Black' - : 'Random'; - return ( - - ); - })} -
-
- - {/* Modifier profile picker (T26). The list is loaded from - the local library on mount and refreshed when the - ModifierProfileEditor closes. Selecting "Custom…" - opens the editor; any URL-pre-selected profile (via - ?modifierProfile=…) shows up as a synthetic entry so - the user's choice is preserved even if they haven't - saved it locally. */} + {/* Modifier profile picker (T26). Rendered ABOVE the + layout picker because a profile with a `layoutId` + binding auto-drives the layout selection — picking + profile first avoids the user overriding a sensible + default layout we're about to snap to. */}
+ setEditorOpen(true)} + disabled={loading} + /> + + {/* Layout ⇄ profile binding mismatch warning. Surfaces + when the active profile was authored against a + specific layout but the user has since picked a + different one — per-instance modifiers are + square-bound to their authoring layout, so mixing + silently yields orphan warnings at apply time. One- + click "Switch back" restores the profile's intended + layout. */} + {selectedProfile !== null && + selectedProfile.layoutId !== undefined && + selectedProfile.layoutId !== selectedLayout.id && + (() => { + const intended = LAYOUT_REGISTRY.get( + selectedProfile.layoutId, + ); + return ( +
+ Heads up: +
+ "{selectedProfile.name}" was authored against{' '} + + {intended?.name ?? selectedProfile.layoutId} + + . Per-instance modifiers may not match the + current layout.{' '} + {intended !== undefined && ( + + )} +
+
+ ); + })()} + + {/* F1 (post-epic-deferrals) — host color preference. + `"white"` is the back-compat default; we send the + field on the wire only when the user picks something + else. `"random"` is resolved server-side at room + creation, so the server's `room.created` response + carries the concrete assigned color. */} +
+ +
+ {(['white', 'black', 'random'] as const).map((choice) => { + const isActive = preferredColor === choice; + const label = + choice === 'white' + ? 'White' + : choice === 'black' + ? 'Black' + : 'Random'; + return ( + + ); + })} +
+
+