fix: address code review findings — data loss, race condition, broken features

- TopBar: call closeBoard() before navigating back to prevent data loss
- board-store: guard debouncedSave against race condition when board is
  closed during an in-flight save
- board-store: add missing updatedAt to setColumnWidth
- useKeyboardShortcuts: remove duplicate Ctrl+K handler that prevented
  command palette from toggling closed
- AttachmentSection: wire up Tauri file dialog for adding attachments
  with link/copy mode support
This commit is contained in:
Your Name
2026-02-15 19:33:25 +02:00
parent 1da5f9834b
commit d2adc68262
4 changed files with 38 additions and 14 deletions

View File

@@ -1,6 +1,8 @@
import { FileIcon, X, Plus } from "lucide-react"; import { FileIcon, X, Plus } from "lucide-react";
import { open } from "@tauri-apps/plugin-dialog";
import { Button } from "@/components/ui/button"; import { Button } from "@/components/ui/button";
import { useBoardStore } from "@/stores/board-store"; import { useBoardStore } from "@/stores/board-store";
import { copyAttachment } from "@/lib/storage";
import type { Attachment } from "@/types/board"; import type { Attachment } from "@/types/board";
interface AttachmentSectionProps { interface AttachmentSectionProps {
@@ -12,11 +14,29 @@ export function AttachmentSection({
cardId, cardId,
attachments, attachments,
}: AttachmentSectionProps) { }: AttachmentSectionProps) {
const addAttachment = useBoardStore((s) => s.addAttachment);
const removeAttachment = useBoardStore((s) => s.removeAttachment); const removeAttachment = useBoardStore((s) => s.removeAttachment);
function handleAdd() { async function handleAdd() {
// Placeholder: Tauri file dialog will be wired in a later task const selected = await open({
console.log("Add attachment (file dialog not yet wired)"); multiple: false,
title: "Select attachment",
});
if (!selected) return;
const fileName = selected.split(/[\\/]/).pop() ?? "attachment";
const board = useBoardStore.getState().board;
if (!board) return;
const mode = board.settings.attachmentMode;
if (mode === "copy") {
const destPath = await copyAttachment(board.id, selected, fileName);
addAttachment(cardId, { name: fileName, path: destPath, mode: "copy" });
} else {
addAttachment(cardId, { name: fileName, path: selected, mode: "link" });
}
} }
return ( return (

View File

@@ -75,7 +75,10 @@ export function TopBar() {
<Button <Button
variant="ghost" variant="ghost"
size="sm" size="sm"
onClick={() => setView({ type: "board-list" })} onClick={() => {
useBoardStore.getState().closeBoard();
setView({ type: "board-list" });
}}
className="text-pylon-text-secondary hover:text-pylon-text" className="text-pylon-text-secondary hover:text-pylon-text"
> >
<ArrowLeft className="size-4" /> <ArrowLeft className="size-4" />

View File

@@ -17,14 +17,8 @@ export function useKeyboardShortcuts(): void {
function handleKeyDown(e: KeyboardEvent) { function handleKeyDown(e: KeyboardEvent) {
const ctrl = e.ctrlKey || e.metaKey; const ctrl = e.ctrlKey || e.metaKey;
// Ctrl+K: open command palette (always works, even in inputs)
if (ctrl && e.key === "k") {
e.preventDefault();
document.dispatchEvent(new CustomEvent("open-command-palette"));
return;
}
// Skip remaining shortcuts when an input is focused // Skip remaining shortcuts when an input is focused
// Note: Ctrl+K for command palette is handled directly by CommandPalette component
if (isInputFocused()) return; if (isInputFocused()) return;
// Ctrl+Shift+Z: redo // Ctrl+Shift+Z: redo

View File

@@ -63,6 +63,7 @@ function now(): string {
function debouncedSave( function debouncedSave(
board: Board, board: Board,
get: () => BoardState & BoardActions,
set: (partial: Partial<BoardState>) => void set: (partial: Partial<BoardState>) => void
): void { ): void {
if (saveTimeout) clearTimeout(saveTimeout); if (saveTimeout) clearTimeout(saveTimeout);
@@ -70,9 +71,14 @@ function debouncedSave(
set({ saving: true }); set({ saving: true });
try { try {
await saveBoard(board); await saveBoard(board);
set({ saving: false, lastSaved: Date.now() }); // Only update state if the same board is still loaded
if (get().board?.id === board.id) {
set({ saving: false, lastSaved: Date.now() });
}
} catch { } catch {
set({ saving: false }); if (get().board?.id === board.id) {
set({ saving: false });
}
} }
}, 500); }, 500);
} }
@@ -86,7 +92,7 @@ function mutate(
if (!board) return; if (!board) return;
const updated = updater(board); const updated = updater(board);
set({ board: updated }); set({ board: updated });
debouncedSave(updated, set); debouncedSave(updated, get, set);
} }
export const useBoardStore = create<BoardState & BoardActions>()( export const useBoardStore = create<BoardState & BoardActions>()(
@@ -166,6 +172,7 @@ export const useBoardStore = create<BoardState & BoardActions>()(
setColumnWidth: (columnId, width) => { setColumnWidth: (columnId, width) => {
mutate(get, set, (b) => ({ mutate(get, set, (b) => ({
...b, ...b,
updatedAt: now(),
columns: b.columns.map((c) => columns: b.columns.map((c) =>
c.id === columnId ? { ...c, width } : c c.id === columnId ? { ...c, width } : c
), ),