From f0b3a2fc17323d82237542d2d534d61cab012e69 Mon Sep 17 00:00:00 2001 From: TimRl Date: Fri, 5 Jun 2026 12:12:30 -0600 Subject: [PATCH 1/2] Adding a delete confirmation for urls and video files --- .../CodexCellEditor/NotebookMetadataModal.tsx | 220 +++++++++++++----- 1 file changed, 168 insertions(+), 52 deletions(-) diff --git a/webviews/codex-webviews/src/CodexCellEditor/NotebookMetadataModal.tsx b/webviews/codex-webviews/src/CodexCellEditor/NotebookMetadataModal.tsx index 79c313e75..d9dfdedaa 100644 --- a/webviews/codex-webviews/src/CodexCellEditor/NotebookMetadataModal.tsx +++ b/webviews/codex-webviews/src/CodexCellEditor/NotebookMetadataModal.tsx @@ -14,6 +14,7 @@ import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from ". import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "../components/ui/tooltip"; import { Separator } from "../components/ui/separator"; import { Badge } from "../components/ui/badge"; +import { Alert, AlertDescription, AlertTitle } from "../components/ui/alert"; import { VSCodeButton } from "@vscode/webview-ui-toolkit/react"; import { formatBytes } from "../lib/utils"; @@ -79,18 +80,24 @@ const NotebookMetadataModal: React.FC = ({ // removal — is persisted unless the user explicitly saves. const [draft, setDraft] = useState(metadata); const [hasChanges, setHasChanges] = useState(false); + // Safety gate for removing a video: the user must type/paste the exact file + // name (local) or URL (remote) before the removal is allowed. `null` means + // the confirmation panel is closed. + const [removalConfirmText, setRemovalConfirmText] = useState(null); // Start the draft from the latest saved metadata each time the modal opens. useEffect(() => { if (isOpen) { setDraft(metadata); setHasChanges(false); + setRemovalConfirmText(null); } // eslint-disable-next-line react-hooks/exhaustive-deps }, [isOpen]); // Picking a file is an immediate host action that updates the saved videoUrl; - // mirror it into the draft so the field reflects the newly picked file. + // mirror it into the draft so the field reflects the newly picked file. Any + // in-progress removal confirmation no longer applies to the new reference. useEffect(() => { if (!isOpen) { return; @@ -98,6 +105,7 @@ const NotebookMetadataModal: React.FC = ({ setDraft((d) => d.videoUrl === metadata.videoUrl ? d : { ...d, videoUrl: metadata.videoUrl } ); + setRemovalConfirmText(null); // eslint-disable-next-line react-hooks/exhaustive-deps }, [metadata.videoUrl]); @@ -106,12 +114,22 @@ const NotebookMetadataModal: React.FC = ({ setDraft((d) => ({ ...d, [key]: value }) as CustomNotebookMetadata); }; - // Clear/remove is deferred: it only empties the draft field. The actual file - // deletion + JSON change happens on Save (the host removes the old local file - // when the saved videoUrl changes). + // Step 1 of removal: open the type-to-confirm panel. Nothing is changed yet. const handleClearVideo = () => { + setRemovalConfirmText(""); + }; + + const cancelClearVideo = () => { + setRemovalConfirmText(null); + }; + + // Step 2 of removal (only reachable once the typed text matches): empty the + // draft field. The actual file deletion + JSON change is still deferred to + // Save (the host removes the old local file when the saved videoUrl changes). + const confirmClearVideo = () => { setHasChanges(true); setDraft((d) => ({ ...d, videoUrl: "" }) as CustomNotebookMetadata); + setRemovalConfirmText(null); }; const handleSave = () => { @@ -122,6 +140,7 @@ const NotebookMetadataModal: React.FC = ({ const handleClose = () => { setDraft(metadata); setHasChanges(false); + setRemovalConfirmText(null); onClose(); }; @@ -186,64 +205,161 @@ const NotebookMetadataModal: React.FC = ({ ? formatBytes(videoSizeBytes) : ""; + // The token the user must type/paste to confirm removal: + // the file name for a local video, or the full URL for a + // remote one. Matching is exact after trimming surrounding + // whitespace (so a pasted value with stray spaces still + // works) — a deliberate "are you sure" gate. + const expectedToken = isLocalFile ? fileName : videoValue; + const isConfirmingRemoval = removalConfirmText !== null; + const trimmedExpected = expectedToken.trim(); + const removalMatches = + isConfirmingRemoval && + trimmedExpected.length > 0 && + (removalConfirmText ?? "").trim() === trimmedExpected; + // When a video is set, lock the field. Changing it requires an // explicit Clear first (deferred — the file is only deleted and // the JSON updated on Save). Once cleared, the field becomes // editable for the next URL or picked file. if (hasVideo) { return ( -
-
- - {displayLabel} - {sizeLabel && !isMissing && ( - - {sizeLabel} - +
+
+
+ + {displayLabel} + {sizeLabel && !isMissing && ( + + {sizeLabel} + + )} + {isMissing && ( + + File missing + + )} +
+ {showFreeSpace && !isConfirmingRemoval && ( + )} - {isMissing && ( - - File missing - + {!isConfirmingRemoval && ( + )}
- {showFreeSpace && ( - + + {isConfirmingRemoval && ( + + + Remove this video? + +

+ {isLocalFile + ? "This removes the video reference and permanently deletes the local file when you save your changes." + : "This removes the video URL from this chapter when you save your changes."} +

+
+ + + {expectedToken} + + + setRemovalConfirmText(e.target.value) + } + onKeyDown={(e) => { + if (e.key === "Escape") { + // Cancel only the confirmation, + // don't let Radix close the modal. + e.preventDefault(); + e.stopPropagation(); + cancelClearVideo(); + } else if (e.key === "Enter") { + e.preventDefault(); + if (removalMatches) { + confirmClearVideo(); + } + } + }} + placeholder={ + isLocalFile + ? "Type the file name to confirm" + : "Type the URL to confirm" + } + className="bg-background text-foreground" + /> + {(removalConfirmText ?? "").length > 0 && + !removalMatches && ( +

+ The text doesn't match yet. +

+ )} +
+
+ + +
+ + )} -
); } From 07e0bc6f11ea4952970bc30505b8776ef7c7b02e Mon Sep 17 00:00:00 2001 From: TimRl Date: Fri, 5 Jun 2026 12:20:26 -0600 Subject: [PATCH 2/2] Preventing redundant video confirmations when adding a new video after having removed it (whether url or video) --- .../codexCellEditorMessagehandling.ts | 16 ++++++++++++---- types/index.d.ts | 6 +++--- .../src/CodexCellEditor/CodexCellEditor.tsx | 12 +++++++++++- 3 files changed, 26 insertions(+), 8 deletions(-) diff --git a/src/providers/codexCellEditorProvider/codexCellEditorMessagehandling.ts b/src/providers/codexCellEditorProvider/codexCellEditorMessagehandling.ts index 0935e006a..53270e559 100644 --- a/src/providers/codexCellEditorProvider/codexCellEditorMessagehandling.ts +++ b/src/providers/codexCellEditorProvider/codexCellEditorMessagehandling.ts @@ -1783,7 +1783,12 @@ const messageHandlers: Record Promise Promise { + pickVideoFile: async ({ event, document, webviewPanel, provider }) => { debug("pickVideoFile message received"); + const typedEvent = event as Extract; // If a local video already exists, confirm replacement up front so the // user can back out before choosing a new file (deletion happens after - // the new file is written successfully). + // the new file is written successfully). The metadata modal only reveals + // the picker after its own type-to-confirm removal step, so it sets + // skipVideoConfirm to avoid a redundant prompt. const existingVideoUrl = document.getNotebookMetadata()?.videoUrl; const existingKind = classifyVideo(existingVideoUrl); - if (existingKind === "local") { + if (existingKind === "local" && !typedEvent.skipVideoConfirm) { const proceed = await confirmVideoReplacement("local", "local"); if (!proceed) { return; diff --git a/types/index.d.ts b/types/index.d.ts index 7ad6ffc0c..76bd0fd67 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -414,8 +414,8 @@ export type EditorPostMessages = | { command: "updateCellLabel"; content: { cellId: string; cellLabel: string; }; } | { command: "updateCellIsLocked"; content: { cellId: string; isLocked: boolean; }; } | { command: "resolveHtmlStructure"; content: { cellId: string; }; } - | { command: "updateNotebookMetadata"; content: CustomNotebookMetadata; } - | { command: "pickVideoFile"; } + | { command: "updateNotebookMetadata"; content: CustomNotebookMetadata; skipVideoConfirm?: boolean; } + | { command: "pickVideoFile"; skipVideoConfirm?: boolean; } | { command: "deleteVideoFile"; } | { command: "freeVideoDiskSpace"; } | { command: "requestVideoStreamUrl"; } @@ -470,7 +470,7 @@ export type EditorPostMessages = | { command: "updateTextDirection"; direction: "ltr" | "rtl"; } | { command: "openSourceText"; content: { chapterNumber: number; }; } | { command: "updateCellLabel"; content: { cellId: string; cellLabel: string; }; } - | { command: "pickVideoFile"; } + | { command: "pickVideoFile"; skipVideoConfirm?: boolean; } | { command: "exportFile"; content: { subtitleData: string; format: string; includeStyles: boolean; }; diff --git a/webviews/codex-webviews/src/CodexCellEditor/CodexCellEditor.tsx b/webviews/codex-webviews/src/CodexCellEditor/CodexCellEditor.tsx index aa1df905e..af6e5a21c 100755 --- a/webviews/codex-webviews/src/CodexCellEditor/CodexCellEditor.tsx +++ b/webviews/codex-webviews/src/CodexCellEditor/CodexCellEditor.tsx @@ -3061,7 +3061,13 @@ const CodexCellEditor: React.FC = () => { }; const handlePickFile = () => { - vscode.postMessage({ command: "pickVideoFile" } as EditorPostMessages); + // The metadata modal only exposes the file picker once the video field is + // empty, which requires passing its type-to-confirm removal step first. + // So the host's own replace/delete confirmation would be redundant here. + vscode.postMessage({ + command: "pickVideoFile", + skipVideoConfirm: true, + } as EditorPostMessages); }; // Stream-and-save: revert the downloaded local copy back to an LFS pointer to @@ -3079,9 +3085,13 @@ const CodexCellEditor: React.FC = () => { setMetadata(updatedMetadata); setVideoUrl(updatedMetadata.videoUrl || ""); debug("metadata", "Saving metadata:", updatedMetadata); + // Any video change in the modal already passed its robust type-to-confirm + // removal step, so the host's own replace/delete confirmation would be a + // redundant second prompt — skip it. The host still deletes the old file. vscode.postMessage({ command: "updateNotebookMetadata", content: updatedMetadata, + skipVideoConfirm: true, } as EditorPostMessages); setIsMetadataModalOpen(false); };