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:
@@ -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 (
|
||||||
|
|||||||
@@ -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" />
|
||||||
|
|||||||
@@ -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
|
||||||
|
|||||||
@@ -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,10 +71,15 @@ function debouncedSave(
|
|||||||
set({ saving: true });
|
set({ saving: true });
|
||||||
try {
|
try {
|
||||||
await saveBoard(board);
|
await saveBoard(board);
|
||||||
|
// Only update state if the same board is still loaded
|
||||||
|
if (get().board?.id === board.id) {
|
||||||
set({ saving: false, lastSaved: Date.now() });
|
set({ saving: false, lastSaved: Date.now() });
|
||||||
|
}
|
||||||
} catch {
|
} catch {
|
||||||
|
if (get().board?.id === board.id) {
|
||||||
set({ saving: false });
|
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
|
||||||
),
|
),
|
||||||
|
|||||||
Reference in New Issue
Block a user