diff --git a/frontend/src/components/FilePreview.dom.test.tsx b/frontend/src/components/FilePreview.dom.test.tsx index 63462a64..1f87fed4 100644 --- a/frontend/src/components/FilePreview.dom.test.tsx +++ b/frontend/src/components/FilePreview.dom.test.tsx @@ -181,6 +181,30 @@ describe("FilePreview · stepping", () => { expect(onFileChange).not.toHaveBeenCalled(); }); + it("a text whose lines fit pages under ← / →, even with the keyboard in it", async () => { + vi.spyOn(api, "previewText").mockResolvedValue({ text: "short", truncated: false }); + const { onFileChange } = mount([file(1, "image"), file(8, "text"), file(9, "text")], 8); + await screen.findByText("short"); + const region = screen.getByRole("region"); + Object.defineProperty(region, "scrollWidth", { value: 300 }); + Object.defineProperty(region, "clientWidth", { value: 300 }); + fireEvent.keyDown(region, { key: "ArrowRight" }); + expect(onFileChange).toHaveBeenLastCalledWith(9); + }); + + it("Escape closes it — after a layer opened above it has taken its own", () => { + const { onClose } = mount(list, 2); + const menu = pushLayer(); + try { + fireEvent.keyDown(document, { key: "Escape" }); + expect(onClose).not.toHaveBeenCalled(); + } finally { + removeLayer(menu); + } + fireEvent.keyDown(document, { key: "Escape" }); + expect(onClose).toHaveBeenCalledTimes(1); + }); + it("stops at either end", () => { const { onFileChange } = mount(list, 1); fireEvent.keyDown(window, { key: "ArrowLeft" }); diff --git a/frontend/src/components/FilePreview.tsx b/frontend/src/components/FilePreview.tsx index 2a5d6725..a37f6a3f 100644 --- a/frontend/src/components/FilePreview.tsx +++ b/frontend/src/components/FilePreview.tsx @@ -77,11 +77,13 @@ export default function FilePreview({ if (next) onFileChange(next.id); }; useKeyScope("fileViewer"); - // The viewer's own place on the layer stack (above its dialog): ← / → are its only while nothing is - // on top of it — its "…" menu, a confirm, the Share or Details dialog opened from that menu. There an - // arrow moves through the layer's own buttons; paging the viewer under it would show one file while - // the dialog acts on another. - const layer = useLayer(true, undefined, { passive: true }); + // The viewer's own place on the layer stack: ← / → are its only while nothing is on top of it — its + // "…" menu, a confirm, the Share or Details dialog opened from that menu. There an arrow moves + // through the layer's own buttons; paging the viewer under it would show one file while the dialog + // acts on another. It mounts with its dialog (both only while there is a file), and effects run + // child first, so it sits just ABOVE the dialog's layer: the dialog is never topmost, and Escape is + // this layer's to take — passive, so the dialog keeps its Tab trap. + const layer = useLayer(!!file, onClose, { passive: true }); const onKey = useEffectEvent((e: KeyboardEvent) => { if (layer.blockingAbove()) return; // The text, scrolled sideways by its arrows: a line wider than the stage is read that way. diff --git a/frontend/src/components/Files.menu.dom.test.tsx b/frontend/src/components/Files.menu.dom.test.tsx index 227f7f4b..eb96f832 100644 --- a/frontend/src/components/Files.menu.dom.test.tsx +++ b/frontend/src/components/Files.menu.dom.test.tsx @@ -8,7 +8,15 @@ // list. i18n is key-passthrough, so labels read as keys. import { useState } from "react"; import { afterEach, describe, expect, it, vi } from "vitest"; -import { act, cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { + act, + cleanup, + createEvent, + fireEvent, + render, + screen, + waitFor, +} from "@testing-library/react"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { api, type FileFolder, type LibraryFile } from "../lib/api"; import { formatDateTime } from "../lib/format"; @@ -28,6 +36,8 @@ const mocks = vi.hoisted(() => ({ fetching: false, titleArgs: [] as unknown[], searched: [] as unknown[], + // The server's search/sort answer is still on its way (no data yet). + wideLoading: false, foldersFailed: false, notify: vi.fn(), })); @@ -63,6 +73,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.wideLoading) + return { + data: undefined, + isError: false, + isLoading: true, + isFetching: true, + refetch: vi.fn(), + }; return { data: key[0] === "files" @@ -87,7 +105,8 @@ vi.mock("./ui/PageShell", async (importOriginal) => ({ })); import Files from "./Files"; -import { FILES_LIST_CAP } from "../lib/api"; +import { FILES_LIST_CAP, type QueueJob } from "../lib/api"; +import { noteQueue, resetQueueWatch, useFinished } from "../lib/downloadQueue"; const file = (id: number, extra: Partial = {}): LibraryFile => ({ @@ -133,6 +152,10 @@ afterEach(() => { mocks.confirm.mockClear(); mocks.keys = []; mocks.fetching = false; + mocks.titleArgs = []; + mocks.wideLoading = false; + mocks.notify.mockClear(); + resetQueueWatch(); }); function mount( @@ -382,6 +405,26 @@ describe("Files · my file's row", () => { fireEvent.click(menuItem("files.actions.delete")); await waitFor(() => expect(del).toHaveBeenCalledWith(2)); }); + + it("a deleted file leaves the tray's finished list: its Open would reach nothing", async () => { + mocks.files = [file(2)]; + mocks.folders = [YT]; + vi.spyOn(api, "deleteFile").mockResolvedValue({} as never); + const job = (status: string) => ({ id: 2, status }) as unknown as QueueJob; + noteQueue([job("running")]); + noteQueue([job("done")]); + let finished: number[] = []; + function Tray() { + finished = useFinished().map((j) => j.id); + return null; + } + render(); + expect(finished).toEqual([2]); + mount(); + await openMenu(() => fireEvent.contextMenu(rowOf("Clip 2"))); + fireEvent.click(menuItem("files.actions.delete")); + await waitFor(() => expect(finished).toEqual([])); + }); }); describe("Files · a library past the list's cap", () => { @@ -406,6 +449,83 @@ describe("Files · a library past the list's cap", () => { expect(rowOf("Régi recept")).toBeTruthy(); expect(mocks.keys).toContainEqual(["files", { q: "recept" }]); }); + + const capped = () => Array.from({ length: FILES_LIST_CAP }, (_, i) => file(i + 1)); + const nameHeader = () => screen.getByRole("button", { name: /files\.columns\.name/ }); + + it("a sort asked of the server keeps the table on screen until its answer is in", () => { + mocks.folders = [YT]; + mocks.files = capped(); + mocks.wideLoading = true; + mount(); + const header = nameHeader(); + fireEvent.click(header); + // Not a skeleton in its place: that would take the header just pressed, and the keyboard with it. + expect(header.isConnected).toBe(true); + expect(rowOf("Clip 1")).toBeTruthy(); + expect(screen.getByText("files.countNewest")).toBeTruthy(); + }); + + it("says a list the server's sort cut holds only its first files; a short search is whole", () => { + mocks.folders = [YT]; + mocks.files = capped(); + mocks.searched = Array.from({ length: FILES_LIST_CAP }, (_, i) => file(i + 1000)); + mount(); + expect(screen.getByText("files.countNewest")).toBeTruthy(); + fireEvent.click(nameHeader()); + expect(screen.getByText("files.countFirst")).toBeTruthy(); + cleanup(); + + mocks.searched = [file(9999, { display_name: "Régi recept" })]; + mount(ALL_FILES, vi.fn(), "recept"); + expect(screen.getByText("files.count")).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(); + mocks.wideLoading = true; + const consumeFocus = vi.fn(); + mount(ALL_FILES, vi.fn(), "clip", { focus: { id: 3, token: 1 }, consumeFocus }); + await new Promise((r) => setTimeout(r, 20)); + expect(consumeFocus).not.toHaveBeenCalled(); + }); + + it("a file past the end of a cut list is not said to be gone — it may only be older", async () => { + mocks.folders = [YT]; + mocks.files = capped(); + mount(ALL_FILES, vi.fn(), "", { focus: { id: 9999, token: 1 } }); + await waitFor(() => + expect(mocks.notify).toHaveBeenCalledWith({ + level: "info", + message: "files.arrival.notListed", + }), + ); + expect(mocks.notify).not.toHaveBeenCalledWith({ level: "info", message: "files.arrival.gone" }); + }); + + it("an arrival's cleared search is asked of the server at once, not after the typing pause", () => { + mocks.folders = [YT]; + mocks.files = capped(); + mocks.searched = []; + const view = (search: string, focus: FilesFocus | null) => ( + + + + + + + + ); + const r = render(view("recept", null)); + expect(mocks.keys).toContainEqual(["files", { q: "recept" }]); + mocks.keys = []; + // openFile: the search cleared and an arrival pending, in one step. + r.rerender(view("", { id: 3, token: 1 })); + expect(mocks.keys).not.toContainEqual(["files", { q: "recept" }]); + }); }); describe("Files · the confirms", () => { @@ -429,6 +549,10 @@ describe("Files · the confirms", () => { ["downloads.confirm.redownloadTitle", "downloads.confirm.redownloadAction"], ["downloads.confirm.deleteTitle", "downloads.actions.delete"], ]); + expect(mocks.titleArgs).toContainEqual([ + "downloads.confirm.redownloadTitle", + { name: "Clip 2" }, + ]); mocks.confirm.mockResolvedValue(true); }); @@ -468,6 +592,10 @@ describe("Files · a shared file's row", () => { ]); fireEvent.click(menuItem("files.actions.removeShared")); await waitFor(() => expect(remove).toHaveBeenCalledWith(4)); + expect(mocks.titleArgs).toContainEqual([ + "downloads.confirm.removeSharedTitle", + { name: "Clip 4" }, + ]); // The shared list is its own read, and nothing can be uploaded into it. expect(mocks.keys).toContainEqual(["files", { shared: true }]); expect(screen.queryByText("files.actions.upload")).toBeNull(); @@ -879,9 +1007,27 @@ describe("Files · dropping a file on the page", () => { mount({ kind: "folder", id: 2 }); drag("drop", [new File(["x"], "recept.pdf")]); expect(await screen.findByText("recept.pdf")).toBeTruthy(); - expect(drag("dragover").defaultPrevented).toBe(true); + const beside = drag("dragover"); + expect(beside.defaultPrevented).toBe(true); + expect((beside as DragEvent).dataTransfer?.dropEffect).toBe("none"); expect(drag("drop", [new File(["y"], "masik.pdf")]).defaultPrevented).toBe(true); }); + + it("the open Upload dialog's own zone keeps the drop it accepted", async () => { + mocks.files = []; + mocks.folders = [YT, OWN]; + vi.spyOn(api, "filesUsage").mockResolvedValue({ upload_max_bytes: 1_000_000 } as never); + mount({ kind: "folder", id: 2 }); + drag("drop", [new File(["x"], "recept.pdf")]); + const zone = (await screen.findByText("recept.pdf")).closest("button")!; + const over = createEvent.dragOver(zone, { + dataTransfer: { types: ["Files"], files: [], dropEffect: "copy" }, + }) as DragEvent; + fireEvent(zone, over); + // A "none" written by the page's own listener (it runs last) would refuse the zone's drop. + expect(over.defaultPrevented).toBe(true); + expect(over.dataTransfer?.dropEffect).toBe("copy"); + }); }); describe("Files · when what it needs is missing", () => { diff --git a/frontend/src/components/Files.tsx b/frontend/src/components/Files.tsx index 260a71a6..db10cba0 100644 --- a/frontend/src/components/Files.tsx +++ b/frontend/src/components/Files.tsx @@ -202,7 +202,9 @@ export default function Files() { // would miss an older file, so the server does both, over the whole view. Asked once the typing // pauses; a sort the server does not know (the source, the retention) stays on the rows. const capped = (filesQ.data?.length ?? 0) >= FILES_LIST_CAP; - const typed = useDebounced(q, 300); + // An arrival at one file clears the search (openFile): the cleared box is taken at once, not after + // a pause, so the file is looked for in the list it is about to be shown in. + const typed = useDebounced(q, 300, focus?.token); const serverSort = sort && (FILE_SORTS as readonly string[]).includes(sort.key) ? sort : null; const wide = capped && (!!typed || !!serverSort); const wideParams: FileListParams = { @@ -214,9 +216,14 @@ export default function Files() { intervalMs: 15000, enabled: wide, }); - const listQ = wide ? wideQ : filesQ; + // 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); + const listQ = wideIn ? wideQ : filesQ; const all = listQ.data ?? []; - const rows = wide ? all : all.filter((f) => matches(f, q, lang)); + const rows = wideIn ? all : all.filter((f) => matches(f, q, lang)); + // The server's cap cut this list: the newest files of the view, or the first by its sort or search. + const cut = all.length >= FILES_LIST_CAP; const listRef = useRef(null); const [renamingId, setRenamingId] = useState(null); @@ -302,19 +309,23 @@ export default function Files() { // (FilesProvider), which turns the table's or the grid's page to the file (`reveal`), puts the // keyboard on it and rings it while this view is open. Not in this view, it is looked for among // every file; nowhere (gone since the link was made), it leaves nothing to arrive at, and is spent. - const here = !!focus && !!filesQ.data?.some((f) => f.id === focus.id); + // Looked for in the rows the page SHOWS — a server sort's own answer, not the rows standing in until + // it comes: the table can only turn to a file it holds. + const shownData = wide && !wideIn ? undefined : listQ.data; + const here = !!focus && !!shownData?.some((f) => f.id === focus.id); // "Not here" only once the list is fresh: a just-finished upload is not in the cached rows yet. - const missing = !!focus && !!filesQ.data && !here && !filesQ.isFetching; + const missing = !!focus && !!shownData && !here && !listQ.isFetching; const lookEverywhere = missing && shown.kind !== "all"; useEffect(() => { if (lookEverywhere) setView(ALL_FILES); else if (here) consumeFocus(viewKey); else if (missing) { consumeFocus(null); - // Said, not swallowed: an old notice's file may have expired or been deleted since. - notify({ level: "info", message: t("files.arrival.gone") }); + // Said, not swallowed. A list the cap cut cannot tell a deleted file from one beyond its end; + // a whole list can: an old notice's file may have expired or been deleted since. + notify({ level: "info", message: t(cut ? "files.arrival.notListed" : "files.arrival.gone") }); } - }, [here, missing, lookEverywhere, viewKey, setView, consumeFocus, t]); + }, [here, missing, lookEverywhere, cut, viewKey, setView, consumeFocus, t]); const arrival = arrived && arrived.view === viewKey ? arrived : null; const ringed = arrival?.id ?? null; const reveal = arrival && { key: String(arrival.id), seq: arrival.token }; @@ -331,14 +342,16 @@ export default function Files() { // ── A file dropped anywhere on the page ───────────────────────────────────────────────────────── // The page is the target, not a box on it: a short folder leaves most of the screen empty. Window // listeners, since the drag starts outside the app. While the dialog is open its own zone takes the - // drop; a drag that carries no files (a link, selected text) is not ours. A file dropped where the - // page takes none (beside the open dialog, on the shared list) is still kept from the browser, - // which would open it in the tab — unloading the app and any upload under way. + // drop: a drag something under the pointer already accepted (that zone) is left as it set it — these + // listeners run last, and a "none" written here would refuse the zone's drop. A drag that carries no + // files (a link, selected text) is not ours. A file dropped where the page takes none (beside the + // open dialog, on the shared list) is still kept from the browser, which would open it in the tab — + // unloading the app and any upload under way. const [dragging, setDragging] = useState(false); const dragDepth = useRef(0); const dropReady = canUpload && upload === null; const onDrag = useEffectEvent((e: DragEvent) => { - if (!e.dataTransfer?.types.includes("Files")) return; + if (!e.dataTransfer?.types.includes("Files") || e.defaultPrevented) return; e.preventDefault(); if (!dropReady) { e.dataTransfer.dropEffect = "none"; @@ -653,8 +666,11 @@ export default function Files() { )} {listQ.data && ( - {/* A capped list is the newest files of the view, and says so. */} - {t(capped && !wide ? "files.countNewest" : "files.count", { count: rows.length })} + {/* 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, + })} )} {isGrid && ( diff --git a/frontend/src/i18n/locales/en/files.json b/frontend/src/i18n/locales/en/files.json index 2d82ab0f..a95ba58e 100644 --- a/frontend/src/i18n/locales/en/files.json +++ b/frontend/src/i18n/locales/en/files.json @@ -5,10 +5,13 @@ "count_other": "{{count}} items", "countNewest_one": "The newest {{count}} item", "countNewest_other": "The newest {{count}} items", + "countFirst_one": "The first {{count}} item", + "countFirst_other": "The first {{count}} items", "rowMenu": "File actions", "folderUnknown": "This folder could not be loaded", "arrival": { - "gone": "That file is no longer in your library" + "gone": "That file is no longer in your library", + "notListed": "That file is past the end of this list — 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 6f941108..d71e49ae 100644 --- a/frontend/src/i18n/locales/hu/files.json +++ b/frontend/src/i18n/locales/hu/files.json @@ -5,10 +5,13 @@ "count_other": "{{count}} elem", "countNewest_one": "A legújabb {{count}} elem", "countNewest_other": "A legújabb {{count}} elem", + "countFirst_one": "Az első {{count}} elem", + "countFirst_other": "Az első {{count}} elem", "rowMenu": "Fájl műveletei", "folderUnknown": "Ez a mappa nem tölthető be", "arrival": { - "gone": "Ez a fájl már nincs a könyvtáradban" + "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" }, "phoneOk": "Telefonon is lejátszható", "rail": {