fix(files): review round 1, the page — honest states, the keyboard, the tray's words
- A library past the list's cap is searched and sorted on the server; the count says "the newest". - Each folder and each search opens on its first page (DataTable pageScope, useCardPager scope). - A file dropped where the page takes none never reaches the browser (which would unload the app). - The viewer's arrows wait while a layer opened from it is on top, and scroll a wide text line; a text shows its first 5000 lines and says where the rest is. - Confirms name the file (no more "your downloads"); the asking menu rows end in "…". - The tray names a running step beside its bar, shows a pause mark for paused work, polls slowly while only paused work waits, and forgets a finished file that was deleted. - An unchanged rename sends nothing; a paste opens the download dialog only with nothing on top; the link box never replaces what was typed. - Honest states: folders that failed to load, a file no longer there, the storage figure; the rail says "nothing downloaded" only when there is nothing. - Focus after deleting the open folder and when the format form opens; the format name's label. - ui/Meter (one danger line for both quota bars) and ui/RenameField in the platform layer. - Copy fixes, stale comments and the speculative hub key; a contract test for the list's parameters; e2e: the Files page and the paste intake.
This commit is contained in:
@@ -62,6 +62,7 @@ import { downloadsApi } from "./api/downloads";
|
||||
export type * from "./api/downloads";
|
||||
import { filesApi } from "./api/files";
|
||||
export type * from "./api/files";
|
||||
export { FILES_LIST_CAP, FILE_SORTS } from "./api/files";
|
||||
import { debugSessionsApi, diagnosticsApi } from "./api/diagnostics";
|
||||
export type * from "./api/diagnostics";
|
||||
export { DIAGNOSTICS_EVENTS_PATH } from "./api/diagnostics";
|
||||
|
||||
@@ -121,8 +121,8 @@ export interface DownloadJob {
|
||||
// Whose it is — only the admin Storage tab's listing of every user's downloads carries them.
|
||||
user_id?: number;
|
||||
user_email?: string;
|
||||
// Where the viewer stopped in it (whole seconds; 0 = from the start). Only the two library listings
|
||||
// (`downloads`, `downloadsShared`) carry it — a job a mutation returns does not.
|
||||
// Where the viewer stopped in it (whole seconds; 0 = from the start). Only the listings carry it (the
|
||||
// Files library's and the queue's) — a job a mutation returns does not.
|
||||
position_seconds?: number;
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,50 @@
|
||||
// @vitest-environment jsdom
|
||||
import { readFileSync } from "node:fs";
|
||||
import { resolve } from "node:path";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { FILE_SORTS, FILES_LIST_CAP, listQuery } from "./files";
|
||||
|
||||
// The Files list's request, held to the route that answers it (backend routes/files.py `list_files`).
|
||||
// FastAPI ignores a query parameter it does not know, so a name renamed on one side only is no error
|
||||
// anywhere: a folder's view lists every file, "Shared with me" lists the reader's own files, a search
|
||||
// matches everything. The route's parameter names are read from its source, as text — the backend
|
||||
// cannot be imported here.
|
||||
const routes = readFileSync(resolve(__dirname, "../../../../backend/app/routes/files.py"), "utf-8");
|
||||
|
||||
function routeParams(): Map<string, string> {
|
||||
const sig = routes.match(/^def list_files\(([\s\S]*?)\) -> /m);
|
||||
expect(sig, "routes/files.py no longer defines list_files").toBeTruthy();
|
||||
const params = new Map<string, string>();
|
||||
for (const m of sig![1]!.matchAll(/^\s*(\w+): ([^\n]*?),?$/gm)) params.set(m[1]!, m[2]!);
|
||||
return params;
|
||||
}
|
||||
|
||||
const sent = (p: Parameters<typeof listQuery>[0]) => [
|
||||
...new URLSearchParams(listQuery(p).replace(/^\?/, "")).keys(),
|
||||
];
|
||||
|
||||
describe("the Files list's request", () => {
|
||||
it("names only parameters the route reads", () => {
|
||||
const known = routeParams();
|
||||
const names = new Set([
|
||||
...sent({ folderId: 3, q: "x", sort: "name", order: "asc" }),
|
||||
...sent({ shared: true }),
|
||||
]);
|
||||
expect([...names].sort()).toEqual(["folder_id", "order", "q", "scope", "sort"]);
|
||||
for (const n of names) expect(known.has(n), `list_files has no parameter "${n}"`).toBe(true);
|
||||
});
|
||||
|
||||
it("asks for the shared list and the sorts the route knows by its own words", () => {
|
||||
const known = routeParams();
|
||||
expect(listQuery({ shared: true })).toBe("?scope=shared");
|
||||
expect(known.get("scope")).toContain('"shared"');
|
||||
for (const s of FILE_SORTS) expect(known.get("sort")).toContain(`"${s}"`);
|
||||
});
|
||||
|
||||
it("knows the cap the route lists up to", () => {
|
||||
// A list as long as the cap may be missing older files; the page then searches on the server.
|
||||
const cap = routes.match(/^MAX_LIST = (\d+)$/m);
|
||||
expect(cap, "routes/files.py no longer defines MAX_LIST").toBeTruthy();
|
||||
expect(FILES_LIST_CAP).toBe(Number(cap![1]));
|
||||
});
|
||||
});
|
||||
@@ -19,7 +19,13 @@ export interface FileFolder {
|
||||
created_at: string | null;
|
||||
}
|
||||
|
||||
// The most files one list answers with (backend `routes/files.py` MAX_LIST, held equal by
|
||||
// files.contract.test.ts): a list this long may be missing older files, so the page asks the
|
||||
// server to search and sort it rather than doing either on the rows it got.
|
||||
export const FILES_LIST_CAP = 500;
|
||||
|
||||
export type FileSort = "added" | "name" | "size";
|
||||
export const FILE_SORTS: readonly FileSort[] = ["added", "name", "size"];
|
||||
export type FileOrder = "asc" | "desc";
|
||||
|
||||
export interface FileListParams {
|
||||
@@ -69,7 +75,8 @@ export interface StorageUsage {
|
||||
upload_max_bytes: number;
|
||||
}
|
||||
|
||||
function listQuery(p: FileListParams): string {
|
||||
/** The list's query string — exported for the contract test that holds its names to the route's. */
|
||||
export function listQuery(p: FileListParams): string {
|
||||
const q = new URLSearchParams();
|
||||
if (p.shared) q.set("scope", "shared");
|
||||
else if (p.folderId != null) q.set("folder_id", String(p.folderId));
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { ensureNavId, peekNavId } from "./history";
|
||||
import { anyBlockingLayer } from "./layerStack";
|
||||
import { createStore } from "./store";
|
||||
|
||||
// Downloading a link is an action you start anywhere: the Files page's link box, a link pasted onto
|
||||
@@ -13,13 +14,15 @@ export function asDownloadLink(text: string | null | undefined): string | null {
|
||||
return /^https?:\/\/\S+$/i.test(s) ? s : null;
|
||||
}
|
||||
|
||||
/** Whether a paste here is the page's to take: not typed into a field, and no dialog on top (the
|
||||
* reader is busy with that, and a link pasted at it is not a request to download). */
|
||||
/** Whether a paste here is the page's to take: not typed into a field, and nothing on top that owns
|
||||
* the keyboard — a dialog, a player, an open menu (the reader is busy with that, and a link pasted
|
||||
* at it is not a request to download). The layer stack knows them all; a player marks no
|
||||
* `aria-modal`, so the markup alone would miss it. */
|
||||
export function pasteIsForThePage(target: EventTarget | null): boolean {
|
||||
const el = target instanceof Element ? target : null;
|
||||
if (el?.closest("input, textarea, select, [contenteditable]:not([contenteditable='false'])"))
|
||||
return false;
|
||||
return !document.querySelector("[aria-modal='true']");
|
||||
return !anyBlockingLayer() && !document.querySelector("[aria-modal='true']");
|
||||
}
|
||||
|
||||
/** Ask to download `url`: the app's download dialog opens on it. */
|
||||
|
||||
@@ -6,6 +6,7 @@ import { beforeEach, describe, expect, it } from "vitest";
|
||||
import type { QueueJob } from "./api";
|
||||
import {
|
||||
clearFinished,
|
||||
forgetFinished,
|
||||
noteQueue,
|
||||
resetQueueWatch,
|
||||
takeUnannounced,
|
||||
@@ -47,4 +48,11 @@ describe("downloadQueue · a finish", () => {
|
||||
clearFinished();
|
||||
expect(finishedIds()).toEqual([]);
|
||||
});
|
||||
|
||||
it("a finished file deleted since leaves the tray: its Open and Copy link would reach nothing", () => {
|
||||
noteQueue([job(1, "running"), job(2, "running")]);
|
||||
noteQueue([job(1, "done"), job(2, "done")]);
|
||||
forgetFinished(1);
|
||||
expect(finishedIds()).toEqual([2]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { api, type QueueJob } from "./api";
|
||||
import { api, type DownloadStatus, type QueueJob } from "./api";
|
||||
import { createStore } from "./store";
|
||||
|
||||
// The download queue as this tab has watched it: which jobs it has seen finish. The tray shows them
|
||||
@@ -8,8 +8,26 @@ import { createStore } from "./store";
|
||||
// through `fetchQueue`, so a finish is seen once whichever of them polled, and nothing has to diff the
|
||||
// previous render's data.
|
||||
|
||||
/** The statuses a job leaves on its own: it is waiting, or working. */
|
||||
export const IN_FLIGHT: ReadonlySet<string> = new Set(["queued", "running", "paused"]);
|
||||
/** The statuses the tray lists as under way: waiting, working, or paused by its user. */
|
||||
export const IN_FLIGHT: ReadonlySet<DownloadStatus> = new Set(["queued", "running", "paused"]);
|
||||
|
||||
/** The statuses a job leaves on its own, so the queue is worth asking about often. A paused job is
|
||||
* not one: it waits for its user, however long that takes. */
|
||||
export const MOVING: ReadonlySet<DownloadStatus> = new Set(["queued", "running"]);
|
||||
|
||||
/** The steps a running job reports that have a name of their own (`downloads.phase.*`). */
|
||||
export const DOWNLOAD_PHASES: readonly string[] = [
|
||||
"video",
|
||||
"audio",
|
||||
"editing",
|
||||
"converting",
|
||||
"merging",
|
||||
"audio_extract",
|
||||
"thumbnail",
|
||||
"sponsorblock",
|
||||
"metadata",
|
||||
"processing",
|
||||
];
|
||||
|
||||
let lastSeen: Map<number, string> | null = null;
|
||||
const finished = createStore<QueueJob[]>([]);
|
||||
@@ -48,6 +66,10 @@ export const useFinished = (): QueueJob[] => finished.useValue();
|
||||
/** The tray was closed: what finished is no longer shown. */
|
||||
export const clearFinished = (): void => finished.set([]);
|
||||
|
||||
/** A file was deleted: a "Done · Open" row for it would open and link nothing. */
|
||||
export const forgetFinished = (id: number): void =>
|
||||
finished.set((list) => (list.some((j) => j.id === id) ? list.filter((j) => j.id !== id) : list));
|
||||
|
||||
/** The finishes not yet announced — the render subscription that wakes the announcer. */
|
||||
export const useUnannounced = (): QueueJob[] => unannounced.useValue();
|
||||
|
||||
|
||||
@@ -108,3 +108,16 @@ describe("useCardPager — following a moved row", () => {
|
||||
expect(result.current.pagerProps.page).toBe(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe("useCardPager — another list", () => {
|
||||
it("starts on page one when the caller's scope changes", () => {
|
||||
const { result, rerender } = renderHook(
|
||||
({ scope }) => useCardPager(rows(100), KEY, true, null, undefined, scope),
|
||||
{ initialProps: { scope: "folder 1" } },
|
||||
);
|
||||
act(() => result.current.pagerProps.onPageChange(3));
|
||||
expect(result.current.pagerProps.page).toBe(3);
|
||||
rerender({ scope: "folder 2" });
|
||||
expect(result.current.pagerProps.page).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -19,10 +19,17 @@ export function useCardPager<T>(
|
||||
reveal?: { seq: number; match: (row: T) => boolean } | null,
|
||||
/** The size picker's name — what the cards ARE (the channel manager's are "cards"). */
|
||||
sizeLabel?: string,
|
||||
/** Which list `rows` are: another one (a folder, a search, a sort) starts at page one. */
|
||||
scope?: string,
|
||||
) {
|
||||
const { t } = useTranslation();
|
||||
const [size, setSize] = useState(() => parseCardSize(readAccount(sizeKey, null)));
|
||||
const [page, setPage] = useState(0);
|
||||
const [scopeSeen, setScopeSeen] = useState(scope);
|
||||
if (scope !== scopeSeen) {
|
||||
setScopeSeen(scope);
|
||||
setPage(0);
|
||||
}
|
||||
|
||||
// During render against the last seq seen, as in DataTable (seeded so a mount never jumps).
|
||||
const [revealSeen, setRevealSeen] = useState(reveal?.seq);
|
||||
|
||||
Reference in New Issue
Block a user