Merge pull request #1245 from RooVetGit/cte/delete-confirmation-enhancements

Delete task confirmation enhancements

.changeset/chilly-bugs-pay.md

@@ -0,0 +1,5 @@
+---
+"roo-cline": patch
+---
+
+Delete task confirmation enhancements

webview-ui/src/components/chat/TaskHeader.tsx

@@ -3,15 +3,18 @@ import { useWindowSize } from "react-use"
 import { VSCodeButton } from "@vscode/webview-ui-toolkit/react"
 import prettyBytes from "pretty-bytes"
 
+import { vscode } from "@/utils/vscode"
+import { formatLargeNumber } from "@/utils/format"
+import { Button } from "@/components/ui"
+
 import { ClineMessage } from "../../../../src/shared/ExtensionMessage"
+import { mentionRegexGlobal } from "../../../../src/shared/context-mentions"
+import { HistoryItem } from "../../../../src/shared/HistoryItem"
+
 import { useExtensionState } from "../../context/ExtensionStateContext"
-import { vscode } from "../../utils/vscode"
 import Thumbnails from "../common/Thumbnails"
-import { mentionRegexGlobal } from "../../../../src/shared/context-mentions"
-import { formatLargeNumber } from "../../utils/format"
 import { normalizeApiConfiguration } from "../settings/ApiOptions"
