From ebbae6794c65a50264ef16d48d709572603f3d2e Mon Sep 17 00:00:00 2001 From: npeter83 Date: Tue, 13 Oct 2026 03:15:59 +0200 Subject: [PATCH] =?UTF-8?q?fix(files):=20review=20round=203,=20the=20page?= =?UTF-8?q?=20=E2=80=94=20a=20search=20waits=20for=20the=20server,=20the?= =?UTF-8?q?=20viewer=20keeps=20its=20file,=20a=20drop=20ends=20the=20drag?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - past the list's cap a search waits in a skeleton, never showing the newest files' 'no match' for a file the server is about to find; a sort keeps its stand-in rows - the viewer keeps the file it was opened on when a server answer replaces the rows under it - a drop another zone took still clears the page's drop overlay - 'The first N items' only for a server sort; the past-the-end notice no longer claims the file exists - a failed server search is tested to say so --- .../src/components/Files.menu.dom.test.tsx | 73 +++++++++++++++++++ frontend/src/components/Files.tsx | 56 +++++++++++--- frontend/src/i18n/locales/en/files.json | 2 +- frontend/src/i18n/locales/hu/files.json | 2 +- 4 files changed, 119 insertions(+), 14 deletions(-) diff --git a/frontend/src/components/Files.menu.dom.test.tsx b/frontend/src/components/Files.menu.dom.test.tsx index eb96f832..710ae732 100644 --- a/frontend/src/components/Files.menu.dom.test.tsx +++ b/frontend/src/components/Files.menu.dom.test.tsx @@ -38,6 +38,8 @@ const mocks = vi.hoisted(() => ({ searched: [] as unknown[], // The server's search/sort answer is still on its way (no data yet). wideLoading: false, + // The server's search/sort answer failed. + wideFailed: false, foldersFailed: false, notify: vi.fn(), })); @@ -73,6 +75,14 @@ vi.mock("../lib/useLiveQuery", () => ({ if (opts?.enabled === false) return { data: undefined, isError: false, isLoading: false }; mocks.keys.push(key); const asked = key[1] as { q?: string; sort?: string } | undefined; + if (key[0] === "files" && (asked?.q || asked?.sort) && mocks.wideFailed) + return { + data: undefined, + isError: true, + isLoading: false, + isFetching: false, + refetch: vi.fn(), + }; if (key[0] === "files" && (asked?.q || asked?.sort) && mocks.wideLoading) return { data: undefined, @@ -154,6 +164,7 @@ afterEach(() => { mocks.fetching = false; mocks.titleArgs = []; mocks.wideLoading = false; + mocks.wideFailed = false; mocks.notify.mockClear(); resetQueueWatch(); }); @@ -476,11 +487,55 @@ describe("Files · a library past the list's cap", () => { expect(screen.getByText("files.countFirst")).toBeTruthy(); cleanup(); + // A search with no sort of the server's: it answers the newest matches first. + mount(ALL_FILES, vi.fn(), "clip"); + expect(screen.getByText("files.countNewest")).toBeTruthy(); + cleanup(); + mocks.searched = [file(9999, { display_name: "Régi recept" })]; mount(ALL_FILES, vi.fn(), "recept"); expect(screen.getByText("files.count")).toBeTruthy(); }); + it("a search past the cap waits for the server's answer — never a 'no match' it has not given", () => { + mocks.folders = [YT]; + mocks.files = capped(); + mocks.wideLoading = true; + mount(ALL_FILES, vi.fn(), "recept"); + expect(screen.getByRole("status")).toBeTruthy(); + expect(screen.queryByText("files.empty.search")).toBeNull(); + }); + + it("a server search that failed says so, not the newest files searched here", () => { + mocks.folders = [YT]; + mocks.files = capped(); + mocks.wideFailed = true; + mount(ALL_FILES, vi.fn(), "clip"); + expect(screen.getByText("common.loadError")).toBeTruthy(); + expect(screen.queryByText("Clip 1")).toBeNull(); + }); + + it("a file opened while the server's sort was on its way stays open when the answer leaves it out", async () => { + mocks.folders = [YT]; + const doc = file(7, { + job_kind: "upload", + display_name: "Doc 7", + asset: { media_kind: "file", preview: "pdf", container: "pdf", size_bytes: 10 }, + } as never); + mocks.files = [doc, ...capped().slice(1)]; + mocks.wideLoading = true; + mount(); + fireEvent.click(nameHeader()); + fireEvent.click(screen.getByText("Doc 7")); + expect(await screen.findByRole("dialog")).toBeTruthy(); + // The answer: the first files by name, without it. + mocks.wideLoading = false; + mocks.searched = Array.from({ length: FILES_LIST_CAP }, (_, i) => file(i + 1000)); + fireEvent.click(nameHeader()); + expect(screen.getByText("Clip 1499")).toBeTruthy(); // by name, from the end + expect(screen.getByRole("dialog")).toBeTruthy(); + }); + it("an arrival waits for the server's answer: the table can only turn to a file it holds", async () => { mocks.folders = [YT]; mocks.files = capped(); @@ -1013,6 +1068,24 @@ describe("Files · dropping a file on the page", () => { expect(drag("drop", [new File(["y"], "masik.pdf")]).defaultPrevented).toBe(true); }); + it("a drop another zone took still ends the page's drag", () => { + mocks.files = []; + mocks.folders = [YT, OWN]; + mount({ kind: "folder", id: 2 }); + drag("dragenter"); + expect(screen.getByText("files.drop.here:Cimboráknak")).toBeTruthy(); + // Taken by a zone over the page (the issue composer's screenshot box): no dragleave follows. + const e = new Event("drop", { bubbles: true, cancelable: true }); + Object.defineProperty(e, "dataTransfer", { + value: { types: ["Files"], files: Object.assign([], { item: () => null }) }, + }); + e.preventDefault(); + act(() => { + window.dispatchEvent(e); + }); + expect(screen.queryByText("files.drop.here:Cimboráknak")).toBeNull(); + }); + it("the open Upload dialog's own zone keeps the drop it accepted", async () => { mocks.files = []; mocks.folders = [YT, OWN]; diff --git a/frontend/src/components/Files.tsx b/frontend/src/components/Files.tsx index db10cba0..da78f1e9 100644 --- a/frontend/src/components/Files.tsx +++ b/frontend/src/components/Files.tsx @@ -117,6 +117,19 @@ function uploadFolder(view: FilesView, folders: readonly FileFolder[]): number | return f && (f.kind === "own" || f.source_key === "upload") ? f.id : null; } +/** The files the viewer pages through: the list's, and the open one even when the list moved on + * without it — a server sort's answer replacing the rows it was opened from. It closes only once the + * library no longer has it (deleted). */ +function previewFiles( + listed: LibraryFile[], + openId: number | null, + library: readonly (readonly LibraryFile[] | undefined)[], +): LibraryFile[] { + if (openId === null || listed.some((f) => f.id === openId)) return listed; + const open = library.flatMap((l) => l ?? []).find((f) => f.id === openId); + return open ? [open, ...listed] : listed; +} + function NameCell({ file, renaming, @@ -216,9 +229,11 @@ export default function Files() { intervalMs: 15000, enabled: wide, }); - // Until the server's first answer is in, the rows already on screen stay, searched and sorted here: - // a skeleton in their place would unmount the table under the header the user just pressed. - const wideIn = wide && (wideQ.data !== undefined || wideQ.isError); + // Until a server sort's first answer is in, the rows already on screen stay, sorted here: a skeleton + // in their place would unmount the table under the header the user just pressed. A search waits in + // a skeleton instead — the newest files searched here would say "no match" for a file the server is + // about to find — its keyboard being in the search box, not the table. + const wideIn = wide && (!!typed || wideQ.data !== undefined || wideQ.isError); const listQ = wideIn ? wideQ : filesQ; const all = listQ.data ?? []; const rows = wideIn ? all : all.filter((f) => matches(f, q, lang)); @@ -351,7 +366,14 @@ export default function Files() { const dragDepth = useRef(0); const dropReady = canUpload && upload === null; const onDrag = useEffectEvent((e: DragEvent) => { - if (!e.dataTransfer?.types.includes("Files") || e.defaultPrevented) return; + if (!e.dataTransfer?.types.includes("Files")) return; + // A drop ends the drag wherever it lands, taken by another zone too (no dragleave follows it): the + // page's "drop here" goes with it. + if (e.type === "drop") { + dragDepth.current = 0; + setDragging(false); + } + if (e.defaultPrevented) return; e.preventDefault(); if (!dropReady) { e.dataTransfer.dropEffect = "none"; @@ -360,7 +382,6 @@ export default function Files() { if (e.type === "dragenter") dragDepth.current += 1; if (e.type === "dragleave") dragDepth.current = Math.max(0, dragDepth.current - 1); if (e.type === "drop") { - dragDepth.current = 0; const dropped = e.dataTransfer.files; const first = dropped.item(0); if (first) setUpload({ file: first }); @@ -666,11 +687,18 @@ export default function Files() { )} {listQ.data && ( - {/* A list the cap cut says which files it holds: the newest, or the first by the - server's sort or search. */} - {t(cut ? (wideIn ? "files.countFirst" : "files.countNewest") : "files.count", { - count: rows.length, - })} + {/* A list the cap cut says which files it holds: the first by the server's sort, else + the newest (a search's too — the server answers it newest first). */} + {t( + cut + ? wideIn && serverSort + ? "files.countFirst" + : "files.countNewest" + : "files.count", + { + count: rows.length, + }, + )} )} {isGrid && ( @@ -744,8 +772,12 @@ export default function Files() { {viewing !== null && ( (f.shared ? sharedRowActions(f) : libraryRowActions(f)).view, + files={previewFiles( + sortRows(rows, columns, sort, lang).filter( + (f) => (f.shared ? sharedRowActions(f) : libraryRowActions(f)).view, + ), + viewing, + [filesQ.data, wideQ.data], )} fileId={viewing} onFileChange={setViewing} diff --git a/frontend/src/i18n/locales/en/files.json b/frontend/src/i18n/locales/en/files.json index a95ba58e..ed9a8844 100644 --- a/frontend/src/i18n/locales/en/files.json +++ b/frontend/src/i18n/locales/en/files.json @@ -11,7 +11,7 @@ "folderUnknown": "This folder could not be loaded", "arrival": { "gone": "That file is no longer in your library", - "notListed": "That file is past the end of this list — search for it by name" + "notListed": "That file isn't among the ones listed here — search for it by name" }, "phoneOk": "Plays on a phone too", "rail": { diff --git a/frontend/src/i18n/locales/hu/files.json b/frontend/src/i18n/locales/hu/files.json index d71e49ae..5faa47aa 100644 --- a/frontend/src/i18n/locales/hu/files.json +++ b/frontend/src/i18n/locales/hu/files.json @@ -11,7 +11,7 @@ "folderUnknown": "Ez a mappa nem tölthető be", "arrival": { "gone": "Ez a fájl már nincs a könyvtáradban", - "notListed": "Ez a fájl kimaradt a listából — keress rá a nevére" + "notListed": "Ez a fájl nincs a listázottak között — keress rá a nevére" }, "phoneOk": "Telefonon is lejátszható", "rail": {