Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 25 additions & 4 deletions client/src/components/deck-builder/CardGrid.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ interface CardGridProps {
onCardHover?: (cardName: string | null) => void;
cardCounts?: Map<string, number>;
legalityFormat?: BrowserLegalityFilter;
/** When false, the add affordance is disabled (copy ceiling reached). */
canAddCard?: (name: string) => boolean;
}

function getArtCropUrl(card: ScryfallCard): string {
Expand All @@ -35,6 +37,7 @@ export function CardGrid({
onCardHover,
cardCounts,
legalityFormat = "all",
canAddCard,
}: CardGridProps) {
return (
<div className="grid auto-rows-min grid-cols-[repeat(auto-fill,minmax(110px,1fr))] gap-2 overflow-y-auto p-2 sm:grid-cols-[repeat(auto-fill,minmax(130px,1fr))]">
Expand All @@ -44,6 +47,7 @@ export function CardGrid({
key={card.id ?? card.name}
card={card}
legal={isFormatLegal(card, legalityFormat)}
canAdd={canAddCard?.(card.name) ?? true}
count={cardCounts?.get(card.name)}
legalityFormat={legalityFormat}
onAddCard={onAddCard}
Expand All @@ -58,6 +62,7 @@ export function CardGrid({
interface CardGridTileProps {
card: ScryfallCard;
legal: boolean;
canAdd: boolean;
count: number | undefined;
legalityFormat: BrowserLegalityFilter;
onAddCard: (card: ScryfallCard) => void;
Expand All @@ -67,6 +72,7 @@ interface CardGridTileProps {
function CardGridTile({
card,
legal,
canAdd,
count,
legalityFormat,
onAddCard,
Expand All @@ -77,6 +83,7 @@ function CardGridTile({
const formatLabel = legalityFormat === "all"
? t("grid.allFormats")
: legalityFormat.charAt(0).toUpperCase() + legalityFormat.slice(1);
const addEnabled = legal && canAdd;

// Touch model (mirrors MobileHandDrawer's DrawerCard): tap adds the card,
// long-press opens the preview. firedRef suppresses the click that follows a
Expand All @@ -89,9 +96,15 @@ function CardGridTile({
firedRef.current = false;
return;
}
if (legal) onAddCard(card);
if (addEnabled) onAddCard(card);
};

const title = !legal
? t("grid.notLegal", { name: card.name, format: formatLabel })
: !canAdd
? t("grid.copyLimitReached", { name: card.name })
: t("grid.addCard", { name: card.name });

return (
<motion.button
type="button"
Expand All @@ -103,10 +116,10 @@ function CardGridTile({
onClick={handleClick}
{...mouseHoverPreview(onCardHover, card.name)}
{...handlers}
disabled={!legal}
title={legal ? t("grid.addCard", { name: card.name }) : t("grid.notLegal", { name: card.name, format: formatLabel })}
disabled={!addEnabled}
title={title}
className={`group relative cursor-pointer overflow-hidden rounded-lg transition-transform hover:scale-105 ${
legal
addEnabled
? "ring-2 ring-transparent hover:ring-green-500"
: "cursor-not-allowed opacity-60 ring-2 ring-red-600"
}`}
Expand All @@ -132,6 +145,14 @@ function CardGridTile({
</div>
)}

{legal && !canAdd && (
<div className="absolute inset-0 flex items-center justify-center bg-black/50">
<span className="rounded bg-red-700 px-2 py-0.5 text-[10px] font-bold text-white">
{t("grid.atCopyLimit")}
</span>
</div>
)}

{/* Legality badge */}
{legalityFormat !== "all" && (
<div className="absolute left-1 top-1">
Expand Down
1 change: 1 addition & 0 deletions client/src/components/deck-builder/DeckBuilder.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,7 @@ export function DeckBuilder({
onCardHover={onCardHover}
cardCounts={cardCounts}
legalityFormat={searchFilters.browseFormat}
canAddCard={canIncrement}
/>
</div>
) : (
Expand Down
73 changes: 73 additions & 0 deletions client/src/components/deck-builder/__tests__/CardGrid.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import { cleanup, fireEvent, render, screen } from "@testing-library/react";
import { afterEach, describe, expect, it, vi } from "vitest";
import { CardGrid } from "../CardGrid";
import type { ScryfallCard } from "../../../services/scryfall";

afterEach(cleanup);

vi.mock("react-i18next", () => ({
useTranslation: () => ({
t: (key: string, opts?: Record<string, string>) => {
if (key === "grid.copyLimitReached") return `${opts?.name} - at copy limit`;
if (key === "grid.atCopyLimit") return "At limit";
if (key === "grid.addCard") return `Add ${opts?.name}`;
if (key === "grid.notLegal") return `${opts?.name} - not legal`;
if (key === "grid.notFormat") return `Not ${opts?.format}`;
if (key === "grid.allFormats") return "all formats";
return key;
},
}),
}));

vi.mock("../../../hooks/useLongPress", () => ({
useLongPress: () => ({ handlers: {}, firedRef: { current: false } }),
}));

function makeCard(name: string): ScryfallCard {
return {
id: name.toLowerCase(),
name,
mana_cost: "",
cmc: 0,
type_line: "Artifact",
color_identity: [],
legalities: { modern: "legal" },
};
}

describe("CardGrid copy-limit affordance", () => {
it("disables add when canAddCard returns false", () => {
const onAddCard = vi.fn();
render(
<CardGrid
cards={[makeCard("Sol Ring")]}
onAddCard={onAddCard}
canAddCard={() => false}
legalityFormat="Modern"
/>,
);

const button = screen.getByRole("button", { name: /Sol Ring/i });
expect(button).toBeDisabled();
expect(button).toHaveAttribute("title", "Sol Ring - at copy limit");
fireEvent.click(button);
expect(onAddCard).not.toHaveBeenCalled();
});

it("adds when canAddCard allows it", () => {
const onAddCard = vi.fn();
render(
<CardGrid
cards={[makeCard("Sol Ring")]}
onAddCard={onAddCard}
canAddCard={() => true}
legalityFormat="Modern"
/>,
);

const button = screen.getByRole("button", { name: /Sol Ring/i });
expect(button).not.toBeDisabled();
fireEvent.click(button);
expect(onAddCard).toHaveBeenCalledTimes(1);
});
});
61 changes: 44 additions & 17 deletions client/src/components/deck-builder/useDeckBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import { getSharedAdapter } from "../../adapter/wasm-adapter";
import { useBracketEstimate } from "../../hooks/useBracketEstimate";
import { projectSignatureSpellForFormat } from "../../services/savedDeckProjection";
import {
canonicalDeckCountKey,
commanderPartnerCandidates,
companionCandidates,
isCardCommanderEligibleForFormat,
Expand Down Expand Up @@ -338,30 +339,41 @@ export function useDeckBuilder({

// CR 100.4a: the copy limit applies to the main deck, sideboard, and command
// zone combined, so the increment gate counts every slot a card can occupy.
// Counts are keyed by the engine's canonical name so alias spellings share a
// bucket (#6659) — never fold accents/case/DFC forms in the display layer.
const [canonicalKeys, setCanonicalKeys] = useState<Map<string, string>>(
() => new Map(),
);

const combinedCopyCounts = useMemo(() => {
const counts = new Map<string, number>();
const add = (name: string, n: number) =>
counts.set(name, (counts.get(name) ?? 0) + n);
const add = (name: string, n: number) => {
const key = canonicalKeys.get(name) ?? name;
counts.set(key, (counts.get(key) ?? 0) + n);
};
for (const entry of deck.main) add(entry.name, entry.count);
for (const entry of deck.sideboard) add(entry.name, entry.count);
for (const name of commanders) add(name, 1);
for (const name of deck.signature_spell ?? []) add(name, 1);
if (deck.companion) add(deck.companion, 1);
return counts;
}, [deck, commanders]);
}, [deck, commanders, canonicalKeys]);
Comment on lines 348 to +360

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

Keep copy-limit aggregation and eligibility in the engine.

This hook independently selects counted zones and derives copy-limit state. Expose an engine add-eligibility/count result for the deck partition and candidate instead; otherwise this UI logic can drift from copy_limit_violations as format rules evolve.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@client/src/components/deck-builder/useDeckBuilder.ts` around lines 348 - 360,
Remove the local combinedCopyCounts aggregation from the deck-builder hook and
use the engine’s add-eligibility/count result for the current deck partition and
candidate instead. Update the consuming logic to rely on the engine’s copy-limit
evaluation, keeping canonicalization and counted-zone selection centralized in
the engine so it remains consistent with copy_limit_violations.

Sources: Coding guidelines, Path instructions


// Distinct names currently in the partition, as a stable key — the ceiling
// for a (name, format) pair never changes, so this only refetches when the
// set of names or the format actually changes, not on every count edit.
const copyLimitKey = useMemo(
() =>
[
...new Set([...deck.main, ...deck.sideboard].map((entry) => entry.name)),
]
.sort()
.join("|"),
[deck.main, deck.sideboard],
);
const copyLimitKey = useMemo(() => {
const names = new Set<string>();
for (const entry of deck.main) names.add(entry.name);
for (const entry of deck.sideboard) names.add(entry.name);
for (const name of commanders) names.add(name);
for (const name of deck.signature_spell ?? []) names.add(name);
if (deck.companion) names.add(deck.companion);
// Search results need ceilings too so CardGrid can disable adds at the
// limit — including when the result's spelling differs from a deck entry.
for (const card of searchResults) names.add(card.name);
return [...names].sort().join("|");
}, [deck, commanders, searchResults]);

// CR 100.2a / CR 903.5b: the ceiling is engine-resolved per card and format
// (basic-land exemption, printed overrides like Relentless Rats or Seven
Expand All @@ -373,19 +385,33 @@ export function useDeckBuilder({
const names = copyLimitKey ? copyLimitKey.split("|") : [];
if (names.length === 0) {
setCopyLimits(new Map());
setCanonicalKeys(new Map());
return;
}
let cancelled = false;
Promise.all(
names.map(async (name) => [name, await maxDeckCopies(name, format)] as const),
names.map(async (name) => {
const [limit, canonical] = await Promise.all([
maxDeckCopies(name, format),
canonicalDeckCountKey(name),
]);
return [name, limit, canonical] as const;
}),
)
.then((results) => {
if (!cancelled) setCopyLimits(new Map(results));
if (cancelled) return;
setCopyLimits(new Map(results.map(([name, limit]) => [name, limit])));
setCanonicalKeys(
new Map(results.map(([name, , canonical]) => [name, canonical])),
);
})
.catch(() => {
// WASM may not be loaded yet; an empty map leaves increments open and
// the engine's compatibility warnings still flag any real violation.
if (!cancelled) setCopyLimits(new Map());
if (!cancelled) {
setCopyLimits(new Map());
setCanonicalKeys(new Map());
}
});
return () => {
cancelled = true;
Expand All @@ -396,9 +422,10 @@ export function useDeckBuilder({
(name: string) => {
const limit = copyLimits.get(name);
if (!limit || limit.type === "Unlimited") return true;
return (combinedCopyCounts.get(name) ?? 0) < limit.data;
const key = canonicalKeys.get(name) ?? name;
return (combinedCopyCounts.get(key) ?? 0) < limit.data;
},
[copyLimits, combinedCopyCounts],
[copyLimits, combinedCopyCounts, canonicalKeys],
);

const handleIncrementCard = useCallback(
Expand Down
4 changes: 3 additions & 1 deletion client/src/i18n/locales/de/deck-builder.json
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,9 @@
"allFormats": "Alle",
"addCard": "{{name}} hinzufügen",
"notLegal": "{{name}} – nicht legal in {{format}}",
"notFormat": "Nicht {{format}}"
"notFormat": "Nicht {{format}}",
"copyLimitReached": "{{name}} – Kopielimit erreicht",
"atCopyLimit": "Limit erreicht"
},
"deckList": {
"currentList": "Aktuelle Liste",
Expand Down
4 changes: 3 additions & 1 deletion client/src/i18n/locales/en/deck-builder.json
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,9 @@
"allFormats": "All",
"addCard": "Add {{name}}",
"notLegal": "{{name}} - Not {{format}} legal",
"notFormat": "Not {{format}}"
"notFormat": "Not {{format}}",
"copyLimitReached": "{{name}} - at copy limit",
"atCopyLimit": "At limit"
},
"deckList": {
"currentList": "Current List",
Expand Down
4 changes: 3 additions & 1 deletion client/src/i18n/locales/es/deck-builder.json
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,9 @@
"allFormats": "Todos",
"addCard": "Añadir {{name}}",
"notLegal": "{{name}} - No es legal en {{format}}",
"notFormat": "No es {{format}}"
"notFormat": "No es {{format}}",
"copyLimitReached": "{{name}} - límite de copias alcanzado",
"atCopyLimit": "En el límite"
},
"deckList": {
"currentList": "Lista actual",
Expand Down
4 changes: 3 additions & 1 deletion client/src/i18n/locales/fr/deck-builder.json
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,9 @@
"allFormats": "Tous",
"addCard": "Ajouter {{name}}",
"notLegal": "{{name}} - Non légal en {{format}}",
"notFormat": "Non {{format}}"
"notFormat": "Non {{format}}",
"copyLimitReached": "{{name}} - limite de copies atteinte",
"atCopyLimit": "Limite atteinte"
},
"deckList": {
"currentList": "Liste actuelle",
Expand Down
4 changes: 3 additions & 1 deletion client/src/i18n/locales/it/deck-builder.json
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,9 @@
"allFormats": "Tutti",
"addCard": "Aggiungi {{name}}",
"notLegal": "{{name}} - Non legale in {{format}}",
"notFormat": "Non {{format}}"
"notFormat": "Non {{format}}",
"copyLimitReached": "{{name}} - limite copie raggiunto",
"atCopyLimit": "Al limite"
},
"deckList": {
"currentList": "Lista corrente",
Expand Down
4 changes: 3 additions & 1 deletion client/src/i18n/locales/pl/deck-builder.json
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,9 @@
"allFormats": "Wszystkie",
"addCard": "Dodaj {{name}}",
"notLegal": "{{name}} - Niedozwolona w formacie {{format}}",
"notFormat": "Nie {{format}}"
"notFormat": "Nie {{format}}",
"copyLimitReached": "{{name}} - osiągnięto limit kopii",
"atCopyLimit": "Limit"
},
"deckList": {
"currentList": "Bieżąca lista",
Expand Down
4 changes: 3 additions & 1 deletion client/src/i18n/locales/pt/deck-builder.json
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,9 @@
"allFormats": "Todos",
"addCard": "Adicionar {{name}}",
"notLegal": "{{name}} - Não válido em {{format}}",
"notFormat": "Não {{format}}"
"notFormat": "Não {{format}}",
"copyLimitReached": "{{name}} - limite de cópias atingido",
"atCopyLimit": "No limite"
},
"deckList": {
"currentList": "Lista Atual",
Expand Down
11 changes: 11 additions & 0 deletions client/src/services/engineRuntime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,17 @@ export async function maxDeckCopies(
return engine.maxDeckCopies(name, format) as DeckCopyLimit;
}

/**
* CR 201.3 + CR 100.2a: Engine canonical key for aggregating deck copy counts.
* Alias spellings, case variants, and DFC combined/front-face names share one
* bucket — the deck builder must key affordance counts on this value.
*/
export async function canonicalDeckCountKey(name: string): Promise<string> {
await ensureCardDatabase();
const engine = await loadEngineModule();
return engine.canonicalDeckCountKey(name) as string;
}

/**
* CR 100.4a: Per-format sideboard policy as a discriminated union.
*
Expand Down
Loading
Loading