diff --git a/src/web/__tests__/fake-react.ts b/src/web/__tests__/fake-react.ts
index 2dfcbe108..a411590a2 100644
--- a/src/web/__tests__/fake-react.ts
+++ b/src/web/__tests__/fake-react.ts
@@ -111,10 +111,11 @@ export function mount
(
let tree: unknown = null;
let shown = props;
let running = false;
+ let gone = false;
const inst: Instance = {
slots: [], effects: [], queued: [], cursor: 0, effectCursor: 0, dirty: false,
update() {
- if (running) return;
+ if (running || gone) return;
running = true;
try {
let passes = 0;
@@ -144,6 +145,12 @@ export function mount
(
return {
get tree() { return tree; },
rerender(next: P = shown) { shown = next; inst.update(); },
+ /** Takes the component away, the way a dialog closing does: every
+ * effect's cleanup runs, and a state set afterwards draws nothing. */
+ unmount() {
+ gone = true;
+ for (const e of inst.effects) if (typeof e?.cleanup === "function") e.cleanup();
+ },
};
}
diff --git a/src/web/__tests__/feedback-dialog-form.test.ts b/src/web/__tests__/feedback-dialog-form.test.ts
index 9473f2d8c..04656b92a 100644
--- a/src/web/__tests__/feedback-dialog-form.test.ts
+++ b/src/web/__tests__/feedback-dialog-form.test.ts
@@ -297,7 +297,7 @@ describe("Send feedback is never greyed out, and never drops focus (#518)", () =
});
it("says aria-busy while the request is out, and refuses a second press itself", () => {
- expect(flatDialog).toMatch(/]*\{\.\.\.selfPressProps\(sending\)\}/);
+ expect(flatDialog).toMatch(/]*\{\.\.\.selfPressProps\(sending\)\}/);
expect(sendHook).toMatch(/if \(!selfPressAccepted\(sendingRef\.current\)\) return;/);
expect(flatDialog).toMatch(/Sending…<\/span>/);
});
diff --git a/src/web/__tests__/feedback-images.test.ts b/src/web/__tests__/feedback-images.test.ts
index 762cc7baf..fbb10585b 100644
--- a/src/web/__tests__/feedback-images.test.ts
+++ b/src/web/__tests__/feedback-images.test.ts
@@ -437,7 +437,7 @@ describe("the dialog takes an image three ways", () => {
it("keeps a file dropped beside the dialog from opening in place of the deck", () => {
// The browser's own answer to a file dropped on a page is to open it,
// which would throw away everything typed.
- expect(flatDialog).toMatch(//);
+ expect(flatDialog).toMatch(/
/);
expect(flatDialog).toMatch(/function refuseBesideDialog\(e: DragEvent\) \{ if \(e\.target !== e\.currentTarget \|\| !fileDrag\(e\)\) return; e\.preventDefault\(\); e\.dataTransfer\.dropEffect = "none"; \}/);
expect(flatDialog).toMatch(/function dropBesideDialog\(e: DragEvent\) \{ if \(fileDrag\(e\)\) e\.preventDefault\(\); \}/);
});
diff --git a/src/web/__tests__/feedback-kind-enter.test.ts b/src/web/__tests__/feedback-kind-enter.test.ts
new file mode 100644
index 000000000..4ee4b521c
--- /dev/null
+++ b/src/web/__tests__/feedback-kind-enter.test.ts
@@ -0,0 +1,47 @@
+// Enter on Bug, Idea or Other sent the report. The kinds are native radios
+// inside the form, and a browser submits a form implicitly on Enter in a
+// radio, so picking a kind with the arrows and pressing Enter to settle on it
+// posted the message there and then, before a screenshot or a contact could
+// be added. Send and the shortcut are the only two ways a report leaves; the
+// contact field already kept its bare Enter for that reason, and the kinds now
+// do the same.
+//
+// FeedbackKinds has no hooks, so it is called as it is and its radios' key
+// handlers are pressed with the keystrokes a browser would hand them.
+import { describe, expect, it, vi } from "vitest";
+
+import { all, type Drawn } from "./fake-react";
+import FeedbackKinds from "../components/FeedbackKinds";
+import { KINDS, type EnterKey } from "../feedback";
+
+const radios = () => all(FeedbackKinds({ kind: "bug", onChange: () => {} }), el => el.type === "input" && el.props.type === "radio");
+
+const key = (over: Partial
= {}): EnterKey =>
+ ({ key: "Enter", metaKey: false, ctrlKey: false, altKey: false, shiftKey: false, ...over });
+
+/** Presses `k` on `radio`; answers whether the browser's own answer — the
+ * implicit submit — was stopped. */
+function press(radio: Drawn, k: EnterKey): boolean {
+ const preventDefault = vi.fn();
+ (radio.props.onKeyDown as ((e: unknown) => void) | undefined)?.({ ...k, nativeEvent: k, preventDefault });
+ return preventDefault.mock.calls.length > 0;
+}
+
+describe("Enter on a kind of feedback", () => {
+ it("draws one radio per kind", () => {
+ expect(radios().map(r => r.props.value)).toEqual(KINDS.map(k => k.value));
+ });
+
+ it("does not send the report from any of them", () => {
+ for (const radio of radios()) expect(press(radio, key()), String(radio.props.value)).toBe(true);
+ });
+
+ it("leaves the send shortcut to the form, and an input method's Enter alone", () => {
+ for (const radio of radios()) {
+ expect(press(radio, key({ ctrlKey: true }))).toBe(false);
+ expect(press(radio, key({ metaKey: true }))).toBe(false);
+ expect(press(radio, key({ isComposing: true }))).toBe(false);
+ expect(press(radio, key({ key: "ArrowRight" }))).toBe(false);
+ }
+ });
+});
diff --git a/src/web/__tests__/feedback-refused-while-sending.test.ts b/src/web/__tests__/feedback-refused-while-sending.test.ts
new file mode 100644
index 000000000..3f040deef
--- /dev/null
+++ b/src/web/__tests__/feedback-refused-while-sending.test.ts
@@ -0,0 +1,113 @@
+// An image refused while Send waited for it was dropped, and the report went
+// without it. Send waits for every image still being redrawn to fit; one that
+// then turns out not to decode, or not to shrink far enough, is taken off the
+// strip and refused beside it — and the send posted what was left, said
+// "Thanks — feedback sent." over the refusal and closed the dialog, so the
+// person never learnt their screenshot had not gone. A refusal said while Send
+// waits now stops the send, and the dialog stays open saying why.
+//
+// Run on fake-react.ts's React: the images hook and the send hook together,
+// as the dialog holds them, with the fit of each image answered by the test.
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+
+import { flush, mount } from "./fake-react";
+import type { FeedbackDraft } from "../feedback";
+import type { Prepared } from "../feedback-images";
+
+vi.mock("react", async () => (await import("./fake-react")).react);
+
+/** Each fit under way, answered when the test says. The decoder is called
+ * first, which is the moment the hook shows the image as resizing. */
+let fits: Array<(prepared: Prepared) => void>;
+vi.mock("../feedback-images", async orig => ({
+ ...(await orig()),
+ browserDecoder: async () => null,
+ prepareImage: async (file: Blob, _budget: number, decode: (file: Blob) => Promise) => {
+ await decode(file);
+ return new Promise(resolve => fits.push(resolve));
+ },
+}));
+
+const { useFeedbackImages } = await import("../use-feedback-images");
+const { useFeedbackSend } = await import("../use-feedback-send");
+
+const DRAFT: FeedbackDraft = { kind: "bug", body: "Here is what the canvas looked like", contact: "" };
+const png = (name: string) => new File([new Uint8Array(64)], name, { type: "image/png" });
+const FITTED: Prepared = { ok: true, blob: new Blob([new Uint8Array(32)], { type: "image/png" }), resized: { width: 8000, height: 6000 } };
+const DAMAGED: Prepared = { ok: false, problem: "unreadable" };
+
+let posts: Array;
+
+beforeEach(() => {
+ fits = [];
+ posts = [];
+ vi.stubGlobal("fetch", vi.fn(async (_url: string, init: RequestInit) => {
+ posts.push(init);
+ return { ok: true, status: 200, json: async () => ({ ok: true }) };
+ }));
+});
+afterEach(() => { vi.unstubAllGlobals(); });
+
+function dialog() {
+ return mount(() => {
+ const images = useFeedbackImages();
+ return { images, ...useFeedbackSend(images) };
+ }, {});
+}
+
+/** One image pasted, and its fit begun: shown, still resizing. */
+async function pasteBig(view: ReturnType, name: string) {
+ view.tree.images.add([png(name)]);
+ await flush();
+}
+
+describe("an image refused while Send waits for it", () => {
+ it("stops the send, and says why, with the refusal still beside the images", async () => {
+ const view = dialog();
+ await pasteBig(view, "damaged.png");
+ expect(view.tree.images.shots).toHaveLength(1);
+ const sent = view.tree.send(DRAFT);
+ fits[0](DAMAGED);
+ await sent;
+ expect(posts).toHaveLength(0);
+ expect(view.tree.outcome.state).toBe("failed");
+ expect(view.tree.outcome.state === "failed" && view.tree.outcome.message).toMatch(/image/i);
+ expect(view.tree.images.problem).toMatch(/“damaged\.png” could not be read as an image/);
+ expect(view.tree.images.shots).toHaveLength(0);
+ });
+
+ it("sends once it is pressed again, with what is there now", async () => {
+ const view = dialog();
+ await pasteBig(view, "damaged.png");
+ const first = view.tree.send(DRAFT);
+ fits[0](DAMAGED);
+ await first;
+ await view.tree.send(DRAFT);
+ expect(posts).toHaveLength(1);
+ expect(view.tree.outcome).toEqual({ state: "sent" });
+ });
+
+ it("does not stop for a refusal already said before Send was pressed", async () => {
+ // That one was on screen when the person chose to send.
+ const view = dialog();
+ await pasteBig(view, "damaged.png");
+ fits[0](DAMAGED);
+ await flush();
+ expect(view.tree.images.problem).not.toBe("");
+ await view.tree.send(DRAFT);
+ expect(posts).toHaveLength(1);
+ expect(view.tree.outcome).toEqual({ state: "sent" });
+ });
+
+ it("sends an image that fits while Send waits for it", async () => {
+ const view = dialog();
+ await pasteBig(view, "canvas.png");
+ const sent = view.tree.send(DRAFT);
+ fits[0](FITTED);
+ await sent;
+ expect(posts).toHaveLength(1);
+ expect(posts[0].body).toBeInstanceOf(FormData);
+ expect((posts[0].body as FormData).getAll("images")).toHaveLength(1);
+ expect(view.tree.outcome).toEqual({ state: "sent" });
+ });
+});
diff --git a/src/web/__tests__/feedback-scrim-dismiss.test.ts b/src/web/__tests__/feedback-scrim-dismiss.test.ts
new file mode 100644
index 000000000..74a5c892a
--- /dev/null
+++ b/src/web/__tests__/feedback-scrim-dismiss.test.ts
@@ -0,0 +1,83 @@
+// The feedback dialog closed under a text selection and took the report with
+// it. A click goes to the nearest element its press and its release share, so
+// a drag that selects part of the message and is let go past the dialog's
+// edge, over the scrim, arrives as a click whose target is the backdrop — the
+// dialog's own stopPropagation is not on that path — and the backdrop closed
+// on any click. The message, the contact and every screenshot were state in
+// the dialog, and went with it.
+//
+// Only a press that starts on the scrim and ends there closes it now. Run on
+// fake-react.ts's React: the backdrop's handlers are called with the targets
+// the browser would hand them.
+import { beforeEach, afterEach, describe, expect, it, vi } from "vitest";
+
+import { mount, one, type Drawn } from "./fake-react";
+
+vi.mock("react", async () => (await import("./fake-react")).react);
+vi.mock("../components/use-modal-dismiss", async orig => ({
+ ...(await orig()),
+ useModalDismiss: () => ({ current: null }),
+}));
+// Built with forwardRef when it loads; the thanks is not what is under test.
+vi.mock("../components/SuccessMark", () => ({ default: () => null }));
+
+const { default: FeedbackDialog } = await import("../components/FeedbackDialog");
+
+beforeEach(() => {
+ // The facts the dialog asks for on open; never answered, which it allows.
+ vi.stubGlobal("fetch", vi.fn(() => new Promise(() => {})));
+});
+afterEach(() => { vi.unstubAllGlobals(); });
+
+const scrim = (tree: unknown) => one(tree, el => el.props.className === "modal-backdrop") as Drawn;
+
+/** Stand-ins for the nodes a press can land on: the scrim itself, and the
+ * message field inside the dialog. */
+const SCRIM = { node: "scrim" };
+const FIELD = { node: "message field" };
+
+/** One mouse press, as the backdrop sees it: down on one node, up on
+ * another, and the click the browser then sends to the nearest node the two
+ * share — the scrim whenever either end was on it, since the dialog is
+ * inside it. Handlers the backdrop does not have are skipped. */
+function press(tree: unknown, down: object, up: object) {
+ const call = (name: string, target: object) =>
+ (scrim(tree).props[name] as ((e: unknown) => void) | undefined)?.({
+ target, currentTarget: SCRIM, stopPropagation() {}, preventDefault() {},
+ });
+ call("onPointerDown", down);
+ call("onPointerUp", up);
+ call("onClick", down === FIELD && up === FIELD ? FIELD : SCRIM);
+}
+
+describe("the feedback dialog's scrim", () => {
+ it("stays open when a selection begun in the message is let go over the scrim", () => {
+ const onClose = vi.fn();
+ const view = mount(FeedbackDialog, { onClose });
+ press(view.tree, FIELD, SCRIM);
+ expect(onClose).not.toHaveBeenCalled();
+ });
+
+ it("stays open when a press begun on the scrim is let go inside the dialog", () => {
+ // Moving off a control before letting go is how a press is taken back.
+ const onClose = vi.fn();
+ const view = mount(FeedbackDialog, { onClose });
+ press(view.tree, SCRIM, FIELD);
+ expect(onClose).not.toHaveBeenCalled();
+ });
+
+ it("closes on a press that starts and ends on the scrim", () => {
+ const onClose = vi.fn();
+ const view = mount(FeedbackDialog, { onClose });
+ press(view.tree, SCRIM, SCRIM);
+ expect(onClose).toHaveBeenCalledTimes(1);
+ });
+
+ it("closes on the next scrim press after a selection that did not", () => {
+ const onClose = vi.fn();
+ const view = mount(FeedbackDialog, { onClose });
+ press(view.tree, FIELD, SCRIM);
+ press(view.tree, SCRIM, SCRIM);
+ expect(onClose).toHaveBeenCalledTimes(1);
+ });
+});
diff --git a/src/web/__tests__/feedback-send-cancel.test.ts b/src/web/__tests__/feedback-send-cancel.test.ts
new file mode 100644
index 000000000..65a4c5581
--- /dev/null
+++ b/src/web/__tests__/feedback-send-cancel.test.ts
@@ -0,0 +1,84 @@
+// Cancel, Esc or the × pressed while a report said "Sending…" did not cancel
+// it. Closing only took the dialog away; the send went on awaiting the images
+// and then posted, so a report the person had just cancelled — screenshots
+// and all — could start uploading after the dialog was gone, and a failure of
+// that post was told to nobody. Closed now, a send not yet posted is never
+// posted and one under way is aborted.
+//
+// Run on fake-react.ts's React: the hook is mounted, the images it waits for
+// and the fetch it makes are held by the test, and unmount is the dialog
+// closing.
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+
+import { flush, mount } from "./fake-react";
+import type { FeedbackImages } from "../use-feedback-images";
+import type { FeedbackDraft } from "../feedback";
+
+vi.mock("react", async () => (await import("./fake-react")).react);
+
+const { useFeedbackSend } = await import("../use-feedback-send");
+
+const DRAFT: FeedbackDraft = { kind: "bug", body: "The usage chart is empty after a restart", contact: "" };
+
+/** Images whose fits finish when the test says. */
+let finishFits: (blobs: Blob[]) => void;
+const images = { ready: () => new Promise(resolve => { finishFits = resolve; }) } as unknown as FeedbackImages;
+
+/** Every post made, with what it was handed. Each one is answered only when
+ * the test says, and fails the way fetch does when its signal is aborted. */
+let posts: Array<{ init: RequestInit; answer: (ok: boolean) => void }>;
+
+beforeEach(() => {
+ posts = [];
+ vi.stubGlobal("fetch", vi.fn((_url: string, init: RequestInit) => new Promise((resolve, reject) => {
+ init.signal?.addEventListener("abort", () => reject(new DOMException("The operation was aborted.", "AbortError")));
+ posts.push({ init, answer: ok => resolve({ ok, status: ok ? 200 : 502, json: async () => ({ ok }) }) });
+ })));
+});
+afterEach(() => { vi.unstubAllGlobals(); });
+
+function open() {
+ return mount(() => useFeedbackSend(images), {});
+}
+
+describe("closing the feedback dialog while a report is sending", () => {
+ it("posts nothing when it closes while the images are still being made ready", async () => {
+ const view = open();
+ void view.tree.send(DRAFT);
+ view.unmount();
+ finishFits([]);
+ await flush();
+ expect(posts).toHaveLength(0);
+ });
+
+ it("aborts the post when it closes while the post is out", async () => {
+ const view = open();
+ void view.tree.send(DRAFT);
+ finishFits([]);
+ await flush();
+ expect(posts).toHaveLength(1);
+ view.unmount();
+ expect(posts[0].init.signal?.aborted).toBe(true);
+ });
+
+ it("still sends, and thanks, when nobody closes it", async () => {
+ const view = open();
+ const sent = view.tree.send(DRAFT);
+ finishFits([]);
+ await flush();
+ expect(posts).toHaveLength(1);
+ posts[0].answer(true);
+ await sent;
+ expect(view.tree.outcome).toEqual({ state: "sent" });
+ });
+
+ it("still says a failure that was not a close", async () => {
+ const view = open();
+ const sent = view.tree.send(DRAFT);
+ finishFits([]);
+ await flush();
+ posts[0].answer(false);
+ await sent;
+ expect(view.tree.outcome.state).toBe("failed");
+ });
+});
diff --git a/src/web/__tests__/feedback-sending-lock.test.ts b/src/web/__tests__/feedback-sending-lock.test.ts
new file mode 100644
index 000000000..c01a17d21
--- /dev/null
+++ b/src/web/__tests__/feedback-sending-lock.test.ts
@@ -0,0 +1,173 @@
+// While a report was sending, the dialog stayed as editable as before it. The
+// request is built once, from the message and the images as they were when
+// Send was pressed, but a paste or a drop still attached an image, the strip's
+// remove and replace still worked and the message still took typing. So a
+// screenshot removed mid-send — say, one that turned out to show an address —
+// was said to be removed and went anyway, one pasted was said to be added and
+// did not go, and words typed meanwhile were lost when the dialog thanked and
+// closed. Everything above Cancel and Send is out of reach now until the
+// answer, and a paste or a drop is not taken.
+//
+// Run on fake-react.ts's React, with the images held by the test so the send
+// stays out for as long as it needs, and a document of stand-in nodes whose
+// focus and inert flags are what the test reads.
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+
+import { flush, mount, one, type Drawn } from "./fake-react";
+import type { FeedbackImages } from "../use-feedback-images";
+
+vi.mock("react", async () => (await import("./fake-react")).react);
+vi.mock("../components/use-modal-dismiss", async orig => ({
+ ...(await orig()),
+ useModalDismiss: () => ({ current: null }),
+}));
+vi.mock("../components/SuccessMark", () => ({ default: () => null }));
+
+/** The dialog's images: nothing attached, ready when the test says, and every
+ * change asked of them counted. */
+let finishFits: (blobs: Blob[]) => void;
+const images = {
+ shots: [], problem: "", announcement: "",
+ add: vi.fn(), replace: vi.fn(), remove: vi.fn(), sayFull: vi.fn(),
+ ready: () => new Promise(resolve => { finishFits = resolve; }),
+};
+vi.mock("../use-feedback-images", () => ({ useFeedbackImages: (): FeedbackImages => images as unknown as FeedbackImages }));
+
+const { default: FeedbackDialog } = await import("../components/FeedbackDialog");
+
+/** A node the document holds: where it sits, whether it is inert, focus. */
+interface Node { tagName: string; inside: Node[]; inert?: boolean; focus(): void; contains(n: unknown): boolean }
+let page: { activeElement: Node | null };
+const node = (tagName: string, inside: Node[] = []): Node => {
+ const n: Node = {
+ tagName, inside,
+ focus: () => { page.activeElement = n; },
+ contains: other => other === n || n.inside.some(c => c.contains(other)),
+ };
+ return n;
+};
+const field = node("TEXTAREA");
+const send = node("BUTTON");
+const fields = node("SECTION", [field]);
+const form = node("FORM", [fields, send]);
+const BODY = node("BODY");
+
+/** Each element the dialog draws with a ref, and the node the document gave it. */
+const NODES: Array<[(el: Drawn) => boolean, Node]> = [
+ [el => el.type === "form", form],
+ [el => el.type === "section", fields],
+ [el => el.type === "textarea", field],
+ [el => el.type === "button" && el.props.type === "submit", send],
+];
+
+/** What the document does with a render: refs get their nodes. */
+function commit(tree: unknown) {
+ for (const [match, n] of NODES) {
+ const el = one(tree, match);
+ if (el?.ref && typeof el.ref === "object") (el.ref as { current: unknown }).current = n;
+ }
+}
+
+let posts: Array<(ok: boolean) => void>;
+
+beforeEach(() => {
+ page = { activeElement: BODY };
+ for (const n of [form, fields, field, send]) n.inert = false;
+ posts = [];
+ images.add.mockClear();
+ vi.stubGlobal("document", page);
+ vi.stubGlobal("HTMLTextAreaElement", class {});
+ vi.stubGlobal("HTMLInputElement", class {});
+ vi.stubGlobal("fetch", vi.fn((_url: string, init?: RequestInit) => new Promise(resolve => {
+ // The facts the dialog asks for when it opens are never answered.
+ if (init?.method !== "POST") return;
+ posts.push(ok => resolve({ ok, status: ok ? 200 : 502, json: async () => ({ ok }) }));
+ })));
+});
+afterEach(() => { vi.unstubAllGlobals(); });
+
+const el = (tree: unknown, match: (e: Drawn) => boolean) => one(tree, match) as Drawn;
+const handler = (e: Drawn, name: string) => e.props[name] as (ev: unknown) => void;
+
+/** The dialog with a message written and Send pressed from the message with
+ * the shortcut; the report is out until the test finishes the fits. */
+function sending() {
+ const view = mount(FeedbackDialog, { onClose: vi.fn() }, { commit });
+ handler(el(view.tree, e => e.type === "textarea"), "onChange")({ target: { value: "The usage chart is empty" } });
+ field.focus();
+ handler(el(view.tree, e => e.type === "form"), "onSubmit")({ preventDefault() {} });
+ return view;
+}
+
+const surface = (tree: unknown) => el(tree, e => typeof e.props.className === "string" && e.props.className.startsWith("modal feedback-dialog"));
+const SHOT = { name: "shot.png" };
+
+describe("the feedback dialog while a report is sending", () => {
+ it("takes no image pasted into it", () => {
+ const view = sending();
+ const preventDefault = vi.fn();
+ handler(surface(view.tree), "onPaste")({
+ clipboardData: { files: [SHOT], types: ["Files"] }, target: {}, preventDefault,
+ });
+ expect(images.add).not.toHaveBeenCalled();
+ });
+
+ it("takes no image dropped on it", () => {
+ const view = sending();
+ const dataTransfer = { types: ["Files"], files: [SHOT], dropEffect: "copy" };
+ handler(surface(view.tree), "onDragEnter")({ dataTransfer, preventDefault() {} });
+ handler(surface(view.tree), "onDrop")({ dataTransfer, preventDefault() {} });
+ expect(images.add).not.toHaveBeenCalled();
+ });
+
+ it("puts the message, the kinds and the images out of reach, and keeps Cancel and Send", () => {
+ sending();
+ expect(fields.inert).toBe(true);
+ expect(form.inert).toBe(false);
+ });
+
+ it("moves focus from the message to Send, which says it is working", () => {
+ sending();
+ expect(page.activeElement).toBe(send);
+ });
+
+ it("gives everything back, focus included, when the send fails", async () => {
+ const view = sending();
+ finishFits([]);
+ await flush();
+ posts[0](false);
+ await flush();
+ expect(fields.inert).toBe(false);
+ expect(page.activeElement).toBe(field);
+ handler(surface(view.tree), "onPaste")({ clipboardData: { files: [SHOT], types: ["Files"] }, target: {}, preventDefault() {} });
+ expect(images.add).toHaveBeenCalledTimes(1);
+ });
+
+ it("gives focus back too when a press on the inert part dropped it meanwhile", async () => {
+ // A click on anything inert lands on nothing that takes focus.
+ sending();
+ BODY.focus();
+ finishFits([]);
+ await flush();
+ posts[0](false);
+ await flush();
+ expect(page.activeElement).toBe(field);
+ });
+
+ it("leaves focus where the reader put it meanwhile", async () => {
+ sending();
+ const cancel = node("BUTTON");
+ cancel.focus();
+ finishFits([]);
+ await flush();
+ posts[0](false);
+ await flush();
+ expect(page.activeElement).toBe(cancel);
+ });
+
+ it("took an image before Send was pressed", () => {
+ const view = mount(FeedbackDialog, { onClose: vi.fn() }, { commit });
+ handler(surface(view.tree), "onPaste")({ clipboardData: { files: [SHOT], types: ["Files"] }, target: {}, preventDefault() {} });
+ expect(images.add).toHaveBeenCalledTimes(1);
+ });
+});
diff --git a/src/web/components/FeedbackDialog.tsx b/src/web/components/FeedbackDialog.tsx
index 9729cc844..16c54137e 100644
--- a/src/web/components/FeedbackDialog.tsx
+++ b/src/web/components/FeedbackDialog.tsx
@@ -21,17 +21,18 @@
//
// SEND IS NEVER GREYED OUT BEFORE IT IS PRESSED. Pressed with nothing written
// it says so beside the message and moves there; while the request is out it
-// stays focusable and says aria-busy (#518). Once sent, the form goes inert
-// under a thanks, focus goes to the × (#1762's rule), the one live
-// region — present from the first render — says it was sent, and the dialog
-// closes itself a moment later.
+// stays focusable and says aria-busy (#518), and everything above it and
+// Cancel is inert, since the request was built from it when Send was pressed.
+// Once sent, the form goes inert under a thanks, focus goes to the × (#1762's
+// rule), the one live region — present from the first render — says it was
+// sent, and the dialog closes itself a moment later.
//
// AN IMAGE ARRIVES THE WAY A SCREENSHOT DOES — pasted, the common way, or
// dropped anywhere on the dialog, which says so while a file is over it — or
// through the picker. What is checked and redrawn before it goes is
// feedback-images.ts's. With no image, Send posts the JSON it always did.
import { useEffect, useRef, useState, type ClipboardEvent, type DragEvent, type FormEvent } from "react";
-import { useModalDismiss } from "./use-modal-dismiss";
+import { useModalDismiss, useScrimDismiss } from "./use-modal-dismiss";
import { focusDropped, selfPressProps } from "../panel-press";
import SuccessMark from "./SuccessMark";
import FeedbackKinds from "./FeedbackKinds";
@@ -61,8 +62,14 @@ export default function FeedbackDialog({ onClose, initialKind, initialBody }: Pr
const closeRef = useRef(null);
const addRef = useRef(null);
const formRef = useRef(null);
+ const fieldsRef = useRef(null);
+ const sendRef = useRef(null);
+ const lockedFrom = useRef(null);
const dragDepth = useRef(0);
const dialogRef = useModalDismiss(onClose, { focusRef: bodyRef });
+ // Only a press of the scrim itself: a selection dragged out of the message
+ // and let go over it would otherwise close the dialog and lose the report.
+ const scrimPress = useScrimDismiss(onClose);
const [kind, setKind] = useState(initialKind ?? "bug");
const [body, setBody] = useState(initialBody ?? "");
const [contact, setContact] = useState("");
@@ -90,6 +97,32 @@ export default function FeedbackDialog({ onClose, initialKind, initialBody }: Pr
if (sent && held) closeRef.current?.focus();
}, [sent]);
+ // Sending, the message, the kinds, the images and the details go inert
+ // until the answer. The request was built from them when Send was pressed,
+ // so an edit made now would not be in it: a screenshot removed would go
+ // anyway, one pasted would not, and words typed would close with the
+ // dialog. Cancel and Send stay live. Focus in there would go down with it,
+ // so it moves to Send, which says it is working, and comes back if the send
+ // fails, to be put right where it was left — from Send, or from nowhere,
+ // when a press on the inert part dropped it; never from where the reader
+ // has since put it.
+ useEffect(() => {
+ const fields = fieldsRef.current;
+ if (!fields) return;
+ if (sending) {
+ const active = document.activeElement as HTMLElement | null;
+ lockedFrom.current = active && fields.contains(active) ? active : null;
+ fields.inert = true;
+ if (lockedFrom.current) sendRef.current?.focus();
+ return;
+ }
+ fields.inert = false;
+ const back = lockedFrom.current;
+ lockedFrom.current = null;
+ const waiting = document.activeElement === sendRef.current || focusDropped(document.activeElement?.tagName ?? null);
+ if (!sent && back && waiting) back.focus();
+ }, [sending, sent]);
+
function submit(event: FormEvent) {
event.preventDefault();
if (!hasMessage(body)) {
@@ -100,10 +133,11 @@ export default function FeedbackDialog({ onClose, initialKind, initialBody }: Pr
void send({ kind, body, contact });
}
- /** The whole dialog takes a dropped file while the form is up. The depth
- * counts enters against leaves, since the pointer crossing into a child is
- * a leave from its parent and would otherwise flicker the overlay off. */
- const accepting = !sent;
+ /** The whole dialog takes a dropped file while the form is up and no send
+ * is out. The depth counts enters against leaves, since the pointer
+ * crossing into a child is a leave from its parent and would otherwise
+ * flicker the overlay off. */
+ const accepting = !sent && !sending;
const fileDrag = (e: DragEvent) => carriesFiles(Array.from(e.dataTransfer.types));
function dragEnter(e: DragEvent) {
if (!fileDrag(e) || !accepting) return;
@@ -152,7 +186,7 @@ export default function FeedbackDialog({ onClose, initialKind, initialBody }: Pr
}
return (
-
-
+
{copy.question}
@@ -225,7 +259,7 @@ export default function FeedbackDialog({ onClose, initialKind, initialBody }: Pr
{capA} {capB} to send
Cancel
-
+
{/* Both words always laid out in one cell, one of them hidden,
so the button is as wide sending as before and Cancel never
shifts under the pointer. */}
diff --git a/src/web/components/FeedbackKinds.tsx b/src/web/components/FeedbackKinds.tsx
index f8f145a91..0f2f19bea 100644
--- a/src/web/components/FeedbackKinds.tsx
+++ b/src/web/components/FeedbackKinds.tsx
@@ -6,8 +6,11 @@
// difference matters.
//
// Native radios under the paint, so the arrows walk them, Tab stops once and a
-// screen reader hears a group of three; each is the target.
-import { KINDS, type Kind } from "../feedback";
+// screen reader hears a group of three; each is the target. Being in
+// the form, a radio would submit it on a bare Enter, which is how a kind
+// picked with the arrows and settled with Enter sent the report on the spot;
+// that Enter does nothing here, as in the contact field (isPlainEnter).
+import { KINDS, isPlainEnter, type Kind } from "../feedback";
interface Props {
kind: Kind;
@@ -26,6 +29,7 @@ export default function FeedbackKinds({ kind, onChange }: Props) {
value={option.value}
checked={kind === option.value}
onChange={() => onChange(option.value)}
+ onKeyDown={e => { if (isPlainEnter(e.nativeEvent)) e.preventDefault(); }}
/>
{option.label}
diff --git a/src/web/components/use-modal-dismiss.ts b/src/web/components/use-modal-dismiss.ts
index 287988738..19e6a2521 100644
--- a/src/web/components/use-modal-dismiss.ts
+++ b/src/web/components/use-modal-dismiss.ts
@@ -170,3 +170,33 @@ export function useModalDismiss(
return dialogRef;
}
+
+/** Where a press on a backdrop landed, read structurally so a test can hand
+ * in plain objects; React's pointer and mouse events both satisfy it. */
+interface ScrimPress {
+ target: EventTarget | null;
+ currentTarget: EventTarget | null;
+}
+
+/** The handlers a backdrop spreads to close its dialog on a press of the
+ * scrim — one that goes down on the scrim itself and comes up there.
+ *
+ * A click goes to the nearest element its press and its release share. A
+ * selection begun in a field and let go past the dialog's edge therefore
+ * arrives as a click whose target is the backdrop, and the dialog's own
+ * stopPropagation is not on that path: a backdrop closing on any click
+ * closed the feedback dialog under a drag-select and threw away the message
+ * and its screenshots. So the press is followed from the pointer going down
+ * to it coming up, and the click closes only when both were on the scrim. */
+export function useScrimDismiss(onDismiss: () => void) {
+ const fromScrim = useRef(false);
+ return {
+ onPointerDown: (e: ScrimPress) => { fromScrim.current = e.target === e.currentTarget; },
+ onPointerUp: (e: ScrimPress) => { if (e.target !== e.currentTarget) fromScrim.current = false; },
+ onClick: (e: ScrimPress) => {
+ const close = fromScrim.current && e.target === e.currentTarget;
+ fromScrim.current = false;
+ if (close) onDismiss();
+ },
+ };
+}
diff --git a/src/web/feedback-images.ts b/src/web/feedback-images.ts
index 9e634d3da..3331fe951 100644
--- a/src/web/feedback-images.ts
+++ b/src/web/feedback-images.ts
@@ -248,6 +248,8 @@ export const ADD_LABEL = "Add screenshot";
export const ADD_HINT = "PNG or JPEG, up to three. You can also paste one, or drop it on this dialog.";
export const SHOTS_NOTE = "A screenshot can show emails, costs and paths. Crop out what should not be seen.";
export const FULL_MESSAGE = "Three images at most. Remove one to add another.";
+/** Why a send stopped: an image was refused while it waited for the fits. */
+export const REFUSED_WHILE_SENDING = "An image could not be attached, so nothing was sent. Check the images and send again; your text is still here.";
/** What a thumbnail says it does, and which image it is: pressed, it opens the
* picker and the file chosen takes this image's place. A redrawn image says
diff --git a/src/web/use-feedback-images.ts b/src/web/use-feedback-images.ts
index d3af757ce..16b9becbc 100644
--- a/src/web/use-feedback-images.ts
+++ b/src/web/use-feedback-images.ts
@@ -7,7 +7,9 @@
// the request's 12 MB and a fit still in progress has no size yet. A drop of
// three large screenshots therefore fits them in turn, and a fourth is left out
// by count rather than by a race. Send waits on the same chain, so what it
-// posts is the images as they will go, never one half-drawn.
+// posts is the images as they will go, never one half-drawn — and when one of
+// them is refused while it waits, it posts nothing: the refusal would be said
+// beside the images just as the thanks covered them and the dialog closed.
//
// An image inside every limit is shown once its header has been read, which
// is at once. One that has to be drawn again is shown the moment the fit
@@ -52,8 +54,9 @@ export interface FeedbackImages {
remove(id: number): void;
/** Says the three are already there, for an add pressed with no room. */
sayFull(): void;
- /** Every image as it will be sent, once the fits under way have finished. */
- ready(): Promise;
+ /** Every image as it will be sent, once the fits under way have finished;
+ * null when something was refused meanwhile, which `problem` says. */
+ ready(): Promise;
}
/** The list with `fresh` drawn in it: in the place of the image it replaces,
@@ -73,9 +76,12 @@ export function useFeedbackImages(): FeedbackImages {
// Each refusal counted as it is said. The words alone cannot tell a second
// refusal from the first: with no room left an add awaits nothing, so its
// clear and its refusal land in one render with the words unchanged, and
- // sayFull clears nothing at all.
+ // sayFull clears nothing at all. The count is kept in a ref as well, so
+ // `ready` can tell a refusal said while it waited.
const [refusal, setRefusal] = useState({ problem: "", id: 0 });
+ const refusals = useRef(0);
const setProblem = useCallback((problem: string) => {
+ if (problem) refusals.current++;
setRefusal(prev => {
if (problem) return { problem, id: prev.id + 1 };
return prev.problem ? { problem: "", id: prev.id } : prev;
@@ -198,7 +204,9 @@ export function useFeedbackImages(): FeedbackImages {
const sayFull = useCallback(() => setProblem(FULL_MESSAGE), [setProblem]);
const ready = useCallback(async () => {
+ const said = refusals.current;
await queue.current;
+ if (refusals.current !== said) return null;
return shotsRef.current.flatMap(shot => (shot.blob ? [shot.blob] : []));
}, []);
diff --git a/src/web/use-feedback-send.ts b/src/web/use-feedback-send.ts
index fc0e5d946..902bc1a70 100644
--- a/src/web/use-feedback-send.ts
+++ b/src/web/use-feedback-send.ts
@@ -12,9 +12,16 @@
// fades out over SENT_EXIT_MS and closes itself, the way a toast leaves: the
// person pressed Send to be done, and a Close button to press afterwards was
// one more thing to do. Escape, the × and the scrim still close it at once.
+//
+// CLOSED WHILE SENDING, IT DOES NOT SEND. Cancel, Escape, the × or the scrim
+// pressed while a report is out is the person taking it back: a send still
+// waiting on its images never posts, one under way is aborted, and neither
+// says anything to a dialog that is gone. Without that the report, images and
+// all, could start uploading after the dialog had closed, and a failure of it
+// was told to nobody.
import { useEffect, useRef, useState } from "react";
import { selfPressAccepted } from "./panel-press";
-import { feedbackRequest } from "./feedback-images";
+import { REFUSED_WHILE_SENDING, feedbackRequest } from "./feedback-images";
import {
SENT_EXIT_MS, SENT_HOLD_MS, feedbackFailure, fieldsToSend, type FeedbackDraft, type Outcome,
} from "./feedback";
@@ -29,20 +36,30 @@ export interface FeedbackSend {
export function useFeedbackSend(images: FeedbackImages): FeedbackSend {
const [outcome, setOutcome] = useState({ state: "idle" });
const sendingRef = useRef(false);
+ const inflight = useRef(null);
+ useEffect(() => () => inflight.current?.abort(), []);
async function send(draft: FeedbackDraft) {
if (!selfPressAccepted(sendingRef.current)) return;
sendingRef.current = true;
+ const { signal } = (inflight.current = new AbortController());
setOutcome({ state: "sending" });
try {
const attached = await images.ready();
- const response = await fetch("/api/feedback", feedbackRequest(fieldsToSend(draft), attached));
+ if (signal.aborted) return;
+ // An image refused while this waited: what would go is not what the
+ // person pressed Send on, and the thanks would cover the refusal.
+ if (!attached) {
+ setOutcome({ state: "failed", message: REFUSED_WHILE_SENDING });
+ return;
+ }
+ const response = await fetch("/api/feedback", { ...feedbackRequest(fieldsToSend(draft), attached), signal });
const d = await response.json().catch(() => null);
setOutcome(response.ok && d?.ok
? { state: "sent" }
: { state: "failed", message: feedbackFailure(response.status, d?.reason, d?.errors) });
} catch {
- setOutcome({ state: "failed", message: feedbackFailure(0, null) });
+ if (!signal.aborted) setOutcome({ state: "failed", message: feedbackFailure(0, null) });
} finally {
sendingRef.current = false;
}