diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 5c41dfb6..7ee47296 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -651,8 +651,9 @@ of a full-width panel and nothing on the right. (the long side: 320 / 480 / 640 px), the expected size ("about 4–10 MB"; one number when both ends read the same), a note over 15 MB (a chat's link preview will not move) or 30 MB (it may be refused), and over 30 s the reason, with Make GIF unavailable. A refused length is shown rounded up, never as one the cap allows. The - cap is judged in whole milliseconds between the ends as sent, the server's own rule (`lib/gif`, held to - `downloads/gif.py` by `gif.contract.test.ts`). The editor has its own Format row (a video only): with GIF + cap is judged in whole milliseconds between the ends as sent, the server's own rule (the limits in `lib/gif` + are held to `downloads/gif.py` by `gif.contract.test.ts`; the rule itself is written on each side and + tested there against the same boundary cases). The editor has its own Format row (a video only): with GIF each kept segment is a GIF of its own, Join is unavailable with its reason in a tooltip, the precise / fast choice gives way to one line (a GIF is always re-encoded), and the notes name each segment over a limit, in one live region. A shared GIF's page shows the picture itself (no player), its name, kind and size, its diff --git a/frontend/src/features/player/ClipPanel.dom.test.tsx b/frontend/src/features/player/ClipPanel.dom.test.tsx index a1d0bd33..ccf4ef10 100644 --- a/frontend/src/features/player/ClipPanel.dom.test.tsx +++ b/frontend/src/features/player/ClipPanel.dom.test.tsx @@ -14,9 +14,11 @@ vi.mock("react-i18next", async (importOriginal) => { const actual = await importOriginal(); return { ...actual, - // A size option reads as its own words with its unit ("480 px"); every other text is its key. + // A size option reads as its own words with its unit ("480 px"), the length with the length it + // gives; every other text is its key. useTranslation: () => ({ - t: (k: string, o?: { n?: string }) => (k === "clip.gif.side" ? `${o?.n} px` : k), + t: (k: string, o?: { n?: string; length?: string }) => + k === "clip.gif.side" ? `${o?.n} px` : k === "clip.length" ? `${k} ${o?.length}` : k, i18n: { language: "en" }, }), }; @@ -322,6 +324,30 @@ describe("ClipPanel GIF", () => { expect(enqueueClip).not.toHaveBeenCalled(); }); + it("says a refused GIF length so it never reads as one the cap allows", () => { + render(); + // A video of it is 0:30.0 long, as it always read… + expect(screen.getByText(/^clip\.length /).textContent).toBe("clip.length 0:30.0"); + fireEvent.click(screen.getByRole("button", { name: "clip.formatGif" })); + // …and a GIF of it, refused, reads 0:30.1. + expect(screen.getByText(/^clip\.length /).textContent).toBe("clip.length 0:30.1"); + const make = screen.getByRole("button", { name: "clip.gif.make" }); + expect(make.getAttribute("aria-disabled")).toBe("true"); + }); + + it("says a note already true when GIF is picked inside a live region that was there before", () => { + render(); + const before = new Map( + [...document.querySelectorAll("[aria-live='polite']")].map((r) => [r, r.textContent]), + ); + fireEvent.click(screen.getByRole("button", { name: "clip.formatGif" })); + // The reason and the estimate change inside regions that were there, empty, before: what + // changes inside a region is announced, a region that arrives with its text is not. + const note = screen.getByText("clip.gif.tooLong").closest("[aria-live='polite']")!; + const estimate = screen.getByText("clip.gif.estimate").closest("[aria-live='polite']")!; + for (const region of [note, estimate]) expect(before.get(region)).toBe(""); + }); + it("says what the expected size means: fine, no moving preview, or past the cap", () => { render(); fireEvent.click(screen.getByRole("button", { name: "clip.formatGif" })); diff --git a/frontend/src/features/player/ClipPanel.tsx b/frontend/src/features/player/ClipPanel.tsx index 238aea08..76613fb7 100644 --- a/frontend/src/features/player/ClipPanel.tsx +++ b/frontend/src/features/player/ClipPanel.tsx @@ -432,33 +432,41 @@ export default function ClipPanel({ label: t("clip.gif.side", { n: formatNumber(s, i18n.language) }), }))} /> - - {t(estimateText.single ? "clip.gif.estimateOne" : "clip.gif.estimate", estimateText)} - )} + {/* The live regions stay mounted whatever the format (laid out as their content alone): a + live region announces what changes inside it, not what arrives together with it, so + a note already true when GIF is picked is said too. */} + + {gif && ( + + {t(estimateText.single ? "clip.gif.estimateOne" : "clip.gif.estimate", estimateText)} + + )} + + +
+ {gif && ( +

+ {verdict === "too_long" + ? t("clip.gif.tooLong", { seconds: GIF_MAX_S }) + : verdict === "may_fail" + ? t("clip.gif.mayFail") + : verdict === "no_preview" + ? t("clip.gif.noPreview") + : t("clip.gif.hint")} +

+ )}
- {gif && ( -

- {verdict === "too_long" - ? t("clip.gif.tooLong", { seconds: GIF_MAX_S }) - : verdict === "may_fail" - ? t("clip.gif.mayFail") - : verdict === "no_preview" - ? t("clip.gif.noPreview") - : t("clip.gif.hint")} -

- )}
({ ...(await importOriginal()), - // A size option reads as its own words with its unit ("480 px"), a too-long note with the length - // it gives; every other text is its key. + // A size option reads as its own words with its unit ("480 px"), a segment's note with the + // segment it names (and a too-long one with the length it gives), the kept summary with its + // length; every other text is its key. useTranslation: () => ({ - t: (key: string, o?: { n?: string; length?: string }) => + t: (key: string, o?: { n?: string; length?: string; dur?: string }) => key === "clip.gif.side" ? `${o?.n} px` : key === "editor.format.segTooLong" - ? `${key} ${o?.length}` - : key, + ? `${key} #${o?.n} ${o?.length}` + : key === "editor.format.segMayFail" || key === "editor.format.segNoPreview" + ? `${key} #${o?.n}` + : key === "editor.keptSummary" + ? `${key} ${o?.dur}` + : key, i18n: { language: "en" }, }), })); @@ -123,7 +128,7 @@ describe("VideoEditor — GIF", () => { const create = screen.getByRole("button", { name: "editor.createGif" }); expect(create.getAttribute("aria-disabled")).toBe("true"); const notes = document.getElementById(create.getAttribute("aria-describedby")!)!; - expect(notes.textContent).toBe("editor.format.segTooLong 1:40.0"); + expect(notes.textContent).toBe("editor.format.segTooLong #1 1:40.0"); fireEvent.click(create); expect(enqueue).not.toHaveBeenCalled(); // A video of the same range has no such limit. @@ -140,7 +145,36 @@ describe("VideoEditor — GIF", () => { const create = screen.getByRole("button", { name: "editor.createGif" }); expect(create.getAttribute("aria-disabled")).toBe("true"); const notes = document.getElementById(create.getAttribute("aria-describedby")!)!; - expect(notes.textContent).toBe("editor.format.segTooLong 0:30.1"); + expect(notes.textContent).toBe("editor.format.segTooLong #1 0:30.1"); + // The segment's own length and the kept summary say it the same way, never 0:30.0. + expect(screen.getByText("0:30.1")).toBeTruthy(); + expect(screen.getByText("editor.keptSummary 0:30.1")).toBeTruthy(); + expect(screen.queryByText("0:30.0")).toBeNull(); + }); + + it("names each segment by its place, the worst note first, a dropped one left out", () => { + const { playhead } = open(80); + playhead(10); + fireEvent.click(screen.getByText("editor.splitHere")); + playhead(30); + fireEvent.click(screen.getByText("editor.splitHere")); + // Segment 1 (0–10) dropped; 2 (10–30) and 3 (30–80) kept. + const track = screen.getByRole("group", { name: "editor.timeline" }); + fireEvent.pointerDown(within(track).getAllByRole("button", { pressed: true })[0]!); + fireEvent.click(screen.getByRole("button", { name: "editor.format.gif" })); + fireEvent.click(screen.getByRole("button", { name: "640 px" })); + const notes = [...document.querySelectorAll("li")].map((l) => l.textContent); + // 50 s is too long, 20 s at 640 may pass 30 MB; the dropped segment says nothing. + expect(notes).toEqual(["editor.format.segTooLong #3 0:50.0", "editor.format.segMayFail #2"]); + }); + + it("says a note already true when GIF is picked inside a live region that was there before", () => { + open(100); + const region = document.querySelector("div[aria-live='polite']")!; + expect(region.textContent).toBe(""); + fireEvent.click(screen.getByRole("button", { name: "editor.format.gif" })); + expect(document.querySelector("div[aria-live='polite']")).toBe(region); + expect(region.textContent).toBe("editor.format.segTooLong #1 1:40.0"); }); it("says a GIF failure as one of GIFs, not of clips", async () => { @@ -187,7 +221,7 @@ describe("VideoEditor — GIF", () => { // 20 s at 640: up to about 32 MB, said inside the same region. fireEvent.click(screen.getByRole("button", { name: "640 px" })); expect(document.querySelector("div[aria-live='polite']")).toBe(region); - expect(region.textContent).toBe("editor.format.segMayFail"); + expect(region.textContent).toBe("editor.format.segMayFail #1"); }); it("says one size for a short GIF, and none when nothing is kept", () => { diff --git a/frontend/src/modules/files/VideoEditor.tsx b/frontend/src/modules/files/VideoEditor.tsx index f0c4ca06..f3534487 100644 --- a/frontend/src/modules/files/VideoEditor.tsx +++ b/frontend/src/modules/files/VideoEditor.tsx @@ -599,7 +599,7 @@ export default function VideoEditor({ job, onClose }: { job: DownloadJob; onClos /> - {tc(sel.end - sel.start)} + {tc(gif ? gifShownLength(sel.start, sel.end) : sel.end - sel.start)} {(inField.invalid || outField.invalid) && (
@@ -633,7 +633,13 @@ export default function VideoEditor({ job, onClose }: { job: DownloadJob; onClos })}
- {t("editor.keptSummary", { n: kept.length, dur: tc(keptDur) })} + {t("editor.keptSummary", { + n: kept.length, + // One GIF: its length as the notes say it (a refused one never reads as allowed). + dur: tc( + gif && kept.length === 1 ? gifShownLength(kept[0]!.start, kept[0]!.end) : keptDur, + ), + })}
@@ -664,37 +670,38 @@ export default function VideoEditor({ job, onClose }: { job: DownloadJob; onClos label: t("clip.gif.side", { n: formatNumber(s, i18n.language) }), }))} /> - {kept.length > 0 && ( - - {t( - gifTotalText.single ? "editor.format.estimateOne" : "editor.format.estimate", - gifTotalText, - )} - - )} )} + {/* The live regions stay mounted whatever the format (laid out as their content + alone): a live region announces what changes inside it, not what arrives together + with it, so a note already true when GIF is picked is said too. */} + + {gif && kept.length > 0 && ( + + {t( + gifTotalText.single ? "editor.format.estimateOne" : "editor.format.estimate", + gifTotalText, + )} + + )} +
)} - {/* Mounted for as long as GIF is chosen: a live region announces what changes inside it, - not what arrives together with it, and the first note is the one that counts. */} - {gif && ( -
- {gifNotes.length > 0 && ( -
    - {gifNotes.map((g) => ( -
  • - {t(GIF_NOTE_KEY[g.verdict as keyof typeof GIF_NOTE_KEY], { - n: g.n, - length: tc(g.length), - seconds: GIF_MAX_S, - })} -
    • - ))} -
- )} -
- )} +
+ {gif && gifNotes.length > 0 && ( +
    + {gifNotes.map((g) => ( +
  • + {t(GIF_NOTE_KEY[g.verdict as keyof typeof GIF_NOTE_KEY], { + n: g.n, + length: tc(g.length), + seconds: GIF_MAX_S, + })} +
    • + ))} +
+ )} +
{/* crop + precision */}