feat(modifiers): persist bound layout on profile + drive Lobby layout from profile

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 <LayoutName>" 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.
This commit is contained in:
Joey Yakimowich-Payne 2026-04-21 13:28:06 -06:00
commit 4b08b0c71c
No known key found for this signature in database
4 changed files with 387 additions and 75 deletions

View file

@ -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<void> {
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<void> {
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');
});
});

View file

@ -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++) {

View file

@ -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
</h2>
<div className="bg-white/50 rounded-xl p-5 border border-neutral-200/60 shadow-inner space-y-4">
<LayoutPicker
value={selectedLayout}
onChange={setSelectedLayout}
activations={presets}
setPresets={setPresets}
onCustomRequested={() => 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. */}
<div className="space-y-1.5">
<label className="text-xs font-bold text-neutral-500 uppercase tracking-widest">
Play as
</label>
<div
className="flex gap-2"
role="radiogroup"
aria-label="Host color preference"
>
{(['white', 'black', 'random'] as const).map((choice) => {
const isActive = preferredColor === choice;
const label =
choice === 'white'
? 'White'
: choice === 'black'
? 'Black'
: 'Random';
return (
<button
key={choice}
type="button"
role="radio"
aria-checked={isActive}
data-testid={`color-preference-${choice}`}
onClick={() => setPreferredColor(choice)}
disabled={loading}
className={`flex-1 px-3 py-2 rounded-lg text-sm font-semibold border transition-colors focus:outline-none focus:ring-2 focus:ring-blue-500 disabled:opacity-50 disabled:cursor-not-allowed ${
isActive
? 'bg-neutral-900 text-white border-neutral-900 hover:bg-neutral-800'
: 'bg-white text-neutral-700 border-neutral-200 hover:bg-neutral-50'
}`}
>
{label}
</button>
);
})}
</div>
</div>
{/* 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. */}
<div className="space-y-1.5">
<label
htmlFor="profile-picker-input"
@ -529,11 +510,6 @@ export function Lobby({ chessState }: LobbyProps = {}) {
{entry.starred ? ' ★' : ''}
</option>
))}
{/* If the picker's selected id isn't in the library
(typical for a ?modifierProfile= URL param), add
a synthetic row so <select> can still reflect the
choice. Using a distinct prefix (__synth__:) to
guarantee no collision with library ids. */}
{selectedProfile !== null &&
!savedProfiles.some((p) => p.id === profilePickerValue) &&
profilePickerValue !== PROFILE_NONE &&
@ -545,11 +521,7 @@ export function Lobby({ chessState }: LobbyProps = {}) {
<option value={PROFILE_CUSTOM}>Custom</option>
</select>
{/* T27: stacked profiles below the primary picker.
Solo-only multiplayer create/join sends only the
primary `selectedProfile`. Each entry can be reordered
or removed; ordering decides priority-wins precedence
(last in list wins). */}
{/* T27: stacked profiles below the primary picker. */}
<div data-testid="profile-stack" className="space-y-1">
{additionalProfiles.length > 0 && (
<p className="text-[10px] font-bold text-neutral-400 uppercase tracking-widest pt-2">
@ -623,7 +595,6 @@ export function Lobby({ chessState }: LobbyProps = {}) {
if (id === '') return;
const entry = savedProfiles.find((p) => p.id === id);
if (entry === undefined) return;
// Prevent duplicate of the primary or any existing stack entry.
if (entry.profile.id === selectedProfile.id) return;
if (additionalProfiles.some((p) => p.id === entry.profile.id)) return;
setAdditionalProfiles((prev) => [...prev, entry.profile]);
@ -649,6 +620,103 @@ export function Lobby({ chessState }: LobbyProps = {}) {
</div>
</div>
<LayoutPicker
value={selectedLayout}
onChange={setSelectedLayout}
activations={presets}
setPresets={setPresets}
onCustomRequested={() => 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 (
<div
data-testid="layout-profile-mismatch"
className="flex items-start gap-2 rounded-md border border-amber-200 bg-amber-50 px-3 py-2 text-xs text-amber-900"
>
<span className="font-semibold">Heads up:</span>
<div className="flex-1">
"{selectedProfile.name}" was authored against{' '}
<span className="font-semibold">
{intended?.name ?? selectedProfile.layoutId}
</span>
. Per-instance modifiers may not match the
current layout.{' '}
{intended !== undefined && (
<button
type="button"
data-testid="layout-profile-mismatch-switch"
onClick={() => setSelectedLayout(intended)}
className="font-semibold underline underline-offset-2 hover:text-amber-950"
>
Switch to {intended.name}
</button>
)}
</div>
</div>
);
})()}
{/* 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. */}
<div className="space-y-1.5">
<label className="text-xs font-bold text-neutral-500 uppercase tracking-widest">
Play as
</label>
<div
className="flex gap-2"
role="radiogroup"
aria-label="Host color preference"
>
{(['white', 'black', 'random'] as const).map((choice) => {
const isActive = preferredColor === choice;
const label =
choice === 'white'
? 'White'
: choice === 'black'
? 'Black'
: 'Random';
return (
<button
key={choice}
type="button"
role="radio"
aria-checked={isActive}
data-testid={`color-preference-${choice}`}
onClick={() => setPreferredColor(choice)}
disabled={loading}
className={`flex-1 px-3 py-2 rounded-lg text-sm font-semibold border transition-colors focus:outline-none focus:ring-2 focus:ring-blue-500 disabled:opacity-50 disabled:cursor-not-allowed ${
isActive
? 'bg-neutral-900 text-white border-neutral-900 hover:bg-neutral-800'
: 'bg-white text-neutral-700 border-neutral-200 hover:bg-neutral-50'
}`}
>
{label}
</button>
);
})}
</div>
</div>
<button
data-action="create-room"
onClick={handleCreate}

View file

@ -95,6 +95,14 @@ export function ModifierProfileEditor({
}, [history.length]);
const [libraryVersion, setLibraryVersion] = useState(0);
/**
* `boundLayout` is the UI-side mirror of `profile.layoutId`. Every
* change writes through to the profile (see the `<select onChange>`
* handler below) so the binding is saved with the rest of the
* descriptor. On modal open and on library-load, we hydrate from
* `profile.layoutId` back to a resolved StartingLayout so the board
* preview + conflict resolver get a real object rather than an id.
*/
const [boundLayout, setBoundLayout] = useState<StartingLayout | null>(null);
const [clipboard, setClipboard] = useState<ModifierClipboard>({ kind: 'empty' });
const layouts = useMemo(() => LAYOUT_REGISTRY.list(), []);
@ -104,9 +112,29 @@ export function ModifierProfileEditor({
if (isOpen) {
setHistory([makeBlankProfile()]);
setHistoryIndex(0);
setBoundLayout(null);
}
}, [isOpen]);
// Keep boundLayout in sync with the active profile's layoutId.
// Triggered on modal open (after history reset) AND on every Load
// from the library / undo / redo — anywhere the profile reference
// changes. Hydrates from LAYOUT_REGISTRY so the Panel/Conflict
// panels get a real StartingLayout object, not just an id.
useEffect(() => {
const desiredId = profile.layoutId;
const currentId = boundLayout?.id ?? undefined;
if (desiredId === currentId) return;
if (desiredId === undefined) {
setBoundLayout(null);
return;
}
const resolved = LAYOUT_REGISTRY.get(desiredId);
setBoundLayout(resolved ?? null);
// boundLayout intentionally omitted from the deps — it's the
// thing we're syncing TO, not an input that should re-trigger.
}, [profile.layoutId, boundLayout?.id]);
// Esc closes the modal regardless of focus.
//
// We use capture phase + stopImmediatePropagation so that other
@ -246,9 +274,16 @@ export function ModifierProfileEditor({
const id = e.target.value;
const newLayout = id ? (LAYOUT_REGISTRY.get(id) ?? null) : null;
setBoundLayout(newLayout);
// Any layout change is tracked as an action because per-instance
// rules are inherently bound to the layout context.
pushSnapshot((p) => ({ ...p }));
// Persist the selection ON the profile so it travels
// with the saved descriptor. `layoutId` is optional
// in the schema — undefined when the user clears it.
pushSnapshot((p) => {
if (newLayout === null) {
const { layoutId: _drop, ...rest } = p;
return { ...rest };
}
return { ...p, layoutId: newLayout.id };
});
}}
className="text-sm border border-neutral-300 rounded px-2 py-1 focus:outline-none focus:ring-2 focus:ring-blue-500 max-w-[160px]"
>