Feature request: Add delete confirmation dialog to file manager #344

Merged
thorved merged 3 commits from feature/file-manager-delete-confirmation into dev-1.8.0 2025-10-05 00:08:55 +00:00
2 changed files with 80 additions and 45 deletions
Showing only changes of commit d3532a24ee - Show all commits

View File

@@ -844,8 +844,12 @@
"selectServerToEdit": "Select a server from the sidebar to start editing files",
"fileOperations": "File Operations",
"confirmDeleteMessage": "Are you sure you want to delete <strong>{{name}}</strong>?",
"confirmDeleteSingleItem": "Are you sure you want to permanently delete \"{{name}}\"?",
"confirmDeleteMultipleItems": "Are you sure you want to permanently delete {{count}} items?",
"confirmDeleteFolder": "Are you sure you want to permanently delete the folder \"{{name}}\" and all its contents?",
"deleteDirectoryWarning": "This will delete the folder and all its contents.",
"actionCannotBeUndone": "This action cannot be undone.",
"permanentDeleteWarning": "This action cannot be undone. The item(s) will be permanently deleted from the server.",
"recent": "Recent",
"pinned": "Pinned",
"folderShortcuts": "Folder Shortcuts",

View File

@@ -9,6 +9,7 @@ import { FileWindow } from "./components/FileWindow";
import { DiffWindow } from "./components/DiffWindow";
import { useDragToDesktop } from "../../../hooks/useDragToDesktop";
import { useDragToSystemDesktop } from "../../../hooks/useDragToSystemDesktop";
import { useConfirmation } from "@/hooks/use-confirmation.ts";
import { Button } from "@/components/ui/button";
import { Input } from "@/components/ui/input";
import { toast } from "sonner";
@@ -82,6 +83,7 @@ function formatFileSize(bytes?: number): string {
function FileManagerContent({ initialHost, onClose }: FileManagerProps) {
const { openWindow } = useWindowManager();
const { t } = useTranslation();
const { confirmWithToast } = useConfirmation();
const [currentHost, setCurrentHost] = useState<SSHHost | null>(
initialHost || null,
@@ -587,54 +589,83 @@ function FileManagerContent({ initialHost, onClose }: FileManagerProps) {
async function handleDeleteFiles(files: FileItem[]) {
gemini-code-assist[bot] commented 2025-10-04 09:47:59 +00:00 (Migrated from github.com)
Review

medium

When multiple items are selected for deletion, the confirmation message is generic (Are you sure you want to permanently delete N items?). If the selection includes one or more folders, this message doesn't warn the user that deleting a folder also deletes all of its contents. This could lead to accidental data loss, which this PR aims to prevent.

To improve this, you could check if any of the selected items are directories and show a more specific warning. This would likely require a new translation key.

For example, you could adjust the logic like this:

    } else {
      const hasDirectory = files.some((file) => file.type === "directory");
      const translationKey = hasDirectory
        ? "fileManager.confirmDeleteMultipleItemsWithFolders" // New key needed
        : "fileManager.confirmDeleteMultipleItems";

      confirmMessage = t(translationKey, {
        count: files.length,
      });
    }

And the new translation key in translation.json could be:
"confirmDeleteMultipleItemsWithFolders": "Are you sure you want to permanently delete {{count}} items? This includes folders and their contents."

![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg) When multiple items are selected for deletion, the confirmation message is generic (`Are you sure you want to permanently delete N items?`). If the selection includes one or more folders, this message doesn't warn the user that deleting a folder also deletes all of its contents. This could lead to accidental data loss, which this PR aims to prevent. To improve this, you could check if any of the selected items are directories and show a more specific warning. This would likely require a new translation key. For example, you could adjust the logic like this: ```typescript } else { const hasDirectory = files.some((file) => file.type === "directory"); const translationKey = hasDirectory ? "fileManager.confirmDeleteMultipleItemsWithFolders" // New key needed : "fileManager.confirmDeleteMultipleItems"; confirmMessage = t(translationKey, { count: files.length, }); } ``` And the new translation key in `translation.json` could be: `"confirmDeleteMultipleItemsWithFolders": "Are you sure you want to permanently delete {{count}} items? This includes folders and their contents."`
thorved commented 2025-10-04 10:00:26 +00:00 (Migrated from github.com)
Review
image
<img width="1892" height="918" alt="image" src="https://github.com/user-attachments/assets/7934842d-fb9c-4ad0-8c87-702d0e6097a8" />
if (!sshSessionId || files.length === 0) return;
try {
await ensureSSHConnection();
for (const file of files) {
await deleteSSHItem(
sshSessionId,
file.path,
file.type === "directory",
currentHost?.id,
currentHost?.userId?.toString(),
);
}
const deletedFiles = files.map((file) => ({
path: file.path,
name: file.name,
}));
const undoAction: UndoAction = {
type: "delete",
description: t("fileManager.deletedItems", { count: files.length }),
data: {
operation: "cut",
deletedFiles,
targetDirectory: currentPath,
},
timestamp: Date.now(),
};
setUndoHistory((prev) => [...prev.slice(-9), undoAction]);
toast.success(
t("fileManager.itemsDeletedSuccessfully", { count: files.length }),
);
handleRefreshDirectory();
clearSelection();
} catch (error: any) {
if (
error.message?.includes("connection") ||
error.message?.includes("established")
) {
toast.error(
`SSH connection failed. Please check your connection to ${currentHost?.name} (${currentHost?.ip}:${currentHost?.port})`,
);
// Determine the confirmation message based on file count and type
let confirmMessage: string;
if (files.length === 1) {
const file = files[0];
if (file.type === "directory") {
confirmMessage = t("fileManager.confirmDeleteFolder", {
name: file.name,
});
} else {
toast.error(t("fileManager.failedToDeleteItems"));
confirmMessage = t("fileManager.confirmDeleteSingleItem", {
name: file.name,
});
}
console.error("Delete failed:", error);
} else {
confirmMessage = t("fileManager.confirmDeleteMultipleItems", {
count: files.length,
});
}
// Add permanent deletion warning
const fullMessage = `${confirmMessage}\n\n${t("fileManager.permanentDeleteWarning")}`;
// Show confirmation dialog
confirmWithToast(
fullMessage,
async () => {
try {
await ensureSSHConnection();
for (const file of files) {
await deleteSSHItem(
sshSessionId,
file.path,
file.type === "directory",
currentHost?.id,
currentHost?.userId?.toString(),
);
}
const deletedFiles = files.map((file) => ({
path: file.path,
name: file.name,
}));
const undoAction: UndoAction = {
type: "delete",
description: t("fileManager.deletedItems", { count: files.length }),
data: {
operation: "cut",
deletedFiles,
targetDirectory: currentPath,
},
timestamp: Date.now(),
};
setUndoHistory((prev) => [...prev.slice(-9), undoAction]);
toast.success(
t("fileManager.itemsDeletedSuccessfully", { count: files.length }),
);
handleRefreshDirectory();
clearSelection();
} catch (error: any) {
if (
error.message?.includes("connection") ||
error.message?.includes("established")
) {
toast.error(
`SSH connection failed. Please check your connection to ${currentHost?.name} (${currentHost?.ip}:${currentHost?.port})`,
);
} else {
toast.error(t("fileManager.failedToDeleteItems"));
}
console.error("Delete failed:", error);
}
},
"destructive",
);
}
function handleCreateNewFolder() {