-import { Button } from "../ui"
-import { HistoryItem } from "../../../../src/shared/HistoryItem"
+import { DeleteTaskDialog } from "../history/DeleteTaskDialog"
 
 interface TaskHeaderProps {
 	task: ClineMessage
@@ -46,7 +49,21 @@ const TaskHeader: React.FC<TaskHeaderProps> = ({
 	const contextWindow = selectedModelInfo?.contextWindow || 1
 
 	/*
-	When dealing with event listeners in React components that depend on state variables, we face a challenge. We want our listener to always use the most up-to-date version of a callback function that relies on current state, but we don't want to constantly add and remove event listeners as that function updates. This scenario often arises with resize listeners or other window events. Simply adding the listener in a useEffect with an empty dependency array risks using stale state, while including the callback in the dependencies can lead to unnecessary re-registrations of the listener. There are react hook libraries that provide a elegant solution to this problem by utilizing the useRef hook to maintain a reference to the latest callback function without triggering re-renders or effect re-runs. This approach ensures that our event listener always has access to the most current state while minimizing performance overhead and potential memory leaks from multiple listener registrations. 
+	When dealing with event listeners in React components that depend on state
+	variables, we face a challenge. We want our listener to always use the most
+	up-to-date version of a callback function that relies on current state, but
+	we don't want to constantly add and remove event listeners as that function
+	updates. This scenario often arises with resize listeners or other window
+	events. Simply adding the listener in a useEffect with an empty dependency
+	array risks using stale state, while including the callback in the
+	dependencies can lead to unnecessary re-registrations of the listener. There
+	are react hook libraries that provide a elegant solution to this problem by
+	utilizing the useRef hook to maintain a reference to the latest callback
+	function without triggering re-renders or effect re-runs. This approach
+	ensures that our event listener always has access to the most current state
+	while minimizing performance overhead and potential memory leaks from
+	multiple listener registrations. 
+
 	Sources
 	- https://usehooks-ts.com/react-hook/use-event-listener
 	- https://streamich.github.io/react-use/?path=/story/sensors-useevent--docs
@@ -350,27 +367,48 @@ export const highlightMentions = (text?: string, withShadow = true) => {
 	})
 }
 
-const TaskActions = ({ item }: { item: HistoryItem | undefined }) => (
-	<div className="flex flex-row gap-1">
-		<Button
-			variant="ghost"
-			size="sm"
-			title="Export task history"
-			onClick={() => vscode.postMessage({ type: "exportCurrentTask" })}>
-			<span className="codicon codicon-cloud-download" />
-		</Button>
-		{!!item?.size && item.size > 0 && (
+const TaskActions = ({ item }: { item: HistoryItem | undefined }) => {
+	const [deleteTaskId, setDeleteTaskId] = useState<string | null>(null)
+
+	return (
+		<div className="flex flex-row gap-1">
 			<Button
 				variant="ghost"
 				size="sm"
-				title="Delete task from history"
-				onClick={() => vscode.postMessage({ type: "deleteTaskWithId", text: item.id })}>
-				<span className="codicon codicon-trash" />
-				{prettyBytes(item.size)}
+				title="Export task history"
+				onClick={() => vscode.postMessage({ type: "exportCurrentTask" })}>
+				<span className="codicon codicon-cloud-download" />
 			</Button>
-		)}
-	</div>
-)
+			{!!item?.size && item.size > 0 && (
+				<>
+					<Button
+						variant="ghost"
+						size="sm"
+						title="Delete Task (Shift + Click to skip confirmation)"
+						onClick={(e) => {
+							e.stopPropagation()
+
+							if (e.shiftKey) {
+								vscode.postMessage({ type: "deleteTaskWithId", text: item.id })
+							} else {
+								setDeleteTaskId(item.id)
+							}
+						}}>
+						<span className="codicon codicon-trash" />
+						{prettyBytes(item.size)}
+					</Button>
+					{deleteTaskId && (
+						<DeleteTaskDialog
+							taskId={deleteTaskId}
+							onOpenChange={(open) => !open && setDeleteTaskId(null)}
+							open
+						/>
+					)}
+				</>
+			)}
+		</div>
+	)
+}
 
 const ContextWindowProgress = ({ contextWindow, contextTokens }: { contextWindow: number; contextTokens: number }) => (
 	<>

webview-ui/src/components/history/DeleteTaskDialog.tsx

@@ -1,4 +1,7 @@
-import React from "react"
+import { useCallback, useEffect } from "react"
+import { useKeyPress } from "react-use"
+import { AlertDialogProps } from "@radix-ui/react-alert-dialog"
+
 import {
 	AlertDialog,
 	AlertDialogAction,
@@ -8,25 +11,36 @@ import {
 	AlertDialogFooter,
 	AlertDialogHeader,
 	AlertDialogTitle,
-} from "@/components/ui/alert-dialog"
-import { Button } from "@/components/ui"
+	Button,
+} from "@/components/ui"
+
 import { vscode } from "@/utils/vscode"
 
-interface DeleteTaskDialogProps {
+interface DeleteTaskDialogProps extends AlertDialogProps {
 	taskId: string
-	open: boolean
-	onOpenChange: (open: boolean) => void
 }
 
-export const DeleteTaskDialog = ({ taskId, open, onOpenChange }: DeleteTaskDialogProps) => {
-	const handleDelete = () => {
-		vscode.postMessage({ type: "deleteTaskWithId", text: taskId })
-		onOpenChange(false)
-	}
+export const DeleteTaskDialog = ({ taskId, ...props }: DeleteTaskDialogProps) => {
+	const [isEnterPressed] = useKeyPress("Enter")
+
+	const { onOpenChange } = props
+
+	const onDelete = useCallback(() => {
+		if (taskId) {
+			vscode.postMessage({ type: "deleteTaskWithId", text: taskId })
+			onOpenChange?.(false)
+		}
+	}, [taskId, onOpenChange])
+
+	useEffect(() => {
+		if (taskId && isEnterPressed) {
+			onDelete()
+		}
+	}, [taskId, isEnterPressed, onDelete])
 
 	return (
-		<AlertDialog open={open} onOpenChange={onOpenChange}>
-			<AlertDialogContent>
+		<AlertDialog {...props}>
+			<AlertDialogContent onEscapeKeyDown={() => onOpenChange?.(false)}>
 				<AlertDialogHeader>
 					<AlertDialogTitle>Delete Task</AlertDialogTitle>
 					<AlertDialogDescription>
@@ -38,7 +52,7 @@ export const DeleteTaskDialog = ({ taskId, open, onOpenChange }: DeleteTaskDialo
 						<Button variant="secondary">Cancel</Button>
 					</AlertDialogCancel>
 					<AlertDialogAction asChild>
-						<Button variant="destructive" onClick={handleDelete}>
+						<Button variant="destructive" onClick={onDelete}>
 							Delete
 						</Button>
 					</AlertDialogAction>

webview-ui/src/components/history/HistoryView.tsx

@@ -38,13 +38,7 @@ const HistoryView = ({ onDone }: HistoryViewProps) => {
 		vscode.postMessage({ type: "showTaskWithId", text: id })
 	}
 
-	const [deleteDialogOpen, setDeleteDialogOpen] = useState(false)
-	const [taskToDelete, setTaskToDelete] = useState<string | null>(null)
-
-	const handleDeleteHistoryItem = (id: string) => {
-		setTaskToDelete(id)
-		setDeleteDialogOpen(true)
-	}
+	const [deleteTaskId, setDeleteTaskId] = useState<string | null>(null)
 
 	const formatDate = (timestamp: number) => {
 		const date = new Date(timestamp)
@@ -230,10 +224,15 @@ const HistoryView = ({ onDone }: HistoryViewProps) => {
 										<Button
 											variant="ghost"
 											size="sm"
-											title="Delete Task"
+											title="Delete Task (Shift + Click to skip confirmation)"
 											onClick={(e) => {
 												e.stopPropagation()
-												handleDeleteHistoryItem(item.id)
+
+												if (e.shiftKey) {
+													vscode.postMessage({ type: "deleteTaskWithId", text: item.id })
+												} else {
+													setDeleteTaskId(item.id)
+												}
 											}}>
 											<span className="codicon codicon-trash" />
 											{item.size && prettyBytes(item.size)}
@@ -403,17 +402,8 @@ const HistoryView = ({ onDone }: HistoryViewProps) => {
 					)}
 				/>
 			</div>
-			{taskToDelete && (
-				<DeleteTaskDialog
-					taskId={taskToDelete}
-					open={deleteDialogOpen}
-					onOpenChange={(open) => {
-						setDeleteDialogOpen(open)
-						if (!open) {
-							setTaskToDelete(null)
-						}
-					}}
-				/>
+			{deleteTaskId && (
+				<DeleteTaskDialog taskId={deleteTaskId} onOpenChange={(open) => !open && setDeleteTaskId(null)} open />
 			)}
 		</div>
 	)

webview-ui/src/components/history/__tests__/HistoryView.test.tsx

@@ -135,26 +135,54 @@ describe("HistoryView", () => {
 		})
 	})
 
-	it("handles task deletion", async () => {
-		const onDone = jest.fn()
-		render(<HistoryView onDone={onDone} />)
+	describe("task deletion", () => {
+		it("shows confirmation dialog on regular click", () => {
+			const onDone = jest.fn()
+			render(<HistoryView onDone={onDone} />)
+
+			// Find and hover over first task
+			const taskContainer = screen.getByTestId("virtuoso-item-1")
+			fireEvent.mouseEnter(taskContainer)
+
+			// Click delete button to open confirmation dialog
+			const deleteButton = within(taskContainer).getByTitle("Delete Task (Shift + Click to skip confirmation)")
+			fireEvent.click(deleteButton)
+
+			// Verify dialog is shown
+			const dialog = screen.getByRole("alertdialog")
+			expect(dialog).toBeInTheDocument()
+
+			// Find and click the confirm delete button in the dialog
+			const confirmDeleteButton = within(dialog).getByRole("button", { name: /delete/i })
+			fireEvent.click(confirmDeleteButton)
+
+			// Verify vscode message was sent
+			expect(vscode.postMessage).toHaveBeenCalledWith({
+				type: "deleteTaskWithId",
+				text: "1",
+			})
+		})
 
-		// Find and hover over first task
-		const taskContainer = screen.getByTestId("virtuoso-item-1")
-		fireEvent.mouseEnter(taskContainer)
+		it("deletes immediately on shift-click without confirmation", () => {
+			const onDone = jest.fn()
+			render(<HistoryView onDone={onDone} />)
 
-		// Click delete button to open confirmation dialog
-		const deleteButton = within(taskContainer).getByTitle("Delete Task")
-		fireEvent.click(deleteButton)
+			// Find and hover over first task
+			const taskContainer = screen.getByTestId("virtuoso-item-1")
+			fireEvent.mouseEnter(taskContainer)
 
-		// Find and click the confirm delete button in the dialog
-		const confirmDeleteButton = screen.getByRole("button", { name: /delete/i })
-		fireEvent.click(confirmDeleteButton)
+			// Shift-click delete button
+			const deleteButton = within(taskContainer).getByTitle("Delete Task (Shift + Click to skip confirmation)")
+			fireEvent.click(deleteButton, { shiftKey: true })
 
-		// Verify vscode message was sent
-		expect(vscode.postMessage).toHaveBeenCalledWith({
-			type: "deleteTaskWithId",
-			text: "1",
+			// Verify no dialog is shown
+			expect(screen.queryByRole("alertdialog")).not.toBeInTheDocument()
+
+			// Verify vscode message was sent
+			expect(vscode.postMessage).toHaveBeenCalledWith({
+				type: "deleteTaskWithId",
+				text: "1",
+			})
 		})
 	})