diff --git a/web/src/App.tsx b/web/src/App.tsx index eae8f58..43d6076 100644 --- a/web/src/App.tsx +++ b/web/src/App.tsx @@ -1,5 +1,5 @@ import { Fragment, lazy, Suspense, useEffect, useState } from "react"; -import { Route, Switch, Redirect, useLocation } from "wouter"; +import { Route, Switch, Redirect, useLocation, Router } from "wouter"; import { useSession } from "@/store/session"; import { useMail } from "@/store/mail"; import { scheduleSupported, useScheduled } from "@/store/scheduled"; @@ -21,6 +21,7 @@ import { PAINTED_FROM_CACHE, useSettings, syncedPart } from "@/store/settings"; import { armSettingsSync, loadRemoteSettings, queueSettingsPush, settingsAlreadyLoadedFor, settingsSyncAvailable } from "@/lib/settingsSync"; import { listenForVerification, renewWebPush } from "@/lib/webpushEnable"; import { useLanguageVersion, whenLanguageReady } from "@/lib/i18n"; +import { confirmLeaveUnsaved, hasUnsavedChanges } from "@/lib/unsavedChanges"; const ContactsView = lazy(() => import("@/views/contacts/ContactsView").then((m) => ({ default: m.ContactsView }))); const CalendarView = lazy(() => import("@/views/calendar/CalendarView").then((m) => ({ default: m.CalendarView }))); @@ -70,11 +71,32 @@ export function App() { ); } return ( - <> + /* + * Every in-app navigation runs through `aroundNav` -- links, redirects and + * `navigate()` alike, since wouter routes them all through the same place. + * That is what makes the guard hold for the app rail and the settings nav + * without either of them knowing an editor exists. + * + * The back button is the gap: by the time `popstate` arrives the history + * has already moved, and the only way to hold the page would be to push an + * entry back, which breaks the button for everyone who has nothing pending. + * Reload and tab close are covered by `beforeunload` instead. + */ + { + if (!hasUnsavedChanges()) { + navigate(to, options); + return; + } + void confirmLeaveUnsaved().then((ok) => { + if (ok) navigate(to, options); + }); + }} + > {status === "anonymous" ? : } - + ); } diff --git a/web/src/lib/unsavedChanges.ts b/web/src/lib/unsavedChanges.ts new file mode 100644 index 0000000..0ac475a --- /dev/null +++ b/web/src/lib/unsavedChanges.ts @@ -0,0 +1,107 @@ +import { useEffect, useRef } from "react"; +import { choiceDialog } from "@/ui/dialog"; +import { t } from "@/lib/i18n"; + +/** + * Editors that would lose work if you walked away from them. + * + * The filter editors keep their edits in component state, so every way out of + * the page -- a settings link, the app rail, the Rules/Scripts switch -- threw + * them away without a word, and the only sign there had been anything to lose + * was a Save button that a screenful of rules had already pushed past the + * bottom of the window. That is issue #175. + * + * An editor registers what it has pending here; navigation asks before it + * happens. The question is deliberately not a yes/no: "leave without saving?" + * makes losing the work the easy answer and saving it the one you have to back + * out and find, when saving is what almost everybody wants. + */ +export interface UnsavedChanges { + /** Whether anything would be lost by leaving right now. */ + dirty: boolean; + /** + * Persist the edits. `false` keeps you where you are: a save that failed is + * exactly the moment not to navigate, because the edits only exist here. + */ + save: () => Promise; + /** Throw the edits away. */ + discard: () => void; + /** Already translated, and specific: it says what is about to be lost. */ + message: string; +} + +/** + * Held by reference rather than by value, so the callbacks the dialog runs are + * the ones from the editor's latest render and not from whenever it mounted. + */ +type Slot = { current: UnsavedChanges }; + +const slots = new Set(); + +/** Register this editor's pending edits for as long as it is on screen. */ +export function useUnsavedChanges(changes: UnsavedChanges): void { + const slot = useRef(changes); + slot.current = changes; + useEffect(() => { + slots.add(slot); + return () => { + slots.delete(slot); + }; + }, []); + /* + * Reloading and closing the tab are the browser's to ask about, and all it + * will show is its own generic wording -- custom text was removed years ago. + * Generic is still better than silent, and it costs one listener, armed only + * while there is something to lose. + */ + useEffect(() => { + if (!changes.dirty) return; + const warn = (e: BeforeUnloadEvent) => { + e.preventDefault(); + e.returnValue = ""; + }; + window.addEventListener("beforeunload", warn); + return () => window.removeEventListener("beforeunload", warn); + }, [changes.dirty]); +} + +/** Whether leaving now would lose anything. Cheap enough for every navigation. */ +export function hasUnsavedChanges(): boolean { + for (const slot of slots) if (slot.current.dirty) return true; + return false; +} + +/** + * Ask about anything pending. Resolves true when it is safe to go. + * + * Dismissing the dialog -- Escape, the backdrop, the close button -- is not + * one of the answers, so it cannot be mistaken for "discard": it leaves you on + * the page with the edits intact. + */ +export async function confirmLeaveUnsaved(): Promise { + /* + * A snapshot, not the live set. `save` and `discard` both make an editor + * clean, but not until React has re-rendered it, and re-reading `slots` + * between questions would find the same editor still dirty and ask twice. + */ + for (const slot of [...slots]) { + const pending = slot.current; + if (!pending.dirty) continue; + const answer = await choiceDialog({ + title: t("Save your changes?"), + message: pending.message, + choices: [ + { value: "save", label: t("Save changes") }, + { value: "discard", label: t("Discard changes"), hint: t("What you changed here will be lost."), danger: true }, + ], + cancelLabel: t("Stay here"), + }); + if (answer === null) return false; + if (answer === "discard") { + pending.discard(); + continue; + } + if (!(await pending.save())) return false; + } + return true; +} diff --git a/web/src/styles/app.css b/web/src/styles/app.css index e7f5fee..14a3406 100644 --- a/web/src/styles/app.css +++ b/web/src/styles/app.css @@ -730,7 +730,7 @@ select optgroup { background-color: var(--bg-elev); color: var(--fg); } .settings-layout { display: grid; grid-template-columns: 240px 1fr; height: 100%; min-height: 0; flex: 1; } .settings-nav { border-right: 1px solid var(--border); padding: 12px 8px; overflow-y: auto; } .settings-nav .nav-item { margin-right: 0; border-radius: var(--radius-sm); } -.settings-content { overflow-y: auto; padding: 24px 32px 64px; max-width: 860px; } +.settings-content { --pad-b: 64px; overflow-y: auto; padding: 24px 32px var(--pad-b); max-width: 860px; } .settings-content h1 { margin: 0 0 4px; font-size: 1.5em; font-weight: 650; } .settings-content h2 { margin: 28px 0 12px; font-size: 1.05em; font-weight: 650; padding-bottom: 6px; border-bottom: 1px solid var(--border); } .settings-content .lead { color: var(--fg-muted); margin: 0 0 20px; } @@ -754,6 +754,18 @@ select optgroup { background-color: var(--bg-elev); color: var(--fg); } .rule-card.drop-below { box-shadow: inset 0 -3px 0 0 var(--accent); } .rule-card .drag-handle { display: flex; align-items: center; padding: 2px; margin-left: -4px; color: var(--fg-faint); cursor: grab; touch-action: none; } .rule-card .drag-handle:active { cursor: grabbing; } + +/* The bar that says whether there is unsaved work, kept where it can be read. + Unpinned it sat after the rules, which on a full list means off the bottom of + the window: the one indicator that anything was pending was the one thing you + had to scroll to find (issue #175). + + The offset is what pins it flush. A sticky `bottom: 0` stops at the scroll + container's padding box, which leaves the pane's bottom padding as a strip + below the bar for rules to scroll through; sticking to minus that padding + closes it, on the narrow layout's larger padding as well. */ +.save-bar { position: sticky; bottom: calc(var(--pad-b, 0px) * -1); z-index: 2; margin-top: 12px; padding: 12px 0; background: var(--bg); border-top: 1px solid var(--border); } +.save-bar .unsaved { color: var(--warn); font-weight: 600; font-size: .9em; } .rule-row { display: grid; grid-template-columns: 1fr 1fr 1fr auto; gap: 8px; align-items: center; margin-bottom: 8px; } .rule-row.actions { grid-template-columns: 1fr 2fr auto; } /* A header typed by hand needs a box of its own, alongside the comparator. */ @@ -1017,7 +1029,7 @@ select optgroup { background-color: var(--bg-elev); color: var(--fg); } .settings-layout.section .settings-nav { display: none; } .settings-layout.root .settings-nav { display: block; border-right: 0; } .settings-layout.root .settings-content { display: none; } - .settings-content { padding: 16px 16px 80px; } + .settings-content { --pad-b: 80px; padding: 16px 16px var(--pad-b); } .contacts-layout { grid-template-columns: 1fr; } .contacts-books { display: none; } .contacts-layout.detail .contacts-list { display: none; } diff --git a/web/src/views/settings/FiltersSettings.tsx b/web/src/views/settings/FiltersSettings.tsx index 5b77317..f510e3c 100644 --- a/web/src/views/settings/FiltersSettings.tsx +++ b/web/src/views/settings/FiltersSettings.tsx @@ -10,10 +10,22 @@ import { Switch, Spinner } from "@/ui/misc"; import { toast } from "@/ui/toast"; import type { SieveScript } from "@/jmap/types"; import { t, tNode } from "@/lib/i18n"; +import { confirmLeaveUnsaved, useUnsavedChanges } from "@/lib/unsavedChanges"; export function FiltersSettings() { const sieve = useSieve(); const [tab, setTab] = useState<"rules" | "scripts">("rules"); + /* + * Switching tabs unmounts the editor you were in, which is a way of losing + * work that never leaves the page and so never reaches the router's guard. + * Ask here for the same reason navigation asks. + */ + const switchTab = (next: "rules" | "scripts") => { + if (next === tab) return; + void confirmLeaveUnsaved().then((ok) => { + if (ok) setTab(next); + }); + }; useEffect(() => { if (sieve.available && !sieve.scripts.length && !sieve.loading) void sieve.load(); // eslint-disable-next-line react-hooks/exhaustive-deps @@ -33,8 +45,8 @@ export function FiltersSettings() {

{t("Filters & rules")}

{t("Sort incoming mail automatically. Rules run on the server (Sieve), so they work for every client you use.")}

- - + +
{sieve.loading && !sieve.scripts.length ? : tab === "rules" ? : } @@ -68,19 +80,30 @@ function RulesEditor() { const isBelow = (el: HTMLElement, y: number) => { const b = el.getBoundingClientRect(); return y > b.top + b.height / 2; }; const activeIsOther = script && script.name !== "ihasmail" && script.isActive; - const save = async (next: SieveRule[]) => { + const save = async (next: SieveRule[]): Promise => { setSaving(true); try { await sieve.saveRules(next); setLocal(null); toast.success(t("Filters saved")); + return true; } catch (err) { toast.error(t("Could not save filters: {error}", { error: (err as Error).message })); + return false; } finally { setSaving(false); } }; + // Before the early returns below: a hook cannot be skipped, and the branches + // they guard have nothing pending anyway. + useUnsavedChanges({ + dirty, + message: t("Your filter rules have changes that have not been saved."), + save: () => save(list), + discard: () => setLocal(null), + }); + // Before the hand-written branch: a script that arrived in part is not a // script someone chose to write themselves, and the way out of it is a reload // rather than the "start with rules" button below, which would write over it. @@ -149,11 +172,16 @@ function RulesEditor() { ))} -
+ {/* + * Pinned, because with more than a screenful of rules this bar was the + * only thing saying there was unsaved work and it sat below the fold. + */} +
+ {dirty && {t("Unsaved changes")}} {dirty && } - +
{content && (
@@ -188,23 +216,39 @@ function ScriptsEditor() { const [name, setName] = useState(""); const [busy, setBusy] = useState(false); const [validation, setValidation] = useState(null); + /** + * What was in the editor when it opened, so "has this been touched" is a + * comparison rather than a flag every edit path has to remember to set. + * `null` means the editor is closed and there is nothing to compare. + */ + const [opened, setOpened] = useState<{ name: string; content: string } | null>(null); + const dirty = opened !== null && (name !== opened.name || content !== opened.content); + + const start = (scriptName: string, source: string) => { + setName(scriptName); + setContent(source); + setOpened({ name: scriptName, content: source }); + }; + + const close = () => { + setSel(null); + setName(""); + setContent(""); + setOpened(null); + setValidation(null); + }; const open = async (s: SieveScript | null) => { setSel(s); setValidation(null); - if (s) { - setName(s.name); - setContent(await sieve.getContent(s.id)); - } else { - setName(""); - setContent('require ["fileinto"];\n\n'); - } + if (s) start(s.name, await sieve.getContent(s.id)); + else start("", 'require ["fileinto"];\n\n'); }; - const save = async (activate: boolean) => { + const save = async (activate: boolean): Promise => { if (!name.trim()) { toast.error(t("Script name is required")); - return; + return false; } setBusy(true); try { @@ -212,38 +256,57 @@ function ScriptsEditor() { setValidation(err); if (err) { toast.error(t("Script has errors")); - return; + return false; } await sieve.saveScript(sel?.id ?? null, name.trim(), content, activate); toast.success(t("Script saved")); - setSel(null); + // All the way closed, back to the list. Clearing only `sel` left the + // editor up -- it is shown whenever there is a name or a body -- but with + // the name unlocked, so saving a second time created a duplicate script + // rather than updating the one just written. + close(); + return true; } catch (err) { toast.error((err as Error).message); + return false; } finally { setBusy(false); } }; - if (sel !== null || name !== "" || content !== "") { - if (sel !== null || name !== "" || content !== "") { - return ( -
-
setName(e.target.value)} disabled={Boolean(sel)} />
-
- -