From 4a1a02535373420d4484e5fcf605985f56db94ba Mon Sep 17 00:00:00 2001 From: npeter83 Date: Thu, 18 Jun 2026 23:59:40 +0200 Subject: [PATCH] feat(settings): explicit Save/Discard for preferences with dirty tracking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Settings-page prefs (theme/scheme/dark-mode/list-view/perf/hints/font + the notification settings) were each auto-saved to the server on every toggle via fire-and-forget savePrefs().catch(() => {}) — silent on failure, and no positive confirmation on success, so the user had zero feedback either way. Make them a draft instead: changes apply locally for instant preview but persist only on an explicit Save (or revert on Discard). App owns the live draft + the last-saved baseline, computes dirty, and exposes a controller to the panel. The panel shows a Save/Discard bar with 'Saving…' → 'Settings saved' (auto-clearing) / 'Couldn't save' feedback. Leaving the page with unsaved changes prompts a confirm (in-app nav + browser Back), and a beforeunload guards reload/close. savePrefs is now idempotent so the Save survives a transient gateway blip; failures surface via the connection-lost status. Language & sidebar layout stay instant (edited outside this page). New i18n keys settings.save.* / settings.unsaved.* in EN/HU/DE. --- frontend/src/App.tsx | 209 ++++++++++++++++++--- frontend/src/components/SettingsPanel.tsx | 205 +++++++++++--------- frontend/src/i18n/locales/de/settings.json | 13 ++ frontend/src/i18n/locales/en/settings.json | 13 ++ frontend/src/i18n/locales/hu/settings.json | 13 ++ frontend/src/lib/api.ts | 3 +- 6 files changed, 336 insertions(+), 120 deletions(-) diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 037cc89..cc1e2d5 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -1,4 +1,4 @@ -import { useEffect, useState } from "react"; +import { useCallback, useEffect, useRef, useState } from "react"; import { useTranslation } from "react-i18next"; import { useQuery } from "@tanstack/react-query"; import { api, HttpError, type FeedFilters } from "./lib/api"; @@ -17,8 +17,10 @@ import { saveLayoutLocal, type SidebarLayout, } from "./lib/sidebarLayout"; -import { configureNotifications, notify } from "./lib/notifications"; -import { setHintsEnabled } from "./lib/hints"; +import { configureNotifications, getNotifSettings, notify, type NotifSettings } from "./lib/notifications"; +import { hintsEnabled, setHintsEnabled } from "./lib/hints"; +import { useConfirm } from "./components/ConfirmProvider"; +import type { PrefsController } from "./components/SettingsPanel"; import Login from "./components/Login"; import Header from "./components/Header"; import NavSidebar from "./components/NavSidebar"; @@ -54,6 +56,18 @@ const DEFAULT_FILTERS: FeedFilters = { const FILTERS_KEY = "subfeed.filters"; const PAGE_KEY = "siftlode.page"; +const PERF_KEY = "subfeed.perfMode"; + +// The preferences edited on the Settings page. They apply locally for instant preview but +// persist to the server only on an explicit Save — so App holds both the live "draft" (the +// individual states below) and the last-saved "baseline" to compute dirty / revert on discard. +type EditablePrefs = { + theme: ThemePrefs; + view: "grid" | "list"; + performanceMode: boolean; + hints: boolean; + notifications: NotifSettings; +}; // Page is navigation state, not a filter, but it's also kept out of the address bar — so // persist it to localStorage to survive a reload. A share link's ?page= still wins. @@ -81,9 +95,29 @@ function loadInitialFilters(): FeedFilters { export default function App() { const { t, i18n } = useTranslation(); + const confirm = useConfirm(); + // Refs the (stable) navigation handlers read so they always see the latest unsaved-prefs + // state without being recreated; populated by an effect each render (below). + const prefsDirtyRef = useRef(false); + const discardRef = useRef<() => void>(() => {}); + const confirmRef = useRef(confirm); + const pageRef = useRef("feed"); const [theme, setThemeState] = useState(() => loadLocalTheme()); const [filters, setFiltersState] = useState(loadInitialFilters); const [view, setView] = useState<"grid" | "list">("grid"); + // Settings-page prefs draft (apply live, persist on Save — see EditablePrefs). + const [perf, setPerf] = useState(() => localStorage.getItem(PERF_KEY) === "1"); + const [hints, setHints] = useState(() => hintsEnabled()); + const [notif, setNotif] = useState(() => getNotifSettings()); + const [savedPrefs, setSavedPrefs] = useState(() => ({ + theme: loadLocalTheme(), + view: "grid", + performanceMode: localStorage.getItem(PERF_KEY) === "1", + hints: hintsEnabled(), + notifications: getNotifSettings(), + })); + const [prefsSaveState, setPrefsSaveState] = useState("idle"); + const saveMsgTimer = useRef(undefined); const [sidebarLayout, setSidebarLayoutState] = useState(loadLayout); const [page, setPageState] = useState(loadInitialPage); const [wizardOpen, setWizardOpen] = useState(false); @@ -125,12 +159,29 @@ export default function App() { function setPage(next: Page) { if (next === page) return; - setPageState(next); - localStorage.setItem(PAGE_KEY, next); - // Push an in-app history entry so the browser/mouse Back button steps through pages - // instead of leaving the app (e.g. back to the OAuth redirect). The URL stays clean — - // the page rides in history.state, not the query string (filters never go in the URL). - window.history.pushState({ ...window.history.state, sfPage: next }, ""); + const go = () => { + setPageState(next); + localStorage.setItem(PAGE_KEY, next); + // Push an in-app history entry so the browser/mouse Back button steps through pages + // instead of leaving the app (e.g. back to the OAuth redirect). The URL stays clean — + // the page rides in history.state, not the query string (filters never go in the URL). + window.history.pushState({ ...window.history.state, sfPage: next }, ""); + }; + // Guard leaving the Settings page with unsaved preference changes. + if (page === "settings" && prefsDirtyRef.current) { + void confirmRef.current({ + title: t("settings.unsaved.title"), + message: t("settings.unsaved.message"), + confirmLabel: t("settings.unsaved.discard"), + danger: true, + }).then((ok) => { + if (!ok) return; + discardRef.current(); + go(); + }); + return; + } + go(); } function setSidebarLayout(next: SidebarLayout) { @@ -141,6 +192,15 @@ export default function App() { useEffect(() => applyTheme(theme), [theme]); + // Apply the draft prefs locally for instant preview (their localStorage mirrors update via + // the stores too); persistence to the server is deferred to an explicit Save. + useEffect(() => { + document.documentElement.dataset.perf = perf ? "1" : ""; + localStorage.setItem(PERF_KEY, perf ? "1" : "0"); + }, [perf]); + useEffect(() => setHintsEnabled(hints), [hints]); + useEffect(() => configureNotifications(notif), [notif]); + // Filters live in localStorage, not the address bar. If we arrived via a "Share view" // link, its params have already hydrated the initial state — persist them and strip the // query so the URL stays clean from here on. @@ -162,6 +222,23 @@ export default function App() { ); function onPop(e: PopStateEvent) { const p = (e.state?.sfPage as Page) ?? "feed"; + // Guard a Back step that leaves Settings with unsaved changes: re-assert the Settings + // entry so nothing is silently stranded, then ask; on discard, go where Back headed. + if (pageRef.current === "settings" && p !== "settings" && prefsDirtyRef.current) { + window.history.pushState({ ...window.history.state, sfPage: "settings" }, ""); + void confirmRef.current({ + title: t("settings.unsaved.title"), + message: t("settings.unsaved.message"), + confirmLabel: t("settings.unsaved.discard"), + danger: true, + }).then((ok) => { + if (!ok) return; + discardRef.current(); + setPageState(p); + localStorage.setItem(PAGE_KEY, p); + }); + return; + } setPageState(p); localStorage.setItem(PAGE_KEY, p); } @@ -182,22 +259,34 @@ export default function App() { useEffect(() => { const prefs = meQuery.data?.preferences; if (!prefs) return; - if (prefs.theme) { - const merged = { ...DEFAULT_THEME, ...prefs.theme }; - setThemeState(merged); - saveLocalTheme(merged); - } - if (prefs.view === "grid" || prefs.view === "list") setView(prefs.view); + // Adopt the server prefs as both the saved baseline and the live draft. The runtime + // effects above apply the draft (theme/perf/hints/notifications) for display. + const adopted: EditablePrefs = { + theme: prefs.theme ? { ...DEFAULT_THEME, ...prefs.theme } : loadLocalTheme(), + view: prefs.view === "grid" || prefs.view === "list" ? prefs.view : "grid", + performanceMode: + typeof prefs.performanceMode === "boolean" + ? prefs.performanceMode + : localStorage.getItem(PERF_KEY) === "1", + hints: typeof prefs.hints === "boolean" ? prefs.hints : hintsEnabled(), + notifications: prefs.notifications + ? { ...getNotifSettings(), ...prefs.notifications } + : getNotifSettings(), + }; + setThemeState(adopted.theme); + saveLocalTheme(adopted.theme); + setView(adopted.view); + setPerf(adopted.performanceMode); + setHints(adopted.hints); + setNotif(adopted.notifications); + setSavedPrefs(adopted); + // Out-of-scope prefs stay instant (edited outside the Settings page). if (prefs.sidebarLayout) { const l = normalizeLayout(prefs.sidebarLayout); setSidebarLayoutState(l); saveLayoutLocal(l); } if (isSupported(prefs.language)) setLanguage(prefs.language); - if (prefs.notifications) configureNotifications(prefs.notifications); - if (typeof prefs.hints === "boolean") setHintsEnabled(prefs.hints); - if (typeof prefs.performanceMode === "boolean") - document.documentElement.dataset.perf = prefs.performanceMode ? "1" : ""; // Nudge admins when access requests are waiting (in lieu of a server-push bell). const pending = meQuery.data?.pending_invites ?? 0; if (meQuery.data?.role === "admin" && pending > 0) { @@ -216,20 +305,87 @@ export default function App() { // eslint-disable-next-line react-hooks/exhaustive-deps }, [meQuery.data?.id]); + // Theme is part of the Settings draft: apply locally for preview, persist on Save. function setTheme(next: ThemePrefs) { setThemeState(next); saveLocalTheme(next); - api.savePrefs({ theme: next }).catch(() => {}); - } - function changeView(v: "grid" | "list") { - setView(v); - api.savePrefs({ view: v }).catch(() => {}); } + // Language is edited outside the Settings page (sidebar/login), so it stays instant. function changeLanguage(code: LangCode) { setLanguage(code); api.savePrefs({ language: code }).catch(() => {}); } + const prefsDirty = + JSON.stringify({ theme, view, performanceMode: perf, hints, notifications: notif }) !== + JSON.stringify(savedPrefs); + + const savePrefs = useCallback(() => { + const payload: EditablePrefs = { theme, view, performanceMode: perf, hints, notifications: notif }; + window.clearTimeout(saveMsgTimer.current); + setPrefsSaveState("saving"); + api + .savePrefs(payload) + .then(() => { + setSavedPrefs(payload); + setPrefsSaveState("saved"); + saveMsgTimer.current = window.setTimeout(() => setPrefsSaveState("idle"), 2500); + }) + .catch(() => { + // api.req() already surfaces the failure (connection-lost status / error dialog); + // just flag the bar so the user sees the save didn't take, then clear it. + setPrefsSaveState("error"); + saveMsgTimer.current = window.setTimeout(() => setPrefsSaveState("idle"), 4000); + }); + }, [theme, view, perf, hints, notif]); + + const discardPrefs = useCallback(() => { + setThemeState(savedPrefs.theme); + saveLocalTheme(savedPrefs.theme); + setView(savedPrefs.view); + setPerf(savedPrefs.performanceMode); + setHints(savedPrefs.hints); + setNotif(savedPrefs.notifications); + window.clearTimeout(saveMsgTimer.current); + setPrefsSaveState("idle"); + }, [savedPrefs]); + + const prefsCtl: PrefsController = { + theme, + setTheme, + view, + setView, + perf, + setPerf, + hints, + setHints, + notif, + setNotif, + dirty: prefsDirty, + save: savePrefs, + discard: discardPrefs, + saveState: prefsSaveState, + }; + + // Keep the navigation guards (setPage / popstate) reading the latest values. + useEffect(() => { + prefsDirtyRef.current = prefsDirty; + discardRef.current = discardPrefs; + confirmRef.current = confirm; + pageRef.current = page; + }); + + // Warn on a hard navigation (reload / tab close) while preference changes are unsaved. + useEffect(() => { + if (!prefsDirty) return; + const handler = (e: BeforeUnloadEvent) => { + e.preventDefault(); + e.returnValue = ""; + }; + window.addEventListener("beforeunload", handler); + return () => window.removeEventListener("beforeunload", handler); + }, [prefsDirty]); + if (meQuery.isLoading) return (
{t("common.loading")}
@@ -307,10 +463,7 @@ export default function App() { ) : page === "settings" ? ( setWizardOpen(true)} /> ) : ( diff --git a/frontend/src/components/SettingsPanel.tsx b/frontend/src/components/SettingsPanel.tsx index 47b44b1..9e1b5d1 100644 --- a/frontend/src/components/SettingsPanel.tsx +++ b/frontend/src/components/SettingsPanel.tsx @@ -1,21 +1,37 @@ import { useCallback, useEffect, useState, useSyncExternalStore } from "react"; import { useTranslation } from "react-i18next"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; -import { Bell, Check, History, Monitor, Pause, Play, RefreshCw, RotateCcw, Trash2, User, UserPlus, X } from "lucide-react"; +import { Bell, Check, History, Monitor, Pause, Play, RefreshCw, RotateCcw, Save, Trash2, User, UserPlus, X } from "lucide-react"; import { SCHEMES, type Scheme, type ThemePrefs } from "../lib/theme"; import { api, type Invite, type Me } from "../lib/api"; import { formatEta, quotaActionLabel } from "../lib/format"; import Avatar from "./Avatar"; -import { - configureNotifications, - getNotifSettings, - notify, - type NotifSettings, -} from "../lib/notifications"; -import { hintsEnabled, setHintsEnabled, subscribeHints } from "../lib/hints"; +import { notify, type NotifSettings } from "../lib/notifications"; import Tooltip from "./Tooltip"; import { useConfirm } from "./ConfirmProvider"; +// The Settings page edits server-persisted preferences as a draft: changes apply locally +// for instant preview but reach the server only on an explicit Save (or revert on Discard). +// App owns the draft + baseline (so the leave-the-page guard can see it); this controller is +// how the panel reads/writes it. See App.tsx. +export type PrefsSaveState = "idle" | "saving" | "saved" | "error"; +export interface PrefsController { + theme: ThemePrefs; + setTheme: (t: ThemePrefs) => void; + view: "grid" | "list"; + setView: (v: "grid" | "list") => void; + perf: boolean; + setPerf: (v: boolean) => void; + hints: boolean; + setHints: (v: boolean) => void; + notif: NotifSettings; + setNotif: (n: NotifSettings) => void; + dirty: boolean; + save: () => void; + discard: () => void; + saveState: PrefsSaveState; +} + type TabId = "appearance" | "notifications" | "sync" | "account"; const TABS: { id: TabId; icon: typeof Monitor }[] = [ { id: "appearance", icon: Monitor }, @@ -37,17 +53,11 @@ function loadSettingsTab(): TabId { // shown by the Header; the tab rail stays as in-page sub-navigation. export default function SettingsPanel({ me, - theme, - setTheme, - view, - setView, + prefs, onOpenWizard, }: { me: Me; - theme: ThemePrefs; - setTheme: (t: ThemePrefs) => void; - view: "grid" | "list"; - setView: (v: "grid" | "list") => void; + prefs: PrefsController; onOpenWizard: () => void; }) { const { t } = useTranslation(); @@ -59,47 +69,87 @@ export default function SettingsPanel({ return (
-
- {/* Vertical tab rail — never wraps; active is a clear accent pill. */} - + + {/* All tabs stacked in one grid cell so the panel sizes to the tallest tab + (stable height, no jump on switch); the active one is shown on top. */} +
+ {TABS.map((tabItem) => ( +
setTab(tabItem.id)} - className={`flex items-center gap-2 px-3 py-2 rounded-lg text-sm text-left transition ${ - active - ? "bg-accent text-accent-fg font-medium shadow-md shadow-accent/20" - : "text-muted hover:text-fg hover:bg-card/60" + className={`[grid-area:1/1] p-4 ${ + tab === tabItem.id ? "" : "invisible pointer-events-none" }`} > - - {t(`settings.tabs.${tabItem.id}`)} - - ); - })} - - - {/* All tabs stacked in one grid cell so the panel sizes to the tallest tab - (stable height, no jump on switch); the active one is shown on top. */} -
- {TABS.map((tabItem) => ( -
- {tabItem.id === "appearance" && ( - - )} - {tabItem.id === "notifications" && } - {tabItem.id === "sync" && } - {tabItem.id === "account" && } -
- ))} + {tabItem.id === "appearance" && } + {tabItem.id === "notifications" && } + {tabItem.id === "sync" && } + {tabItem.id === "account" && } +
+ ))} +
+ + {/* Save/Discard bar — spans the whole card (a change can come from any tab). Only the + Appearance/Notifications prefs are drafted; the Sync/Account tabs act immediately. */} + +
+
+ ); +} + +function PrefsSaveBar({ prefs }: { prefs: PrefsController }) { + const { t } = useTranslation(); + // Stay mounted while a transient "saved"/"error" message is fading, even after dirty clears. + if (!prefs.dirty && prefs.saveState === "idle") return null; + const saving = prefs.saveState === "saving"; + return ( +
+ + {prefs.saveState === "saved" + ? {t("settings.save.saved")} + : prefs.saveState === "error" + ? {t("settings.save.failed")} + : t("settings.save.unsaved")} + +
+ +
); @@ -155,35 +205,11 @@ function Switch({ checked, onChange }: { checked: boolean; onChange: (v: boolean ); } -const PERF_KEY = "subfeed.perfMode"; - -function Appearance({ - theme, - setTheme, - view, - setView, -}: { - theme: ThemePrefs; - setTheme: (t: ThemePrefs) => void; - view: "grid" | "list"; - setView: (v: "grid" | "list") => void; -}) { +function Appearance({ prefs }: { prefs: PrefsController }) { const { t } = useTranslation(); - const [perf, setPerf] = useState(() => localStorage.getItem(PERF_KEY) === "1"); - const hints = useSyncExternalStore(subscribeHints, hintsEnabled, hintsEnabled); - - useEffect(() => { - document.documentElement.dataset.perf = perf ? "1" : ""; - }, [perf]); - function togglePerf(v: boolean) { - setPerf(v); - localStorage.setItem(PERF_KEY, v ? "1" : "0"); - api.savePrefs({ performanceMode: v }).catch(() => {}); - } - function toggleHints(v: boolean) { - setHintsEnabled(v); - api.savePrefs({ hints: v }).catch(() => {}); - } + // Controlled by App's prefs draft: each change applies locally for preview but is only + // persisted on an explicit Save (handled by the panel's Save/Discard bar). + const { theme, setTheme, view, setView, perf, setPerf, hints, setHints } = prefs; return ( <> @@ -217,13 +243,13 @@ function Appearance({ label={t("settings.appearance.performanceMode")} hint={t("settings.appearance.performanceModeHint")} > - + - + @@ -243,15 +269,12 @@ function Appearance({ ); } -function Notifications() { +function Notifications({ prefs }: { prefs: PrefsController }) { const { t } = useTranslation(); - const [cfg, setCfg] = useState(() => getNotifSettings()); - function update(p: Partial) { - const next = { ...cfg, ...p }; - setCfg(next); - configureNotifications(next); - api.savePrefs({ notifications: next }).catch(() => {}); - } + // Controlled by App's prefs draft (live preview via configureNotifications there); the + // panel's Save bar persists it. The "send test" button below still fires immediately. + const { notif: cfg, setNotif } = prefs; + const update = (p: Partial) => setNotif({ ...cfg, ...p }); const autoOn = cfg.durationMs > 0; return ( diff --git a/frontend/src/i18n/locales/de/settings.json b/frontend/src/i18n/locales/de/settings.json index 26230d7..0b9677f 100644 --- a/frontend/src/i18n/locales/de/settings.json +++ b/frontend/src/i18n/locales/de/settings.json @@ -6,6 +6,19 @@ "sync": "Synchronisierung", "account": "Konto" }, + "save": { + "unsaved": "Nicht gespeicherte Änderungen", + "saving": "Speichern…", + "saved": "Einstellungen gespeichert", + "failed": "Speichern fehlgeschlagen", + "save": "Speichern", + "discard": "Verwerfen" + }, + "unsaved": { + "title": "Nicht gespeicherte Änderungen", + "message": "Du hast nicht gespeicherte Einstellungen. Verwerfen und verlassen?", + "discard": "Änderungen verwerfen" + }, "appearance": { "colorScheme": "Farbschema", "display": "Anzeige", diff --git a/frontend/src/i18n/locales/en/settings.json b/frontend/src/i18n/locales/en/settings.json index 0b39c94..b7e837f 100644 --- a/frontend/src/i18n/locales/en/settings.json +++ b/frontend/src/i18n/locales/en/settings.json @@ -6,6 +6,19 @@ "sync": "Sync", "account": "Account" }, + "save": { + "unsaved": "Unsaved changes", + "saving": "Saving…", + "saved": "Settings saved", + "failed": "Couldn't save", + "save": "Save", + "discard": "Discard" + }, + "unsaved": { + "title": "Unsaved changes", + "message": "You have unsaved settings. Discard them and leave?", + "discard": "Discard changes" + }, "appearance": { "colorScheme": "Color scheme", "display": "Display", diff --git a/frontend/src/i18n/locales/hu/settings.json b/frontend/src/i18n/locales/hu/settings.json index f58012b..0212e51 100644 --- a/frontend/src/i18n/locales/hu/settings.json +++ b/frontend/src/i18n/locales/hu/settings.json @@ -6,6 +6,19 @@ "sync": "Szinkronizálás", "account": "Fiók" }, + "save": { + "unsaved": "Nem mentett változások", + "saving": "Mentés…", + "saved": "Beállítások elmentve", + "failed": "Nem sikerült menteni", + "save": "Mentés", + "discard": "Elvetés" + }, + "unsaved": { + "title": "Nem mentett változások", + "message": "Vannak nem mentett beállításaid. Elveted őket és továbblépsz?", + "discard": "Változások elvetése" + }, "appearance": { "colorScheme": "Színséma", "display": "Megjelenítés", diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index be31fd8..33aae14 100644 --- a/frontend/src/lib/api.ts +++ b/frontend/src/lib/api.ts @@ -464,8 +464,9 @@ export const api = { videoDetail: (id: string): Promise => req(`/api/videos/${id}`), pauseSync: () => req("/api/sync/pause", { method: "POST" }), resumeSync: () => req("/api/sync/resume", { method: "POST" }), + // Overwriting preferences is idempotent, so let it retry on a transient gateway blip. savePrefs: (p: Record) => - req("/api/me/preferences", { method: "PUT", body: JSON.stringify(p) }), + req("/api/me/preferences", { method: "PUT", body: JSON.stringify(p) }, { idempotent: true }), // --- channel manager --- myStatus: (): Promise => req("/api/sync/my-status"),