From a4c0e04ab9025e70fe29a8a56ccb5de2bdf370e5 Mon Sep 17 00:00:00 2001 From: John Coffey Date: Tue, 1 Sep 2026 20:51:51 -0700 Subject: [PATCH] Edit a text file where you are already reading it v2 of the viewer: Edit, on text and Markdown, in the dialog and on the row menu. Save is explicit -- every save mints a new blob, so autosave would burn quota and multiply the conflicts it cannot see. Two people editing one file is the case worth getting right. `saveText` re-reads the node and compares the blob the editor started from: if somebody else saved in the meantime it refuses, says so, and leaves the work in the box to copy out. `ifInState` is the obvious tool and the wrong one -- it is the state of every FileNode in the account, so an unrelated upload in another folder would fail the save, and a warning that cries wolf is a warning people click through. Editing is not offered where saving would lose something: a file truncated for display would have its tail written away, and one that did not decode as UTF-8 would have mojibake written over whatever encoding it really is. Both open read-only and say which. Nor is it offered without mayModifyContent -- a read-only share just has no Edit. Closing or cancelling with unsaved changes asks first, Ctrl+S saves, and mail attachments are unaffected: they pass no onSave, because a message part is not a thing that can be written back. --- web/src/store/files.ts | 36 ++++ web/src/styles/app.css | 2 + web/src/ui/filepreview.tsx | 291 +++++++++++++++++++++++++----- web/src/views/files/FilesView.tsx | 45 ++++- 4 files changed, 320 insertions(+), 54 deletions(-) diff --git a/web/src/store/files.ts b/web/src/store/files.ts index b20a563..8f22c29 100644 --- a/web/src/store/files.ts +++ b/web/src/store/files.ts @@ -5,6 +5,7 @@ import { foldersNeeded, type PlannedUpload } from "@/lib/dropUpload"; import { isAppFolder } from "@/lib/appFolder"; import type { FileNode, GetResponse, Id, QueryResponse, SetResponse } from "@/jmap/types"; import { useSession } from "./session"; +import { t as translate } from "@/lib/i18n"; interface SharedAccount { id: Id; @@ -56,6 +57,12 @@ interface FilesState { mkdir(parentId: Id | null, name: string): Promise; upload(parentId: Id | null, files: File[]): Promise; rename(id: Id, name: string): Promise; + /** + * Write text back over a file. `seenBlobId` is what the editor started from: + * if the node has moved on since, somebody else saved and this throws rather + * than quietly winning. + */ + saveText(id: Id, text: string, seenBlobId: Id | null): Promise; move(id: Id, parentId: Id | null): Promise; destroy(ids: Id[]): Promise; refresh(ids: Id[]): Promise; @@ -307,6 +314,35 @@ export const useFiles = create((set, get) => ({ void get().loadTree(); }, + async saveText(id, text, seenBlobId) { + const accountId = get().accountId!; + /* + * Look before writing. + * + * `ifInState` is the obvious tool and the wrong one here: it is the state + * of every FileNode in the account, so an unrelated upload in another + * folder would fail this save, and a reader who is told "someone changed + * it" when nobody did learns to click through the warning. The node's own + * blobId is the thing that actually answers the question. + */ + const fresh = await client.call>("FileNode/get", { accountId, ids: [id], properties: fileNodeProps() }); + const now = fresh.list[0]; + if (!now) throw new Error(translate("That file is no longer there.")); + if (now.blobId !== seenBlobId) throw new Error(translate("Somebody else saved this file while it was open. Copy your changes, close it, and start again.")); + + const type = now.type || "text/plain"; + const blob = new Blob([text], { type }); + const up = await client.upload(accountId, blob, { type }); + const res = await client.call>("FileNode/set", { + accountId, + update: { [id]: { blobId: up.blobId, type, size: blob.size } }, + }); + const err = res.notUpdated?.[id]; + if (err) throw new Error(setErrorMessage(err)); + await get().refresh([id]); + return up.blobId; + }, + async rename(id, name) { const accountId = get().accountId!; const res = await client.call("FileNode/set", { accountId, update: { [id]: { name } } }); diff --git a/web/src/styles/app.css b/web/src/styles/app.css index bf9a589..95e0358 100644 --- a/web/src/styles/app.css +++ b/web/src/styles/app.css @@ -320,6 +320,8 @@ a.menu-item:hover { color: var(--fg); } .segmented button.active { background: var(--accent-soft); color: var(--accent-soft-fg); font-weight: 600; } .segmented button:focus-visible { outline: 2px solid var(--accent); outline-offset: -2px; } .segmented.left { margin-right: auto; } +.dialog-foot .hint.left { margin-right: auto; } +.dialog-foot textarea.code, .dialog-body > textarea.code { font-family: var(--font-mono); } /* Rendered Markdown in the file viewer. Deliberately plain: this is somebody's notes, not a web page, and the point is to read it. */ diff --git a/web/src/ui/filepreview.tsx b/web/src/ui/filepreview.tsx index 553d25b..9256891 100644 --- a/web/src/ui/filepreview.tsx +++ b/web/src/ui/filepreview.tsx @@ -1,6 +1,6 @@ -import { useEffect, useMemo, useRef, useState, type ReactNode } from "react"; -import { Code2, Download, Eye, Printer } from "lucide-react"; -import { Dialog } from "./dialog"; +import { useCallback, useEffect, useMemo, useRef, useState, type ReactNode } from "react"; +import { Code2, Download, Eye, Pencil, Printer, Save, X } from "lucide-react"; +import { confirmDialog, Dialog } from "./dialog"; import { formatSize } from "@/lib/format"; import { previewKind, TEXT_PREVIEW_CHARS, TEXT_PREVIEW_MAX } from "@/lib/preview"; import { isMarkdown, renderMarkdown } from "@/lib/markdown"; @@ -21,21 +21,128 @@ export interface PreviewFile { inlineUrl: string; } +type Mode = "rendered" | "source" | "edit"; + /** * Shows a file without downloading it: pictures, PDFs, and anything text. * - * Grown out of the attachment preview in MessageView, which is where it still - * has one of its two callers -- the other is Files, which until now could only - * hand you the bytes. + * Grown out of the attachment preview in MessageView, which is still one of its + * two callers -- the other is Files, which could only hand you the bytes. + * + * `onSave` is what makes it an editor. Files passes one; mail does not, because + * a message part is not a thing that can be written back. */ -export function FilePreviewDialog({ file, onClose, caption }: { file: PreviewFile | null; onClose: () => void; caption?: ReactNode }) { +export function FilePreviewDialog({ + file, + onClose, + caption, + onSave, + startInEdit, +}: { + file: PreviewFile | null; + onClose: () => void; + caption?: ReactNode; + /** Write the text back. Rejecting with a message is how a conflict is reported. */ + onSave?: (text: string) => Promise; + /** Open straight into the editor -- what the row menu's Edit asks for. */ + startInEdit?: boolean; +}) { const kind = file ? previewKind(file.type, file.name) : null; const tooBig = kind === "text" && typeof file?.size === "number" && file.size > TEXT_PREVIEW_MAX; - const pdfRef = useRef(null); const markdown = Boolean(file) && kind === "text" && isMarkdown(file!.type, file!.name); - /* Markdown opens as the document it is meant to be; the source is a click - away for anyone who wants to see what it actually says. */ - const [rendered, setRendered] = useState(true); + const pdfRef = useRef(null); + + const loaded = useTextFile(kind === "text" && !tooBig ? file?.url ?? null : null); + const [mode, setMode] = useState("source"); + const [draft, setDraft] = useState(""); + const [saving, setSaving] = useState(false); + const [saveError, setSaveError] = useState(null); + const [pendingEdit, setPendingEdit] = useState(false); + + /* A new file starts over: Markdown as the document it is, everything else as + what it says, and never in the editor. */ + useEffect(() => { + setMode(markdown ? "rendered" : "source"); + setDraft(""); + setSaveError(null); + setPendingEdit(Boolean(startInEdit)); + }, [file?.url, markdown, startInEdit]); + + /* + * Three reasons not to offer editing, and each of them would lose data: + * + * - the file was truncated for display, so saving would write the tail away; + * - it did not decode as UTF-8 (the replacement character is the giveaway), + * so saving would write mojibake over whatever encoding it really is; + * - the caller has no way to save it, or the reader has no right to. + */ + const lossy = loaded.text?.includes("�") ?? false; + const editable = Boolean(onSave) && kind === "text" && !tooBig && !loaded.truncated && !lossy && loaded.text !== null && !loaded.failed; + const editing = mode === "edit"; + const dirty = editing && draft !== (loaded.text ?? ""); + + /* Opening straight into the editor has to wait for the text to arrive, and + may still land in the read-only view: whether a file can be edited is not + known until it has been read (truncated? not UTF-8?), and the row menu + could only guess from its name. The note under the pane says why. */ + useEffect(() => { + if (!pendingEdit || loaded.text === null) return; + setPendingEdit(false); + if (!editable) return; + setDraft(loaded.text); + setMode("edit"); + }, [pendingEdit, editable, loaded.text]); + + const startEditing = () => { + setDraft(loaded.text ?? ""); + setSaveError(null); + setMode("edit"); + }; + + const stopEditing = async () => { + if (dirty && !(await confirmDialog({ title: t("Throw away your changes?"), confirmLabel: t("Discard"), danger: true }))) return; + setMode(markdown ? "rendered" : "source"); + setSaveError(null); + }; + + const save = useCallback(async () => { + if (!onSave || saving) return; + setSaving(true); + setSaveError(null); + try { + await onSave(draft); + loaded.replace(draft); + setMode(markdown ? "rendered" : "source"); + } catch (err) { + setSaveError((err as Error).message); + } finally { + setSaving(false); + } + }, [onSave, saving, draft, loaded, markdown]); + + /* The shortcut everyone's hands already know. Only while the editor is open, + so it does not shadow the browser's own Save anywhere else. */ + useEffect(() => { + if (!editing) return; + const onKey = (ev: KeyboardEvent) => { + if ((ev.ctrlKey || ev.metaKey) && ev.key.toLowerCase() === "s") { + ev.preventDefault(); + void save(); + } + }; + window.addEventListener("keydown", onKey); + return () => window.removeEventListener("keydown", onKey); + }, [editing, save]); + + /* Closing with unsaved work asks first -- Escape and the backdrop both come + through here. */ + const requestClose = () => { + if (!dirty) { + onClose(); + return; + } + void confirmDialog({ title: t("Close without saving?"), confirmLabel: t("Discard"), danger: true }).then((yes) => yes && onClose()); + }; /* * Print what is on screen, not the mail or the file list behind it. @@ -76,27 +183,43 @@ export function FilePreviewDialog({ file, onClose, caption }: { file: PreviewFil clear(); } }; + return ( - {markdown && !tooBig && ( -
- - -
- )} - {kind && !tooBig && } - {t("Download")} - - )} + closeOnBackdrop={!editing} + footer={ + file && ( + <> + {editing ? ( + <> + {dirty && {t("Unsaved changes")}} + + + + ) : ( + <> + {markdown && !tooBig && ( +
+ + +
+ )} + {editable && } + {kind && !tooBig && } + {t("Download")} + + )} + + ) + } > {file && ( <> + {saveError &&
{saveError}
} {tooBig ? (

{t("This file is too big to show here ({size}) — download it to read it.", { size: formatSize(file.size ?? 0) })}

) : kind === "image" ? ( @@ -104,10 +227,14 @@ export function FilePreviewDialog({ file, onClose, caption }: { file: PreviewFil ) : kind === "pdf" ? (