From 24aca7ae722b235ea341ce5aba1e1502cb255e1e Mon Sep 17 00:00:00 2001 From: MaheshtheDev <38828053+MaheshtheDev@users.noreply.github.com> Date: Tue, 12 May 2026 00:44:21 +0000 Subject: [PATCH] =?UTF-8?q?fix:=20apply=20review=20feedback=20=E2=80=94=20?= =?UTF-8?q?fix=20URL=20submission,=20partial=20failures,=20retry/remove?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix note-tab URL save using plain text instead of markdown (prevents corrupted links) - Fix 'Save all' closing modal even with partial file failures - Add retry/remove buttons for failed file uploads in note tab - Extract CTA label logic into useMemo - Deduplicate close-confirmation logic via onRequestClose prop - Use normalizeUrl in link.tsx instead of inline protocol-prepend - Simplify LinkData interface (remove unused title/description/image) - Case-insensitive protocol check in detectContentType - Add explanatory comment on skipMutationCloseRef pattern --- apps/web/components/add-document/index.tsx | 149 ++++++++++++++------- apps/web/components/add-document/link.tsx | 17 +-- apps/web/components/add-document/note.tsx | 51 ++++--- 3 files changed, 137 insertions(+), 80 deletions(-) diff --git a/apps/web/components/add-document/index.tsx b/apps/web/components/add-document/index.tsx index 0d9ebe0c..1b745c38 100644 --- a/apps/web/components/add-document/index.tsx +++ b/apps/web/components/add-document/index.tsx @@ -1,6 +1,6 @@ "use client" -import { useState, useEffect, useCallback, useRef } from "react" +import { useState, useEffect, useCallback, useRef, useMemo } from "react" import { useQueryState } from "nuqs" import { Dialog, DialogContent, DialogTitle } from "@repo/ui/components/dialog" import { cn } from "@lib/utils" @@ -10,7 +10,12 @@ import { Button } from "@ui/components/button" import { ConnectContent } from "./connections" import { NoteContent } from "./note" import { LinkContent, type LinkData } from "./link" -import { FileContent, type FileData, type FileQueueItem, fileQueueKey } from "./file" +import { + FileContent, + type FileData, + type FileQueueItem, + fileQueueKey, +} from "./file" import { useProject } from "@/stores" import { toast } from "sonner" import { useDocumentMutations } from "../../hooks/use-document-mutations" @@ -31,17 +36,22 @@ export function AddDocumentModal({ isOpen, onClose }: AddDocumentModalProps) { const hasUnsavedContentRef = useRef<() => boolean>(() => false) const isSubmittingRef = useRef(false) - const handleCloseRequest = useCallback(() => { + const confirmThenClose = useCallback(() => { if (isSubmittingRef.current) return // Block close during submission if (hasUnsavedContentRef.current()) { - const confirmed = window.confirm("You have unsaved content. Are you sure you want to close?") + const confirmed = window.confirm( + "You have unsaved content. Are you sure you want to close?", + ) if (!confirmed) return } onClose() }, [onClose]) return ( - !open && handleCloseRequest()}> + !open && confirmThenClose()} + > void + onRequestClose?: () => void isOpen?: boolean hasUnsavedContentRef?: React.MutableRefObject<() => boolean> isSubmittingRef?: React.MutableRefObject @@ -125,13 +138,12 @@ export function AddDocument({ // Form data state for button click handling const [noteContent, setNoteContent] = useState("") + const [notePlainText, setNotePlainText] = useState("") const [noteContentType, setNoteContentType] = useState<"note" | "link">( "note", ) const [linkData, setLinkData] = useState({ url: "", - title: "", - description: "", }) const [fileData, setFileData] = useState({ items: [], @@ -143,6 +155,10 @@ export function AddDocument({ const [noteDroppedFiles, setNoteDroppedFiles] = useState([]) + // When submitting both text and files simultaneously, the first mutation to + // complete would trigger onClose and dismiss the modal before the second + // finishes. This ref gates the onClose callback so the combined submission + // in handleButtonClick can await both promises and close the modal itself. const skipMutationCloseRef = useRef(false) const { noteMutation, linkMutation, fileMutation } = useDocumentMutations({ @@ -160,6 +176,7 @@ export function AddDocument({ useEffect(() => { if (!isOpen) { setFileData({ items: [], title: "", description: "" }) + setNotePlainText("") setNoteContentType("note") setNoteDroppedFiles([]) } @@ -173,7 +190,9 @@ export function AddDocument({ return } if (contentType === "link") { - const normalizedUrl = normalizeUrl(content.trim()) + // Use plain text for URL — markdown serialization may wrap URLs + // in link syntax like [url](url) which would corrupt the value. + const normalizedUrl = normalizeUrl(notePlainText.trim()) linkMutation.mutate({ url: normalizedUrl, project: localSelectedProject, @@ -182,7 +201,7 @@ export function AddDocument({ noteMutation.mutate({ content, project: localSelectedProject }) } }, - [noteMutation, linkMutation, localSelectedProject], + [noteMutation, linkMutation, localSelectedProject, notePlainText], ) const handleLinkSubmit = useCallback( @@ -281,15 +300,23 @@ export function AddDocument({ setNoteDroppedFiles((prev) => prev.filter((f) => f.id !== id)) }, []) + const handleRetryNoteFile = useCallback((id: string) => { + setNoteDroppedFiles((prev) => + prev.map((f) => + f.id === id + ? { ...f, status: "pending" as const, errorMessage: undefined } + : f, + ), + ) + }, []) + const handleNoteFileSubmit = useCallback(async () => { const pending = noteDroppedFiles.filter((i) => i.status === "pending") if (pending.length === 0) return setNoteDroppedFiles((prev) => prev.map((i) => - i.status === "pending" - ? { ...i, status: "uploading" as const } - : i, + i.status === "pending" ? { ...i, status: "uploading" as const } : i, ), ) @@ -334,6 +361,10 @@ export function AddDocument({ setNoteContent(content) }, []) + const handleNotePlainTextChange = useCallback((text: string) => { + setNotePlainText(text) + }, []) + const handleLinkDataChange = useCallback((data: LinkData) => { setLinkData(data) }, []) @@ -354,10 +385,12 @@ export function AddDocument({ // Save all: text + files skipMutationCloseRef.current = true try { + // Use plain text for link URLs — markdown may wrap them in + // link syntax like [url](url) which would corrupt the value. const textPromise = noteContentType === "link" ? linkMutation.mutateAsync({ - url: normalizeUrl(noteContent.trim()), + url: normalizeUrl(notePlainText.trim()), project: localSelectedProject, }) : noteMutation.mutateAsync({ @@ -365,8 +398,13 @@ export function AddDocument({ project: localSelectedProject, }) const filePromise = handleNoteFileSubmit() - await Promise.all([textPromise, filePromise]) - onClose() + const [, fileResult] = await Promise.all([textPromise, filePromise]) + // Only close if all file uploads succeeded; partial failures + // should keep the modal open so users can retry or remove items. + const hasFileFailures = fileResult && fileResult.failures.length > 0 + if (!hasFileFailures) { + onClose() + } } catch { // At least one failed — modal stays open } finally { @@ -390,7 +428,23 @@ export function AddDocument({ void handleFileSubmit(fileData) break } - }, [activeTab, noteDroppedFiles, noteContent, noteContentType, linkMutation, noteMutation, localSelectedProject, handleNoteFileSubmit, onClose, handleNoteSubmit, handleLinkSubmit, linkData, handleFileSubmit, fileData]) + }, [ + activeTab, + noteDroppedFiles, + noteContent, + notePlainText, + noteContentType, + linkMutation, + noteMutation, + localSelectedProject, + handleNoteFileSubmit, + onClose, + handleNoteSubmit, + handleLinkSubmit, + linkData, + handleFileSubmit, + fileData, + ]) const handleNoteRequestSubmit = useCallback(() => { // Called by Cmd+Enter from the editor — uses same logic as CTA button @@ -420,14 +474,30 @@ export function AddDocument({ } }, [isSubmitting, isSubmittingRef]) - const handleClose = useCallback(() => { - if (isSubmitting) return - if (hasUnsavedContent()) { - const confirmed = window.confirm("You have unsaved content. Are you sure you want to close?") - if (!confirmed) return + // Delegate to the parent's confirmation-aware close when available (modal + // context), otherwise fall back to closing directly (standalone usage). + const handleClose = onRequestClose ?? onClose + + const ctaLabel = useMemo(() => { + if (activeTab === "note") { + const noteHasPendingFiles = noteDroppedFiles.some( + (i) => i.status === "pending", + ) + const noteHasText = noteContent.trim().length > 0 + if (noteHasText && noteHasPendingFiles) { + return "Save all" + } + if (noteHasPendingFiles) { + const pendingCount = noteDroppedFiles.filter( + (i) => i.status === "pending", + ).length + return pendingCount === 1 ? "Save file" : `Save ${pendingCount} files` + } + return noteContentType === "link" ? "Save link" : "Save note" } - onClose() - }, [isSubmitting, hasUnsavedContent, onClose]) + if (activeTab === "link") return "Save link" + return `+ Add ${activeTab}` + }, [activeTab, noteDroppedFiles, noteContent, noteContentType]) const fileTabHasPending = fileData.items.some((i) => i.status === "pending") const fileTabSubmitDisabled = @@ -500,12 +570,18 @@ export function AddDocument({ ({ id: f.id, name: f.file.name, @@ -589,30 +665,7 @@ export function AddDocument({ ) : ( <> - {(() => { - if (activeTab === "note") { - const noteHasPendingFiles = noteDroppedFiles.some( - (i) => i.status === "pending", - ) - const noteHasText = noteContent.trim().length > 0 - if (noteHasText && noteHasPendingFiles) { - return "Save all" - } - if (noteHasPendingFiles) { - const pendingCount = noteDroppedFiles.filter( - (i) => i.status === "pending", - ).length - return pendingCount === 1 - ? "Save file" - : `Save ${pendingCount} files` - } - return noteContentType === "link" - ? "Save link" - : "Save note" - } - if (activeTab === "link") return "Save link" - return `+ Add ${activeTab}` - })()}{" "} + {ctaLabel}{" "} {!isMobile && ( { if (canSubmit && onSubmit) { - let normalizedUrl = url.trim() - if ( - !normalizedUrl.startsWith("http://") && - !normalizedUrl.startsWith("https://") - ) { - normalizedUrl = `https://${normalizedUrl}` - } - onSubmit({ url: normalizedUrl, title: "", description: "" }) + onSubmit({ url: normalizeUrl(url.trim()) }) } } const handleUrlChange = (newUrl: string) => { setUrl(newUrl) - onDataChange?.({ url: newUrl, title: "", description: "" }) + onDataChange?.({ url: newUrl }) } useHotkeys("mod+enter", handleSubmit, { @@ -56,7 +47,7 @@ export function LinkContent({ useEffect(() => { if (!isOpen) { setUrl("") - onDataChange?.({ url: "", title: "", description: "" }) + onDataChange?.({ url: "" }) } }, [isOpen, onDataChange]) diff --git a/apps/web/components/add-document/note.tsx b/apps/web/components/add-document/note.tsx index 1d26d8a9..37e9a64e 100644 --- a/apps/web/components/add-document/note.tsx +++ b/apps/web/components/add-document/note.tsx @@ -18,13 +18,9 @@ function detectContentType(plainText: string): "note" | "link" { if (!trimmed) return "note" // Must be a single token (no whitespace) if (/\s/.test(trimmed)) return "note" - // Try as-is first (has protocol) - if ( - trimmed.startsWith("http://") || - trimmed.startsWith("https://") || - trimmed.startsWith("HTTP://") || - trimmed.startsWith("HTTPS://") - ) { + // Try as-is first (has protocol — case-insensitive check) + const lower = trimmed.toLowerCase() + if (lower.startsWith("http://") || lower.startsWith("https://")) { if (isValidUrl(trimmed)) return "link" return "note" } @@ -38,12 +34,14 @@ function detectContentType(plainText: string): "note" | "link" { interface NoteContentProps { onSubmit?: (content: string, contentType: "note" | "link") => void onContentChange?: (content: string) => void + onPlainTextChange?: (text: string) => void onContentTypeChange?: (type: "note" | "link") => void onRequestSubmit?: () => void isSubmitting?: boolean isOpen?: boolean onFilesDropped?: (files: File[]) => void onRemoveFile?: (id: string) => void + onRetryFile?: (id: string) => void droppedFiles?: { id: string name: string @@ -56,12 +54,14 @@ interface NoteContentProps { export function NoteContent({ onSubmit, onContentChange, + onPlainTextChange, onContentTypeChange, onRequestSubmit, isSubmitting, isOpen, onFilesDropped, onRemoveFile, + onRetryFile, droppedFiles, }: NoteContentProps) { const [content, setContent] = useState("") @@ -150,6 +150,7 @@ export function NoteContent({ onContentChange={handleContentChange} onPlainTextChange={(text) => { setPlainText(text) + onPlainTextChange?.(text) const type = detectContentType(text) onContentTypeChange?.(type) }} @@ -177,15 +178,16 @@ export function NoteContent({ {(file.size / 1024 / 1024).toFixed(1)}MB - {file.status === "pending" && onRemoveFile && ( - - )} + {(file.status === "pending" || file.status === "error") && + onRemoveFile && ( + + )} {file.status === "uploading" && ( )} @@ -193,9 +195,20 @@ export function NoteContent({ )} {file.status === "error" && ( - - - + <> + + + + {onRetryFile && ( + + )} + )} ))}