From e3a565d9764d57c8c94a72f43a2fc0deb6e56abc Mon Sep 17 00:00:00 2001
From: gorkem-bwl <gorkem.cetin@bluewavelabs.ca>
Date: Sat, 29 Aug 2026 10:51:07 -0400
Subject: [PATCH] fix(client): Escape closes the card picker, and lay it out in
 two columns
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Escape did not just fail to close the dialog — it reset the dashboard.
The shared Dialog routes an escapeKeyDown close to onCancel, and this
picker puts Reset to defaults in the cancel slot, so pressing Escape
wiped the user's card selection. Dialog gains an optional onEscape so a
dialog whose cancel slot carries another action can dismiss properly;
without it the behaviour is unchanged for every other caller.

The card list is now two columns. Seventeen cards in a single column
made the dialog scroll, hiding most of the catalogue behind a scrollbar.
Groups flow down the columns and are kept whole with break-inside, so a
group is never split across the gap. Single column below the sm
breakpoint.

Also hardens the selection against silent loss: the persisting effect no
longer writes on mount. That write was a no-op in the normal case, since
the value had just been read from storage — but if the read ever fell
back to the defaults (unparseable value, storage throwing), the effect
immediately overwrote the real selection and made the loss permanent.

Claude-Session: https://claude.ai/code/session_01AbYLvwpEaZFXktHVcqrFRd
---
 client/src/Components/inputs/Dialog.tsx       | 14 +++++++++++-
 .../Pages/Dashboard/Components/CardPicker.tsx | 22 ++++++++++++++++---
 .../src/Pages/Dashboard/useCardSelection.ts   | 12 +++++++++-
 3 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/client/src/Components/inputs/Dialog.tsx b/client/src/Components/inputs/Dialog.tsx
index 80d260a55..dafc3845a 100644
--- a/client/src/Components/inputs/Dialog.tsx
+++ b/client/src/Components/inputs/Dialog.tsx
@@ -27,6 +27,7 @@ export const DialogInput = ({
 	fullWidth = false,
 	additionalButtons,
 	cancelSx,
+	onEscape,
 }: {
 	open: boolean;
 	title?: string;
@@ -43,6 +44,12 @@ export const DialogInput = ({
 	additionalButtons?: ReactNode;
 	/** Styles the cancel button — e.g. an auto margin to push it left. */
 	cancelSx?: SxProps;
+	/**
+	 * Called when Escape is pressed. Defaults to `onCancel`, which is right when
+	 * cancel simply dismisses — but not when it does something else, so a dialog
+	 * whose cancel slot carries another action passes its own dismiss here.
+	 */
+	onEscape?(): void;
 }) => {
 	const { t } = useTranslation();
 	const theme = useTheme();
@@ -52,7 +59,12 @@ export const DialogInput = ({
 			maxWidth={maxWidth}
 			fullWidth={fullWidth}
 			onClose={(_event, reason) => {
-				if (reason !== "backdropClick" && onCancel) onCancel(undefined);
+				if (reason === "backdropClick") return;
+				if (onEscape) {
+					onEscape();
+					return;
+				}
+				if (onCancel) onCancel(undefined);
 			}}
 		>
 			{title && (
diff --git a/client/src/Pages/Dashboard/Components/CardPicker.tsx b/client/src/Pages/Dashboard/Components/CardPicker.tsx
index 2ff927142..a01346926 100644
--- a/client/src/Pages/Dashboard/Components/CardPicker.tsx
+++ b/client/src/Pages/Dashboard/Components/CardPicker.tsx
@@ -1,4 +1,5 @@
 import Stack from "@mui/material/Stack";
+import Box from "@mui/material/Box";
 import Typography from "@mui/material/Typography";
 import FormControlLabel from "@mui/material/FormControlLabel";
 import { useTheme } from "@mui/material/styles";
@@ -59,19 +60,34 @@ export const CardPicker = ({
 			onCancel={onReset}
 			cancelText={t("pages.dashboard.picker.reset")}
 			cancelSx={{ marginRight: "auto" }}
+			// Escape must dismiss, not reset. The dialog sends Escape to onCancel by
+			// default, and this one's cancel slot resets the selection — so without
+			// this, Escape would wipe the user's cards instead of closing.
+			onEscape={onClose}
 			// Changes apply immediately, so Done only dismisses; it is the primary
 			// action here and carries the usual contained styling.
 			onConfirm={onClose}
 			confirmText={t("pages.dashboard.picker.done")}
 		>
-			<Stack
-				gap={theme.spacing(LAYOUT.MD)}
+			{/*
+			 * Two columns: seventeen cards in one list makes the dialog scroll,
+			 * which hides most of the catalogue behind a scrollbar. Groups flow
+			 * down the columns and are kept whole, so a group is never split
+			 * across the gap.
+			 */}
+			<Box
 				pt={theme.spacing(LAYOUT.SM)}
+				sx={{
+					columnCount: { xs: 1, sm: 2 },
+					columnGap: theme.spacing(LAYOUT.XL),
+				}}
 			>
 				{grouped.map(({ group, cards }) => (
 					<Stack
 						key={group}
 						gap={theme.spacing(LAYOUT.XXS)}
+						pb={theme.spacing(LAYOUT.MD)}
+						sx={{ breakInside: "avoid" }}
 					>
 						<Typography
 							variant="eyebrow"
@@ -110,7 +126,7 @@ export const CardPicker = ({
 						))}
 					</Stack>
 				))}
-			</Stack>
+			</Box>
 		</Dialog>
 	);
 };
diff --git a/client/src/Pages/Dashboard/useCardSelection.ts b/client/src/Pages/Dashboard/useCardSelection.ts
index a8e939c0a..7d8ae5530 100644
--- a/client/src/Pages/Dashboard/useCardSelection.ts
+++ b/client/src/Pages/Dashboard/useCardSelection.ts
@@ -1,4 +1,4 @@
-import { useCallback, useEffect, useMemo, useState } from "react";
+import { useCallback, useEffect, useMemo, useRef, useState } from "react";
 
 import { logger } from "@/Utils/logger";
 import { useIsAdmin } from "@/Hooks/useIsAdmin";
@@ -50,7 +50,17 @@ export const useCardSelection = () => {
 		() => readStored() ?? DEFAULT_CARD_IDS
 	);
 
+	// Skip the write on mount. The value has just been read from storage, so
+	// rewriting it achieves nothing — but if the read failed for any reason and
+	// fell back to the defaults, that write would overwrite the user's real
+	// selection and make the loss permanent.
+	const hasMounted = useRef(false);
+
 	useEffect(() => {
+		if (!hasMounted.current) {
+			hasMounted.current = true;
+			return;
+		}
 		try {
 			window.localStorage.setItem(DASHBOARD_CARDS_STORAGE_KEY, JSON.stringify(selected));
 		} catch (error) {