From 3b11288c97062411e6098eb529791787a2657f2c Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Mon, 27 Jul 2026 20:49:15 +0200 Subject: [PATCH] fix(web): unsaved custom display settings are visible and recoverable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Everything else on the Displays page auto-applies — a preset click, the game-session choice, the experimental toggles — but the Custom block does not, and its Save button sat at the bottom of a block taller than most viewports. People edited, never scrolled far enough to find it, navigated away, and silently lost the lot. The draft is now compared against the last-seeded server value, and that one boolean drives the whole affordance: a badge in the card header (visible without scrolling), an amber ring plus a title on the block, and a sticky action bar pinned to the viewport for as long as any part of the block is on screen. The bar states which of the two states you are in rather than leaving it implied, Save disables when there is nothing to save, and Discard changes puts the stored policy back. Two ways edits could still vanish are closed: a preset click now confirms before overwriting pending edits, and a reload/close warns. Re-picking Custom while already on Custom is a no-op instead of re-seeding, which would otherwise fill in defaults for unset fields and report "unsaved changes" for a click that changed nothing. Adds a `warning` badge variant + --warning token for the pending state — attention, not failure, and distinct from the destructive red. Co-Authored-By: Claude Opus 5 (1M context) (cherry picked from commit c4318609c0da5ae1313081c5e488f685504f70d1) --- web/messages/de.json | 6 + web/messages/en.json | 6 + web/src/components/ui/badge.tsx | 3 + web/src/sections/Displays/DisplayCard.tsx | 357 +++++++++++++++++----- web/src/stories/Badge.stories.tsx | 1 + web/src/styles.css | 5 + 6 files changed, 310 insertions(+), 68 deletions(-) diff --git a/web/messages/de.json b/web/messages/de.json index caa7a743..591f4168 100644 --- a/web/messages/de.json +++ b/web/messages/de.json @@ -136,6 +136,12 @@ "display_preset_update": "Auf aktuelle Einstellungen aktualisieren", "display_preset_delete": "Löschen", "display_preset_delete_confirm": "Diese eigene Voreinstellung löschen?", + "display_custom_title": "Eigene Einstellungen", + "display_unsaved": "Nicht gespeicherte Änderungen", + "display_unsaved_hint": "Diese Optionen greifen erst, wenn du speicherst.", + "display_all_saved": "Alle Änderungen gespeichert", + "display_revert": "Änderungen verwerfen", + "display_discard_confirm": "Du hast nicht gespeicherte eigene Einstellungen. Verwerfen?", "clients_title": "Gekoppelte Geräte", "clients_empty": "Noch keine gekoppelten Geräte.", "clients_name": "Name", diff --git a/web/messages/en.json b/web/messages/en.json index 2c4e300c..11450235 100644 --- a/web/messages/en.json +++ b/web/messages/en.json @@ -136,6 +136,12 @@ "display_preset_update": "Update to current settings", "display_preset_delete": "Delete", "display_preset_delete_confirm": "Delete this custom preset?", + "display_custom_title": "Custom settings", + "display_unsaved": "Unsaved changes", + "display_unsaved_hint": "These options only take effect once you save.", + "display_all_saved": "All changes saved", + "display_revert": "Discard changes", + "display_discard_confirm": "You have unsaved custom settings. Discard them?", "clients_title": "Paired clients", "clients_empty": "No paired clients yet.", "clients_name": "Name", diff --git a/web/src/components/ui/badge.tsx b/web/src/components/ui/badge.tsx index 4bf9f77f..6ac845e0 100644 --- a/web/src/components/ui/badge.tsx +++ b/web/src/components/ui/badge.tsx @@ -12,6 +12,9 @@ const badgeVariants = cva( destructive: "border-transparent bg-destructive text-destructive-foreground", success: "border-transparent bg-[var(--success)] text-white", + // Attention, not failure — pending/unsaved state. Dark text: amber is a light + // colour in both themes, and white on it fails contrast. + warning: "border-transparent bg-[var(--warning)] text-black", outline: "text-foreground", }, }, diff --git a/web/src/sections/Displays/DisplayCard.tsx b/web/src/sections/Displays/DisplayCard.tsx index 2fae425a..3f1f83c8 100644 --- a/web/src/sections/Displays/DisplayCard.tsx +++ b/web/src/sections/Displays/DisplayCard.tsx @@ -2,10 +2,18 @@ import { useQueryClient } from "@tanstack/react-query"; import { Button } from "@unom/ui/button"; import { toast } from "@unom/ui/toast"; import { Pencil, Plus, RefreshCw, Trash2 } from "lucide-react"; -import { type FC, type MouseEvent, type ReactNode, useEffect, useRef, useState } from "react"; import { - getGetDisplayStateQueryKey, + type FC, + type MouseEvent, + type ReactNode, + useEffect, + useRef, + useState, +} from "react"; +import { ApiError } from "@/api/fetcher"; +import { getGetDisplaySettingsQueryKey, + getGetDisplayStateQueryKey, useCreateCustomPreset, useDeleteCustomPreset, useGetDisplaySettings, @@ -28,7 +36,6 @@ import type { Preset, Topology, } from "@/api/gen/model"; -import { ApiError } from "@/api/fetcher"; import { QueryState } from "@/components/query-state"; import { Badge } from "@/components/ui/badge"; import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; @@ -89,15 +96,47 @@ export const DisplaySection: FC = () => { }, ); + // Pending edits: the Custom fields do NOT auto-apply (unlike a preset click or an experimental + // toggle), so the draft can silently diverge from what the host is actually running. Reading the + // ref during render is safe here because every write to it is paired with a `setDraft`, so a + // changed seed always comes with the re-render that reads it. + const dirty = + draft !== null && + seeded.current !== null && + !deepEqual(draft, seeded.current); + const revert = () => { + if (seeded.current) setDraft(seeded.current); + }; + + // Last line of defence: a reload/close with pending edits loses them silently otherwise. The + // browser shows its own generic wording — the text is ignored, only returning a value counts. + useEffect(() => { + if (!dirty) return; + const warn = (e: BeforeUnloadEvent) => e.preventDefault(); + window.addEventListener("beforeunload", warn); + return () => window.removeEventListener("beforeunload", warn); + }, [dirty]); + return (
- {m.display_config_title()} +
+ {m.display_config_title()} + {/* Visible without scrolling to the save button — the card is taller than the + viewport, which is exactly how the pending edits went unnoticed. */} + {dirty && {m.display_unsaved()}} +
-

{m.host_displays_help()}

- +

+ {m.host_displays_help()} +

+ {q.data && draft && ( { customPresets={q.data.custom_presets} apply={apply} busy={save.isPending} + dirty={dirty} + revert={revert} error={apiErrorMessage(save.error)} /> )} @@ -141,8 +182,22 @@ const DisplayForm: FC<{ customPresets: CustomPreset[]; apply: (p: DisplayPolicy) => void; busy: boolean; + /** The draft differs from what the host has stored — drives the save bar + the discard guard. */ + dirty: boolean; + /** Throw the draft away and go back to the stored policy. */ + revert: () => void; error?: string; -}> = ({ draft, setDraft, presets, customPresets, apply, busy, error }) => { +}> = ({ + draft, + setDraft, + presets, + customPresets, + apply, + busy, + dirty, + revert, + error, +}) => { const qc = useQueryClient(); const createPreset = useCreateCustomPreset(); const updatePreset = useUpdateCustomPreset(); @@ -169,11 +224,19 @@ const DisplayForm: FC<{ max_displays: draft.max_displays ?? 4, }; const effective: EffectivePolicy = - (isCustom ? undefined : presets.find((p) => p.id === preset)?.fields) ?? customFields; + (isCustom ? undefined : presets.find((p) => p.id === preset)?.fields) ?? + customFields; // The five named presets apply in ONE click; "Custom" reveals the fields, seeded from the current // effective behavior (nothing changes until you Save). const pickPreset = (id: string) => { + // A preset click overwrites the whole policy, so hand-edits that were never saved would + // vanish without a word — the same failure as not finding the Save button, one click later. + if (dirty && id !== "custom" && !confirm(m.display_discard_confirm())) + return; + // Already hand-editing: re-seeding would fill in defaults for fields the stored policy + // leaves unset and flag "unsaved changes" for a click that changed nothing. + if (id === "custom" && isCustom) return; if (id === "custom") { setDraft({ version: 1, @@ -201,7 +264,8 @@ const DisplayForm: FC<{ // Applying a custom preset writes a `Custom` policy carrying its saved fields + game-session (the // one axis a preset DOES set) — the host has no separate apply route (design/gamemode-and-…). - const applyCustomPreset = (p: CustomPreset) => + const applyCustomPreset = (p: CustomPreset) => { + if (dirty && !confirm(m.display_discard_confirm())) return; apply({ version: 1, preset: "custom", @@ -216,6 +280,7 @@ const DisplayForm: FC<{ // (found on-glass, .136). Every orthogonal axis has to be listed. capture_monitor: draft.capture_monitor ?? null, }); + }; // A custom card is "current" when the in-force policy is a Custom one whose fields + game-session // value-match this preset (there is no id on DisplayPolicy — match by value). @@ -231,7 +296,11 @@ const DisplayForm: FC<{ if (!name) return; // cancelled or empty createPreset.mutate( { - data: { name, fields: effective, game_session: draft.game_session ?? "auto" }, + data: { + name, + fields: effective, + game_session: draft.game_session ?? "auto", + }, }, { onSuccess: invalidateSettings }, ); @@ -240,7 +309,14 @@ const DisplayForm: FC<{ const name = prompt(m.display_preset_name(), p.name)?.trim(); if (!name) return; updatePreset.mutate( - { id: p.id, data: { name, fields: p.fields, game_session: p.game_session ?? "auto" } }, + { + id: p.id, + data: { + name, + fields: p.fields, + game_session: p.game_session ?? "auto", + }, + }, { onSuccess: invalidateSettings }, ); }; @@ -248,7 +324,11 @@ const DisplayForm: FC<{ updatePreset.mutate( { id: p.id, - data: { name: p.name, fields: effective, game_session: draft.game_session ?? "auto" }, + data: { + name: p.name, + fields: effective, + game_session: draft.game_session ?? "auto", + }, }, { onSuccess: invalidateSettings }, ); @@ -259,21 +339,27 @@ const DisplayForm: FC<{ const ka = customFields.keep_alive; // The duration value, remembered across the Off/Keep toggle so switching back restores it. - const [keepSecs, setKeepSecs] = useState(ka.mode === "duration" ? ka.seconds : 300); + const [keepSecs, setKeepSecs] = useState( + ka.mode === "duration" ? ka.seconds : 300, + ); return (
{/* One-click presets — a 2-up grid so each has room to breathe */}
- +
{PRESET_ORDER.map((id) => { const p = presets.find((x) => x.id === id); const fields = id === "custom" ? undefined : p?.fields; - const summary = id === "custom" ? m.display_custom_desc() : p?.summary; + const summary = + id === "custom" ? m.display_custom_desc() : p?.summary; // The built-in "Custom" card is the hand-edit mode; when the active Custom policy // value-matches a saved preset, that preset's card owns the "current" ring instead. - const selected = preset === id && !(id === "custom" && anyCustomSelected); + const selected = + preset === id && !(id === "custom" && anyCustomSelected); const soon = DISABLED_PRESETS.has(id); const disabled = busy || soon; const pick = () => { @@ -310,18 +396,30 @@ const DisplayForm: FC<{ )} {selected && ( - {m.display_preset_current()} + + {m.display_preset_current()} + )}
{summary && ( -

{summary}

+

+ {summary} +

)} {fields && (
- {fmtKeepAlive(fields.keep_alive)} - {tr(TOPOLOGY_LABEL, fields.topology)} - {tr(CONFLICT_LABEL, fields.mode_conflict)} - {tr(IDENTITY_LABEL, fields.identity)} + + {fmtKeepAlive(fields.keep_alive)} + + + {tr(TOPOLOGY_LABEL, fields.topology)} + + + {tr(CONFLICT_LABEL, fields.mode_conflict)} + + + {tr(IDENTITY_LABEL, fields.identity)} +
)} @@ -364,20 +462,40 @@ const DisplayForm: FC<{
)} {presetError && ( -

{presetError}

+

+ {presetError} +

)}
- {/* Custom: every option by hand */} + {/* Custom: every option by hand. Unlike everything else on this page these fields do NOT + auto-apply, so the block is titled, ringed while dirty, and ends in a sticky save bar. */} {isCustom && ( -
- +
+
+

+ {m.display_custom_title()} +

+ {dirty && {m.display_unsaved()}} +
+ +
@@ -386,7 +504,10 @@ const DisplayForm: FC<{ variant={ka.mode === "duration" ? "default" : "outline"} disabled={busy} onClick={() => - setDraft({ ...draft, keep_alive: { mode: "duration", seconds: keepSecs } }) + setDraft({ + ...draft, + keep_alive: { mode: "duration", seconds: keepSecs }, + }) } > {m.display_keep_alive_keep()} @@ -395,7 +516,9 @@ const DisplayForm: FC<{ size="sm" variant={ka.mode === "forever" ? "default" : "outline"} disabled={busy} - onClick={() => setDraft({ ...draft, keep_alive: { mode: "forever" } })} + onClick={() => + setDraft({ ...draft, keep_alive: { mode: "forever" } }) + } > {m.display_keep_alive_forever()} @@ -410,7 +533,10 @@ const DisplayForm: FC<{ onChange={(e) => { const n = Math.max(0, Number(e.target.value) || 0); setKeepSecs(n); - setDraft({ ...draft, keep_alive: { mode: "duration", seconds: n } }); + setDraft({ + ...draft, + keep_alive: { mode: "duration", seconds: n }, + }); }} /> @@ -437,7 +563,9 @@ const DisplayForm: FC<{ options={["separate", "steal", "join", "reject"]} labels={CONFLICT_LABEL} disabled={busy} - onPick={(v) => setDraft({ ...draft, mode_conflict: v as ModeConflict })} + onPick={(v) => + setDraft({ ...draft, mode_conflict: v as ModeConflict }) + } /> setDraft({ ...draft, - layout: { mode: v as LayoutMode, positions: draft.layout?.positions ?? {} }, + layout: { + mode: v as LayoutMode, + positions: draft.layout?.positions ?? {}, + }, }) } /> @@ -474,14 +605,39 @@ const DisplayForm: FC<{ onChange={(e) => setDraft({ ...draft, - max_displays: Math.min(16, Math.max(1, Number(e.target.value) || 1)), + max_displays: Math.min( + 16, + Math.max(1, Number(e.target.value) || 1), + ), }) } /> -
- + )} +
@@ -537,15 +693,27 @@ const DisplayForm: FC<{ {/* What's in force right now */}
- {m.display_effective()}: + + {m.display_effective()}: + {fmtKeepAlive(effective.keep_alive)} - {tr(TOPOLOGY_LABEL, effective.topology)} - {tr(CONFLICT_LABEL, effective.mode_conflict)} - {tr(IDENTITY_LABEL, effective.identity)} - {tr(LAYOUT_LABEL, effective.layout.mode)} + + {tr(TOPOLOGY_LABEL, effective.topology)} + + + {tr(CONFLICT_LABEL, effective.mode_conflict)} + + + {tr(IDENTITY_LABEL, effective.identity)} + + + {tr(LAYOUT_LABEL, effective.layout.mode)} + {`${effective.max_displays}×`} {(draft.game_session ?? "auto") === "dedicated" && ( - {m.display_game_session_dedicated()} + + {m.display_game_session_dedicated()} + )} {(draft.ddc_power_off ?? false) && ( {m.display_ddc_badge()} @@ -555,8 +723,12 @@ const DisplayForm: FC<{ )}
-

{m.display_pending_note()}

- {error &&

{error}

} +

+ {m.display_pending_note()} +

+ {error && ( +

{error}

+ )}
); }; @@ -571,7 +743,9 @@ const Field: FC<{ label: string; help?: string; children: ReactNode }> = ({
{children} - {help &&

{help}

} + {help && ( +

{help}

+ )}
); @@ -682,9 +856,13 @@ const CustomPresetCard: FC<{ )} >
- {preset.name} + + {preset.name} +
- {selected && {m.display_preset_current()}} + {selected && ( + {m.display_preset_current()} + )}
@@ -744,14 +926,21 @@ const LiveDisplays: FC = () => { const doRelease = (slot?: number) => release.mutate( { data: { slot: slot ?? null } }, - { onSuccess: () => qc.invalidateQueries({ queryKey: getGetDisplayStateQueryKey() }) }, + { + onSuccess: () => + qc.invalidateQueries({ queryKey: getGetDisplayStateQueryKey() }), + }, ); return (
{/* Wrap in QueryState (like the settings card) so a failed/in-flight `/display/state` fetch surfaces as loading/error instead of masquerading as "no live displays". */} - + {kept.length > 0 && (
)} {displays.length === 0 ? ( -

{m.display_none_live()}

+

+ {m.display_none_live()} +

) : (
    {displays.map((d) => ( @@ -790,18 +981,24 @@ const LiveDisplays: FC = () => { * `PUT /display/layout`, which switches the host to a manual layout and applies from the next connect. * Shown only for a ≥2-display group — arranging a single display is moot. */ -const DisplayArrangement: FC<{ displays: ApiDisplayInfo[] }> = ({ displays }) => { +const DisplayArrangement: FC<{ displays: ApiDisplayInfo[] }> = ({ + displays, +}) => { const qc = useQueryClient(); const saveLayout = useSetDisplayLayout(); // Only displays with a stable identity slot can be pinned (shared/anonymous ones have no key). const arrangeable = displays.filter((d) => d.identity_slot != null); // Local edit buffer keyed by identity-slot string → {x, y}, seeded once from the current positions. - const [pos, setPos] = useState | null>(null); + const [pos, setPos] = useState | null>(null); useEffect(() => { if (pos === null && arrangeable.length > 0) { const seed: Record = {}; - for (const d of arrangeable) seed[String(d.identity_slot)] = { x: d.x, y: d.y }; + for (const d of arrangeable) + seed[String(d.identity_slot)] = { x: d.x, y: d.y }; setPos(seed); } }, [arrangeable, pos]); @@ -831,15 +1028,21 @@ const DisplayArrangement: FC<{ displays: ApiDisplayInfo[] }> = ({ displays }) => return (

    {m.display_arrange()}

    -

    {m.display_arrange_help()}

    +

    + {m.display_arrange_help()} +

    {arrangeable.map((d) => { const slot = d.identity_slot as number; const p = cur[String(slot)] ?? { x: d.x, y: d.y }; return ( -
    +
    - {d.mode} #{slot} + {d.mode}{" "} + #{slot}
    ); @@ -879,11 +1086,11 @@ const DisplayArrangement: FC<{ displays: ApiDisplayInfo[] }> = ({ displays }) => ); }; -const DisplayRow: FC<{ d: ApiDisplayInfo; busy: boolean; onRelease: () => void }> = ({ - d, - busy, - onRelease, -}) => { +const DisplayRow: FC<{ + d: ApiDisplayInfo; + busy: boolean; + onRelease: () => void; +}> = ({ d, busy, onRelease }) => { const active = d.state === "active"; const stateLabel = d.state === "active" @@ -898,7 +1105,9 @@ const DisplayRow: FC<{ d: ApiDisplayInfo; busy: boolean; onRelease: () => void } {d.mode} {stateLabel} {active && d.sessions > 0 && ( - {m.display_sessions({ count: d.sessions })} + + {m.display_sessions({ count: d.sessions })} + )}
    @@ -974,17 +1183,29 @@ const GAME_SESSION_LABEL: Record string> = { * (handles the nested `keep_alive` variants + `layout.positions` map; key order doesn't matter). */ const deepEqual = (a: unknown, b: unknown): boolean => { if (a === b) return true; - if (typeof a !== "object" || typeof b !== "object" || a === null || b === null) return false; + if ( + typeof a !== "object" || + typeof b !== "object" || + a === null || + b === null + ) + return false; const ak = Object.keys(a as object); const bk = Object.keys(b as object); if (ak.length !== bk.length) return false; return ak.every((k) => - deepEqual((a as Record)[k], (b as Record)[k]), + deepEqual( + (a as Record)[k], + (b as Record)[k], + ), ); }; /** Look up a localized label, tolerating an unknown/undefined key (falls back to the raw value). */ -const tr = (map: Record string>, key: string | null | undefined): string => { +const tr = ( + map: Record string>, + key: string | null | undefined, +): string => { const fn = key == null ? undefined : map[key]; return fn ? fn() : String(key ?? ""); }; diff --git a/web/src/stories/Badge.stories.tsx b/web/src/stories/Badge.stories.tsx index 33733e89..a973ed67 100644 --- a/web/src/stories/Badge.stories.tsx +++ b/web/src/stories/Badge.stories.tsx @@ -5,6 +5,7 @@ const VARIANTS = [ "default", "secondary", "success", + "warning", "destructive", "outline", ] as const; diff --git a/web/src/styles.css b/web/src/styles.css index fae655ab..7a8cd5fe 100644 --- a/web/src/styles.css +++ b/web/src/styles.css @@ -56,6 +56,9 @@ --primary-foreground: #ffffff; --success: oklch(0.6 0.14 160); + /* Amber — "needs your attention but nothing is broken" (unsaved edits), the badge + counterpart of the inline amber warning text. */ + --warning: oklch(0.72 0.15 75); --destructive: oklch(0.55 0.22 18); --destructive-foreground: #ffffff; @@ -101,6 +104,7 @@ --primary-foreground: #141019; --success: oklch(0.7 0.15 160); + --warning: oklch(0.78 0.16 75); --destructive: oklch(0.62 0.21 18); --destructive-foreground: oklch(0.985 0 0); } @@ -142,6 +146,7 @@ --color-destructive: var(--destructive); --color-destructive-foreground: var(--destructive-foreground); --color-success: var(--success); + --color-warning: var(--warning); --color-border: var(--border); --color-input: var(--input); --color-ring: var(--ring);