fix(frontend): the scheduler's two number editors stop swallowing what you typed
Both refused an out-of-range value by discarding it: the entry vanished, the box snapped back to the server's number, and nothing said whether the save had happened, been rejected, or been ignored. That is the house rule about honest states, applied to a write instead of a read. They now keep the draft, mark the field aria-invalid and name the range (HU+EN), and the Save control is disabled rather than silently inert. The interval editor also dropped a keyboard user's focus on every exit — Escape, Cancel and Save all unmount the focused input — so it hands focus back to the pencil that opened it, the pattern IssueComposer already uses. Tests: the four interval-editor cases added in S2 are relabelled CHARACTERIZATION, because they pass against the pre-sweep code too (the old effect was already guarded by `editing`). The editor that genuinely lost typing to a poll is the maintenance batch box, whose old effect had no guard at all, and it had no such test — it does now, anchored on a value that follows the server so it cannot pass on a page that simply has not polled yet.
This commit is contained in:
@@ -80,6 +80,11 @@ afterEach(() => {
|
|||||||
const intervalBox = () => screen.queryByRole("spinbutton", { name: "" });
|
const intervalBox = () => screen.queryByRole("spinbutton", { name: "" });
|
||||||
const batchBox = () => screen.getByLabelText("scheduler.maintenance.batchLabel");
|
const batchBox = () => screen.getByLabelText("scheduler.maintenance.batchLabel");
|
||||||
|
|
||||||
|
// The four tests below are CHARACTERIZATION, not regression guards for E27 S2: the old JobRow's
|
||||||
|
// re-sync effect was already guarded by `if (!editing)` and the mirror only rendered while editing,
|
||||||
|
// so every one of them passes against the pre-sweep code too. They are here because the editor had
|
||||||
|
// no tests at all. The editor that genuinely lost the user's typing to a poll is the maintenance
|
||||||
|
// batch box below — its old effect had no guard — and that one has its own test.
|
||||||
describe("Scheduler · the job interval editor", () => {
|
describe("Scheduler · the job interval editor", () => {
|
||||||
it("opens on the current interval and commits a new one", async () => {
|
it("opens on the current interval and commits a new one", async () => {
|
||||||
const update = vi.spyOn(api, "updateSchedulerJob").mockResolvedValue(undefined as never);
|
const update = vi.spyOn(api, "updateSchedulerJob").mockResolvedValue(undefined as never);
|
||||||
@@ -129,9 +134,77 @@ describe("Scheduler · the job interval editor", () => {
|
|||||||
fireEvent.click(screen.getByRole("button", { name: /scheduler.everyMin/ }));
|
fireEvent.click(screen.getByRole("button", { name: /scheduler.everyMin/ }));
|
||||||
expect(intervalBox()).toHaveProperty("value", "90");
|
expect(intervalBox()).toHaveProperty("value", "90");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("refuses a value outside the range instead of silently dropping it", async () => {
|
||||||
|
const update = vi.spyOn(api, "updateSchedulerJob").mockResolvedValue(undefined as never);
|
||||||
|
await mount();
|
||||||
|
|
||||||
|
fireEvent.click(screen.getByRole("button", { name: /scheduler.everyMin/ }));
|
||||||
|
fireEvent.change(intervalBox()!, { target: { value: "99999" } });
|
||||||
|
fireEvent.keyDown(intervalBox()!, { key: "Enter" });
|
||||||
|
|
||||||
|
expect(update).not.toHaveBeenCalled();
|
||||||
|
// Still open, still showing what was typed, and saying why.
|
||||||
|
expect(intervalBox()).toHaveProperty("value", "99999");
|
||||||
|
expect(intervalBox()!.getAttribute("aria-invalid")).toBe("true");
|
||||||
|
expect(screen.getByRole("alert").textContent).toBe("scheduler.range");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("hands focus back to the trigger when the editor closes", async () => {
|
||||||
|
await mount();
|
||||||
|
const trigger = screen.getByRole("button", { name: /scheduler.everyMin/ });
|
||||||
|
fireEvent.click(trigger);
|
||||||
|
expect(document.activeElement).toBe(intervalBox());
|
||||||
|
|
||||||
|
fireEvent.keyDown(intervalBox()!, { key: "Escape" });
|
||||||
|
await waitFor(() =>
|
||||||
|
expect(document.activeElement).toBe(
|
||||||
|
screen.getByRole("button", { name: /scheduler.everyMin/ }),
|
||||||
|
),
|
||||||
|
);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe("Scheduler · the maintenance batch editor", () => {
|
describe("Scheduler · the maintenance batch editor", () => {
|
||||||
|
it("keeps what you are typing when a poll changes the batch underneath", async () => {
|
||||||
|
// THE regression this editor needed: its pre-E27 effect re-synced on every
|
||||||
|
// `revalidate_batch` change with no editing guard, so a poll landing mid-keystroke replaced
|
||||||
|
// what the admin had typed.
|
||||||
|
await mount();
|
||||||
|
fireEvent.change(batchBox(), { target: { value: "9000" } });
|
||||||
|
|
||||||
|
// The poll carries a new interval TOO, and the interval label follows the server unconditionally
|
||||||
|
// — so waiting for it is how this test knows the new status has actually been rendered. Without
|
||||||
|
// that anchor the assertion below passes on a page that simply has not polled yet.
|
||||||
|
await poll(
|
||||||
|
status({
|
||||||
|
jobs: [{ ...status().jobs[0]!, interval_minutes: 90 }],
|
||||||
|
maintenance: { ...status().maintenance, revalidate_batch: 12000 },
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
await waitFor(() =>
|
||||||
|
expect(screen.getByRole("button", { name: /scheduler.everyMin/ })).toBeTruthy(),
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(batchBox()).toHaveProperty("value", "9000");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("refuses a value outside min..max instead of silently dropping it", async () => {
|
||||||
|
const save = vi.spyOn(api, "updateMaintenanceBatch").mockResolvedValue({
|
||||||
|
revalidate_batch: 5000,
|
||||||
|
});
|
||||||
|
await mount();
|
||||||
|
|
||||||
|
fireEvent.change(batchBox(), { target: { value: "10" } }); // below m.min = 1000
|
||||||
|
fireEvent.keyDown(batchBox(), { key: "Enter" });
|
||||||
|
|
||||||
|
expect(save).not.toHaveBeenCalled();
|
||||||
|
expect(batchBox()).toHaveProperty("value", "10");
|
||||||
|
expect(batchBox().getAttribute("aria-invalid")).toBe("true");
|
||||||
|
expect(screen.getByRole("alert").textContent).toBe("scheduler.range");
|
||||||
|
expect(screen.getByRole("button", { name: "common.save" })).toHaveProperty("disabled", true);
|
||||||
|
});
|
||||||
|
|
||||||
it("is not dirty until edited, and stops being dirty once saved", async () => {
|
it("is not dirty until edited, and stops being dirty once saved", async () => {
|
||||||
const save = vi
|
const save = vi
|
||||||
.spyOn(api, "updateMaintenanceBatch")
|
.spyOn(api, "updateMaintenanceBatch")
|
||||||
|
|||||||
@@ -1,4 +1,4 @@
|
|||||||
import { useEffect, useState } from "react";
|
import { useEffect, useId, useRef, useState } from "react";
|
||||||
import { qk } from "../lib/queryKeys";
|
import { qk } from "../lib/queryKeys";
|
||||||
import { useTranslation } from "react-i18next";
|
import { useTranslation } from "react-i18next";
|
||||||
import i18n from "../i18n";
|
import i18n from "../i18n";
|
||||||
@@ -143,15 +143,29 @@ function JobRow({
|
|||||||
// nothing for an effect to re-sync: `null` means "follow the job". The poll behind this page
|
// nothing for an effect to re-sync: `null` means "follow the job". The poll behind this page
|
||||||
// refreshes every few seconds, which is exactly when a mirror-plus-effect would have shown the
|
// refreshes every few seconds, which is exactly when a mirror-plus-effect would have shown the
|
||||||
// previous value for a frame.
|
// previous value for a frame.
|
||||||
|
const rangeId = useId();
|
||||||
const [draft, setDraft] = useState<string | null>(null);
|
const [draft, setDraft] = useState<string | null>(null);
|
||||||
const editing = draft !== null;
|
const editing = draft !== null;
|
||||||
const val = draft ?? String(job.interval_minutes);
|
const val = draft ?? String(job.interval_minutes);
|
||||||
const next = secsUntil(job.next_run);
|
const next = secsUntil(job.next_run);
|
||||||
|
const m = parseInt(val, 10);
|
||||||
|
const valid = Number.isFinite(m) && m >= 1 && m <= 1440;
|
||||||
|
// Every way out of the editor unmounts the focused control, so hand focus back to the trigger that
|
||||||
|
// opened it — otherwise Escape drops a keyboard user onto <body> and the next Tab restarts at the
|
||||||
|
// top of the page. Same pattern as IssueComposer's review buttons.
|
||||||
|
const triggerRef = useRef<HTMLButtonElement | null>(null);
|
||||||
|
function close() {
|
||||||
|
setDraft(null);
|
||||||
|
queueMicrotask(() => triggerRef.current?.focus());
|
||||||
|
}
|
||||||
|
|
||||||
function save() {
|
function save() {
|
||||||
const m = parseInt(val, 10);
|
// A value outside the range is REFUSED, not swallowed: the draft stays so the user can see and
|
||||||
if (Number.isFinite(m) && m >= 1 && m <= 1440 && m !== job.interval_minutes) onSave(m);
|
// correct what they typed, and `aria-invalid` + the hint say why. Dropping it silently left the
|
||||||
setDraft(null);
|
// field showing the server's number with no clue whether the save had happened.
|
||||||
|
if (!valid) return;
|
||||||
|
if (m !== job.interval_minutes) onSave(m);
|
||||||
|
close();
|
||||||
}
|
}
|
||||||
|
|
||||||
return (
|
return (
|
||||||
@@ -172,33 +186,45 @@ function JobRow({
|
|||||||
max={1440}
|
max={1440}
|
||||||
value={val}
|
value={val}
|
||||||
autoFocus
|
autoFocus
|
||||||
|
aria-invalid={!valid}
|
||||||
|
aria-errormessage={valid ? undefined : rangeId}
|
||||||
onChange={(e) => setDraft(e.target.value)}
|
onChange={(e) => setDraft(e.target.value)}
|
||||||
onKeyDown={(e) => {
|
onKeyDown={(e) => {
|
||||||
if (e.key === "Enter") save();
|
if (e.key === "Enter") save();
|
||||||
if (e.key === "Escape") setDraft(null);
|
if (e.key === "Escape") close();
|
||||||
}}
|
}}
|
||||||
className="w-16 bg-surface border border-border rounded-sm px-1.5 py-0.5 text-xs focus:outline-hidden focus:ring-2 focus:ring-accent/40"
|
className={`w-16 bg-surface border rounded-sm px-1.5 py-0.5 text-xs focus:outline-hidden focus:ring-2 ${
|
||||||
|
valid
|
||||||
|
? "border-border focus:ring-accent/40"
|
||||||
|
: "border-danger focus:ring-danger/40"
|
||||||
|
}`}
|
||||||
/>
|
/>
|
||||||
<span className="text-muted text-xs">{t("scheduler.minutes")}</span>
|
<span className="text-muted text-xs">{t("scheduler.minutes")}</span>
|
||||||
<FocusSafeButton
|
<FocusSafeButton
|
||||||
onClick={save}
|
onClick={save}
|
||||||
disabled={saving}
|
disabled={saving || !valid}
|
||||||
className="text-success hover:opacity-80 disabled:opacity-50"
|
className="text-success hover:opacity-80 disabled:opacity-50"
|
||||||
aria-label={t("common.save")}
|
aria-label={t("common.save")}
|
||||||
>
|
>
|
||||||
<Check className="w-3.5 h-3.5" />
|
<Check className="w-3.5 h-3.5" />
|
||||||
</FocusSafeButton>
|
</FocusSafeButton>
|
||||||
<button
|
<button
|
||||||
onClick={() => setDraft(null)}
|
onClick={close}
|
||||||
className="text-muted hover:text-fg"
|
className="text-muted hover:text-fg"
|
||||||
aria-label={t("common.cancel")}
|
aria-label={t("common.cancel")}
|
||||||
>
|
>
|
||||||
<X className="w-3.5 h-3.5" />
|
<X className="w-3.5 h-3.5" />
|
||||||
</button>
|
</button>
|
||||||
|
{!valid && (
|
||||||
|
<span id={rangeId} role="alert" className="text-2xs text-danger">
|
||||||
|
{t("scheduler.range", { min: 1, max: 1440 })}
|
||||||
|
</span>
|
||||||
|
)}
|
||||||
</span>
|
</span>
|
||||||
) : (
|
) : (
|
||||||
<Tooltip hint={t("scheduler.editInterval")}>
|
<Tooltip hint={t("scheduler.editInterval")}>
|
||||||
<button
|
<button
|
||||||
|
ref={triggerRef}
|
||||||
onClick={() => setDraft(String(job.interval_minutes))}
|
onClick={() => setDraft(String(job.interval_minutes))}
|
||||||
className="group text-muted font-normal hover:text-accent inline-flex items-center gap-1 transition"
|
className="group text-muted font-normal hover:text-accent inline-flex items-center gap-1 transition"
|
||||||
>
|
>
|
||||||
@@ -258,15 +284,22 @@ function MaintenanceSettings({
|
|||||||
saving: boolean;
|
saving: boolean;
|
||||||
}) {
|
}) {
|
||||||
const { t } = useTranslation();
|
const { t } = useTranslation();
|
||||||
// Same shape as the job row above: a draft while editing, the server's value otherwise.
|
// Same shape as the job row above: a draft while editing, the server's value otherwise. This is
|
||||||
|
// the editor a poll really could overwrite mid-keystroke before E27 S2 — its old effect had no
|
||||||
|
// `editing` guard at all.
|
||||||
|
const rangeId = useId();
|
||||||
const [draft, setDraft] = useState<string | null>(null);
|
const [draft, setDraft] = useState<string | null>(null);
|
||||||
const val = draft ?? String(m.revalidate_batch);
|
const val = draft ?? String(m.revalidate_batch);
|
||||||
|
const n = parseInt(val, 10);
|
||||||
|
const valid = Number.isFinite(n) && n >= m.min && n <= m.max;
|
||||||
|
|
||||||
function save() {
|
function save() {
|
||||||
const n = parseInt(val, 10);
|
// Out of range is REFUSED, not swallowed: keep the draft so the typed value stays visible and
|
||||||
if (Number.isFinite(n) && n >= m.min && n <= m.max && n !== m.revalidate_batch) onSave(n);
|
// correctable, and say why. Dropping it left the field showing the server's number with nothing
|
||||||
// Dropping the draft is what lets the box follow the server again once the save lands — and what
|
// to tell the user whether the save had happened, been rejected, or been ignored.
|
||||||
// clears a value that failed validation instead of leaving it stuck in the field.
|
if (!valid) return;
|
||||||
|
if (n !== m.revalidate_batch) onSave(n);
|
||||||
|
// Dropping the draft is what lets the box follow the server again once the save lands.
|
||||||
setDraft(null);
|
setDraft(null);
|
||||||
}
|
}
|
||||||
const dirty = String(m.revalidate_batch) !== val;
|
const dirty = String(m.revalidate_batch) !== val;
|
||||||
@@ -288,14 +321,26 @@ function MaintenanceSettings({
|
|||||||
step={1000}
|
step={1000}
|
||||||
value={val}
|
value={val}
|
||||||
onChange={(e) => setDraft(e.target.value)}
|
onChange={(e) => setDraft(e.target.value)}
|
||||||
onKeyDown={(e) => e.key === "Enter" && save()}
|
onKeyDown={(e) => {
|
||||||
|
if (e.key === "Enter") save();
|
||||||
|
if (e.key === "Escape") setDraft(null);
|
||||||
|
}}
|
||||||
aria-label={t("scheduler.maintenance.batchLabel")}
|
aria-label={t("scheduler.maintenance.batchLabel")}
|
||||||
className="w-28 bg-surface border border-border rounded-sm px-2 py-1 text-sm tabular-nums focus:outline-hidden focus:ring-2 focus:ring-accent/40"
|
aria-invalid={!valid}
|
||||||
|
aria-errormessage={valid ? undefined : rangeId}
|
||||||
|
className={`w-28 bg-surface border rounded-sm px-2 py-1 text-sm tabular-nums focus:outline-hidden focus:ring-2 ${
|
||||||
|
valid ? "border-border focus:ring-accent/40" : "border-danger focus:ring-danger/40"
|
||||||
|
}`}
|
||||||
/>
|
/>
|
||||||
<Button variant="glass" size="sm" onClick={save} disabled={!dirty || saving}>
|
<Button variant="glass" size="sm" onClick={save} disabled={!dirty || saving || !valid}>
|
||||||
{t("common.save")}
|
{t("common.save")}
|
||||||
</Button>
|
</Button>
|
||||||
</div>
|
</div>
|
||||||
|
{!valid && (
|
||||||
|
<div id={rangeId} role="alert" className="text-2xs text-danger mt-1.5">
|
||||||
|
{t("scheduler.range", { min: m.min, max: m.max })}
|
||||||
|
</div>
|
||||||
|
)}
|
||||||
<div className="text-2xs text-muted mt-1.5">
|
<div className="text-2xs text-muted mt-1.5">
|
||||||
{t("scheduler.maintenance.batchNote", {
|
{t("scheduler.maintenance.batchNote", {
|
||||||
default: formatNumber(m.revalidate_batch_default),
|
default: formatNumber(m.revalidate_batch_default),
|
||||||
|
|||||||
@@ -23,6 +23,7 @@
|
|||||||
"quotaRemaining": "{{formatted}} left",
|
"quotaRemaining": "{{formatted}} left",
|
||||||
"quotaNote": "Shared YouTube Data API budget; resets at midnight US Pacific. Backfill yields to leave headroom for interactive use.",
|
"quotaNote": "Shared YouTube Data API budget; resets at midnight US Pacific. Backfill yields to leave headroom for interactive use.",
|
||||||
"minutes": "min",
|
"minutes": "min",
|
||||||
|
"range": "Must be between {{min}} and {{max}}.",
|
||||||
"editInterval": "Click to change how often this runs",
|
"editInterval": "Click to change how often this runs",
|
||||||
"intervalFailed": "Couldn't change the interval",
|
"intervalFailed": "Couldn't change the interval",
|
||||||
"runNow": "Run now",
|
"runNow": "Run now",
|
||||||
|
|||||||
@@ -23,6 +23,7 @@
|
|||||||
"quotaRemaining": "{{formatted}} maradt",
|
"quotaRemaining": "{{formatted}} maradt",
|
||||||
"quotaNote": "Közös YouTube Data API-keret; éjfélkor (US Pacific) nullázódik. A backfill hátrébb sorol, hogy maradjon hely az interaktív használatnak.",
|
"quotaNote": "Közös YouTube Data API-keret; éjfélkor (US Pacific) nullázódik. A backfill hátrébb sorol, hogy maradjon hely az interaktív használatnak.",
|
||||||
"minutes": "perc",
|
"minutes": "perc",
|
||||||
|
"range": "{{min}} és {{max}} között lehet.",
|
||||||
"editInterval": "Kattints a gyakoriság módosításához",
|
"editInterval": "Kattints a gyakoriság módosításához",
|
||||||
"intervalFailed": "Nem sikerült módosítani a gyakoriságot",
|
"intervalFailed": "Nem sikerült módosítani a gyakoriságot",
|
||||||
"runNow": "Futtatás most",
|
"runNow": "Futtatás most",
|
||||||
|
|||||||
Reference in New Issue
Block a user