From a7b7327ce595627b59f30497221d1401a8eb196a Mon Sep 17 00:00:00 2001 From: Nicola Bottini Date: Thu, 16 Jul 2026 12:22:18 -0400 Subject: [PATCH 1/5] feat: autosave canvas after configurable inactivity delay MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds an opt-in autosave feature (disabled by default) that silently saves the canvas after a configurable period of inactivity. - frontend/src/utils/autosaveSettings.ts (new) — localStorage-backed setting {enabled, delay} with pub/sub CustomEvent pattern (mirrors alignmentSettings.ts). Key: homelable.autosave. - frontend/src/App.tsx: * handleSave accepts optional { silent?: boolean } — skips the success toast when called from autosave to avoid noise on every periodic save * autosave state (useState + subscribeAutosaveSettings) * debounce useEffect: resets on nodes/edges change; when quiet for autosave.delay seconds and hasUnsavedChanges is true, calls handleSaveRef.current silently - frontend/src/components/modals/SettingsModal.tsx: * Canvas section: Autosave canvas toggle + Save after delay selector (3/5/10/30/60 s). Changes persist immediately to localStorage via writeAutosaveSettings + propagate cross-tab via CustomEvent. Default: disabled — no behaviour change for existing users. Manual Ctrl+S and the Save button continue to work normally. Errors always show a toast regardless of the silent flag. --- frontend/src/App.tsx | 20 ++++++-- .../src/components/modals/SettingsModal.tsx | 48 +++++++++++++++++++ frontend/src/utils/autosaveSettings.ts | 43 +++++++++++++++++ 3 files changed, 107 insertions(+), 4 deletions(-) create mode 100644 frontend/src/utils/autosaveSettings.ts diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index d46f8e4..f28626e 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -37,6 +37,7 @@ import { ScanHistoryModal } from '@/components/modals/ScanHistoryModal' import { ShortcutsModal } from '@/components/modals/ShortcutsModal' import { ConfirmAddToGroupModal } from '@/components/modals/ConfirmAddToGroupModal' import { useCanvasStore } from '@/stores/canvasStore' +import { readAutosaveSettings, subscribeAutosaveSettings, type AutosaveSettings } from '@/utils/autosaveSettings' import { useDesignStore } from '@/stores/designStore' import { useAuthStore } from '@/stores/authStore' import { useThemeStore } from '@/stores/themeStore' @@ -53,7 +54,7 @@ import { buildProxmoxClusterEdges } from '@/components/proxmox/clusterEdges' const STANDALONE = import.meta.env.VITE_STANDALONE === 'true' export default function App() { - const { loadCanvas, markSaved, markUnsaved, selectedNodeId, selectedNodeIds, addNode, updateNode, deleteNode, onConnect, updateEdge, deleteEdge, setProxmoxContainerMode, setNodeZIndex, editingGroupRectId, setEditingGroupRectId, editingTextId, setEditingTextId, nodes, edges, snapshotHistory, undo, redo, addToGroup, addToContainer, floorMap, setFloorMap } = useCanvasStore() + const { loadCanvas, markSaved, markUnsaved, hasUnsavedChanges, selectedNodeId, selectedNodeIds, addNode, updateNode, deleteNode, onConnect, updateEdge, deleteEdge, setProxmoxContainerMode, setNodeZIndex, editingGroupRectId, setEditingGroupRectId, editingTextId, setEditingTextId, nodes, edges, snapshotHistory, undo, redo, addToGroup, addToContainer, floorMap, setFloorMap } = useCanvasStore() const canvasRef = useRef(null) const { isAuthenticated } = useAuthStore() const { activeTheme, setTheme, customStyle, setCustomStyle } = useThemeStore() @@ -61,6 +62,9 @@ export default function App() { useStatusPolling() + const [autosave, setAutosave] = useState(readAutosaveSettings) + useEffect(() => subscribeAutosaveSettings(setAutosave), []) + const [themeModalOpen, setThemeModalOpen] = useState(false) const [styleEditorType, setStyleEditorType] = useState(null) const [searchOpen, setSearchOpen] = useState(false) @@ -93,7 +97,7 @@ export default function App() { // Declare handleSave before the Ctrl+S effect so it is in scope. // Returns true on success, false on failure — the design-switch effect relies // on this to avoid loading (and clobbering) the canvas when a save fails. - const handleSave = useCallback(async (designIdOverride?: string): Promise => { + const handleSave = useCallback(async (designIdOverride?: string, options?: { silent?: boolean }): Promise => { try { const saveDesignId = designIdOverride ?? activeDesignId if (STANDALONE) { @@ -101,7 +105,7 @@ export default function App() { // Floor plans are backend-only (upload/serve), so standalone never persists one. standaloneStorage.saveCanvas(saveDesignId, { nodes, edges, theme_id: activeTheme, custom_style: customStyle }) markSaved() - toast.success('Canvas saved') + if (!options?.silent) toast.success('Canvas saved') return true } const nodesToSave = nodes.map(serializeNode) @@ -110,7 +114,7 @@ export default function App() { if (floorMap) viewport.floor_map = floorMap await canvasApi.save({ nodes: nodesToSave, edges: edgesToSave, viewport, custom_style: customStyle, design_id: saveDesignId }) markSaved() - toast.success('Canvas saved') + if (!options?.silent) toast.success('Canvas saved') return true } catch { toast.error('Save failed') @@ -122,6 +126,14 @@ export default function App() { const handleSaveRef = useRef(handleSave) useEffect(() => { handleSaveRef.current = handleSave }, [handleSave]) + // Autosave: debounce — resets on every node/edge change, fires after `delay` seconds + // of inactivity if there are unsaved changes and autosave is enabled. + useEffect(() => { + if (!autosave.enabled || !hasUnsavedChanges) return + const t = setTimeout(() => { void handleSaveRef.current(undefined, { silent: true }) }, autosave.delay * 1000) + return () => clearTimeout(t) + }, [nodes, edges, autosave.enabled, autosave.delay, hasUnsavedChanges]) + const loadCanvasFromApi = useCallback(async (designId?: string) => { try { const res = await canvasApi.load(designId) diff --git a/frontend/src/components/modals/SettingsModal.tsx b/frontend/src/components/modals/SettingsModal.tsx index fa16651..58f98a5 100644 --- a/frontend/src/components/modals/SettingsModal.tsx +++ b/frontend/src/components/modals/SettingsModal.tsx @@ -18,6 +18,12 @@ import { writeAlignmentSettings, subscribeAlignmentSettings, } from '@/utils/alignmentSettings' +import { + type AutosaveSettings, + readAutosaveSettings, + writeAutosaveSettings, + subscribeAutosaveSettings, +} from '@/utils/autosaveSettings' const STANDALONE = import.meta.env.VITE_STANDALONE === 'true' @@ -124,6 +130,7 @@ export function SettingsModal({ open, onClose }: SettingsModalProps) { const [zwInterval, setZwInterval] = useState(3600) const [zwSyncing, setZwSyncing] = useState(false) const [alignment, setAlignment] = useState(readAlignmentSettings) + const [autosave, setAutosave] = useState(readAutosaveSettings) const hideIp = useCanvasStore((s) => s.hideIp) const setHideIp = useCanvasStore((s) => s.setHideIp) @@ -160,6 +167,7 @@ export function SettingsModal({ open, onClose }: SettingsModalProps) { }, [open]) useEffect(() => subscribeAlignmentSettings(setAlignment), []) + useEffect(() => subscribeAutosaveSettings(setAutosave), []) const updateAlignment = (patch: Partial) => { const next = { ...alignment, ...patch } @@ -167,6 +175,12 @@ export function SettingsModal({ open, onClose }: SettingsModalProps) { writeAlignmentSettings(next) } + const updateAutosave = (patch: Partial) => { + const next = { ...autosave, ...patch } + setAutosave(next) + writeAutosaveSettings(next) + } + const handleSyncNow = async () => { setPmSyncing(true) try { @@ -353,6 +367,40 @@ export function SettingsModal({ open, onClose }: SettingsModalProps) { Distance at which dragged nodes snap to neighbours. Hold Alt while dragging to disable.

+ + + +
+ +
+ + of inactivity +
+

+ Saves silently after this many seconds with no changes. Manual Ctrl+S still works. +

+
diff --git a/frontend/src/utils/autosaveSettings.ts b/frontend/src/utils/autosaveSettings.ts new file mode 100644 index 0000000..3b54941 --- /dev/null +++ b/frontend/src/utils/autosaveSettings.ts @@ -0,0 +1,43 @@ +// Persisted client-side autosave preference. +// Kept in localStorage (per-user UI preference, not canvas data). +// Same-tab updates propagate via a CustomEvent so App.tsx and SettingsModal +// can stay in sync without a global store. + +export interface AutosaveSettings { + enabled: boolean + delay: number // seconds of inactivity before auto-saving +} + +export const DEFAULT_AUTOSAVE_SETTINGS: AutosaveSettings = { enabled: false, delay: 5 } + +const KEY = 'homelable.autosave' +const EVENT = 'homelable:autosave-settings-changed' + +export function readAutosaveSettings(): AutosaveSettings { + try { + const raw = localStorage.getItem(KEY) + if (!raw) return DEFAULT_AUTOSAVE_SETTINGS + const parsed = JSON.parse(raw) as Partial + return { + enabled: parsed.enabled ?? DEFAULT_AUTOSAVE_SETTINGS.enabled, + delay: typeof parsed.delay === 'number' && parsed.delay > 0 ? parsed.delay : DEFAULT_AUTOSAVE_SETTINGS.delay, + } + } catch { + return DEFAULT_AUTOSAVE_SETTINGS + } +} + +export function writeAutosaveSettings(s: AutosaveSettings): void { + try { + localStorage.setItem(KEY, JSON.stringify(s)) + window.dispatchEvent(new CustomEvent(EVENT, { detail: s })) + } catch { + /* quota / SSR */ + } +} + +export function subscribeAutosaveSettings(listener: (s: AutosaveSettings) => void): () => void { + const handler = (e: Event) => listener((e as CustomEvent).detail) + window.addEventListener(EVENT, handler) + return () => window.removeEventListener(EVENT, handler) +} From 310b9cb3fde8d1504012a3c19652d4534ade07fa Mon Sep 17 00:00:00 2001 From: Pouzor Date: Fri, 17 Jul 2026 11:16:56 +0200 Subject: [PATCH 2/5] fix(canvas): guard autosave against design-switch clobber, add tests Extract the debounced autosave into a useAutosave hook. Pin the active design id when the timer is armed and re-check it at fire time: if the user switched designs while the timer was pending, the in-memory canvas belongs to a different design, so skip the save instead of writing it under the wrong design id. Also skip when there is no active design. Add unit tests for autosaveSettings (persistence, validation, pub/sub) and useAutosave (debounce, enable/unsaved/design guards, switch skip). ha-relevant: maybe --- frontend/src/App.tsx | 19 ++-- .../src/hooks/__tests__/useAutosave.test.ts | 94 +++++++++++++++++++ frontend/src/hooks/useAutosave.ts | 65 +++++++++++++ .../utils/__tests__/autosaveSettings.test.ts | 58 ++++++++++++ 4 files changed, 229 insertions(+), 7 deletions(-) create mode 100644 frontend/src/hooks/__tests__/useAutosave.test.ts create mode 100644 frontend/src/hooks/useAutosave.ts create mode 100644 frontend/src/utils/__tests__/autosaveSettings.test.ts diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index f28626e..fba646d 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -38,6 +38,7 @@ import { ShortcutsModal } from '@/components/modals/ShortcutsModal' import { ConfirmAddToGroupModal } from '@/components/modals/ConfirmAddToGroupModal' import { useCanvasStore } from '@/stores/canvasStore' import { readAutosaveSettings, subscribeAutosaveSettings, type AutosaveSettings } from '@/utils/autosaveSettings' +import { useAutosave } from '@/hooks/useAutosave' import { useDesignStore } from '@/stores/designStore' import { useAuthStore } from '@/stores/authStore' import { useThemeStore } from '@/stores/themeStore' @@ -126,13 +127,17 @@ export default function App() { const handleSaveRef = useRef(handleSave) useEffect(() => { handleSaveRef.current = handleSave }, [handleSave]) - // Autosave: debounce — resets on every node/edge change, fires after `delay` seconds - // of inactivity if there are unsaved changes and autosave is enabled. - useEffect(() => { - if (!autosave.enabled || !hasUnsavedChanges) return - const t = setTimeout(() => { void handleSaveRef.current(undefined, { silent: true }) }, autosave.delay * 1000) - return () => clearTimeout(t) - }, [nodes, edges, autosave.enabled, autosave.delay, hasUnsavedChanges]) + // Debounced, opt-in autosave. Pins the active design when armed and re-checks + // it at fire time so a mid-switch save can't clobber the wrong design. + useAutosave({ + enabled: autosave.enabled, + delaySeconds: autosave.delay, + hasUnsavedChanges, + activeDesignId, + changeSignals: [nodes, edges], + getActiveDesignId: () => useDesignStore.getState().activeDesignId, + onSave: (designId) => { void handleSaveRef.current(designId, { silent: true }) }, + }) const loadCanvasFromApi = useCallback(async (designId?: string) => { try { diff --git a/frontend/src/hooks/__tests__/useAutosave.test.ts b/frontend/src/hooks/__tests__/useAutosave.test.ts new file mode 100644 index 0000000..a299013 --- /dev/null +++ b/frontend/src/hooks/__tests__/useAutosave.test.ts @@ -0,0 +1,94 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest' +import { renderHook } from '@testing-library/react' +import { useAutosave } from '../useAutosave' + +interface Args { + enabled?: boolean + delaySeconds?: number + hasUnsavedChanges?: boolean + activeDesignId?: string | null + changeSignals?: unknown[] + getActiveDesignId?: () => string | null + onSave?: (designId: string) => void +} + +function args(over: Args = {}) { + const activeDesignId = 'activeDesignId' in over ? (over.activeDesignId ?? null) : 'design-a' + return { + enabled: over.enabled ?? true, + delaySeconds: over.delaySeconds ?? 5, + hasUnsavedChanges: over.hasUnsavedChanges ?? true, + activeDesignId, + changeSignals: over.changeSignals ?? [[], []], + getActiveDesignId: over.getActiveDesignId ?? (() => activeDesignId), + onSave: over.onSave ?? vi.fn(), + } +} + +describe('useAutosave', () => { + beforeEach(() => vi.useFakeTimers()) + afterEach(() => vi.useRealTimers()) + + it('saves after the inactivity delay when enabled with unsaved changes', () => { + const onSave = vi.fn() + renderHook(() => useAutosave(args({ onSave, delaySeconds: 5 }))) + expect(onSave).not.toHaveBeenCalled() + vi.advanceTimersByTime(4999) + expect(onSave).not.toHaveBeenCalled() + vi.advanceTimersByTime(1) + expect(onSave).toHaveBeenCalledExactlyOnceWith('design-a') + }) + + it('does nothing when disabled', () => { + const onSave = vi.fn() + renderHook(() => useAutosave(args({ onSave, enabled: false }))) + vi.advanceTimersByTime(60_000) + expect(onSave).not.toHaveBeenCalled() + }) + + it('does nothing when there are no unsaved changes', () => { + const onSave = vi.fn() + renderHook(() => useAutosave(args({ onSave, hasUnsavedChanges: false }))) + vi.advanceTimersByTime(60_000) + expect(onSave).not.toHaveBeenCalled() + }) + + it('does nothing when there is no active design', () => { + const onSave = vi.fn() + renderHook(() => useAutosave(args({ onSave, activeDesignId: null }))) + vi.advanceTimersByTime(60_000) + expect(onSave).not.toHaveBeenCalled() + }) + + it('debounces: a change before the delay resets the timer', () => { + const onSave = vi.fn() + const { rerender } = renderHook((p: Args) => useAutosave(args(p)), { + initialProps: { onSave, changeSignals: [1] }, + }) + vi.advanceTimersByTime(4000) + rerender({ onSave, changeSignals: [2] }) // edit resets debounce + vi.advanceTimersByTime(4000) + expect(onSave).not.toHaveBeenCalled() + vi.advanceTimersByTime(1000) + expect(onSave).toHaveBeenCalledExactlyOnceWith('design-a') + }) + + it('skips the save if the active design changed while the timer was pending', () => { + const onSave = vi.fn() + // Pinned at arm time = 'design-a', but live id has moved on to 'design-b'. + renderHook(() => + useAutosave(args({ onSave, activeDesignId: 'design-a', getActiveDesignId: () => 'design-b' })), + ) + vi.advanceTimersByTime(5000) + expect(onSave).not.toHaveBeenCalled() + }) + + it('saves under the pinned design id, not the live one, when they still match', () => { + const onSave = vi.fn() + renderHook(() => + useAutosave(args({ onSave, activeDesignId: 'design-a', getActiveDesignId: () => 'design-a' })), + ) + vi.advanceTimersByTime(5000) + expect(onSave).toHaveBeenCalledExactlyOnceWith('design-a') + }) +}) diff --git a/frontend/src/hooks/useAutosave.ts b/frontend/src/hooks/useAutosave.ts new file mode 100644 index 0000000..e4ac35c --- /dev/null +++ b/frontend/src/hooks/useAutosave.ts @@ -0,0 +1,65 @@ +import { useEffect, useRef } from 'react' + +interface UseAutosaveOptions { + /** Whether autosave is enabled (user opt-in). */ + enabled: boolean + /** Inactivity delay in seconds before firing a save. */ + delaySeconds: number + /** True when the canvas has edits not yet persisted. */ + hasUnsavedChanges: boolean + /** The design the in-memory canvas currently belongs to. */ + activeDesignId: string | null + /** + * Values that represent canvas edits (e.g. nodes, edges). Any change to one + * of these resets the debounce timer, so the save only fires after a quiet + * period. Kept separate from the trigger flags so the caller controls exactly + * what counts as "activity". + */ + changeSignals: unknown[] + /** + * Reads the *live* active design id at fire time. Used to detect that the user + * switched designs while the timer was pending — if so, the in-memory canvas + * now belongs to a different design and saving it under the pinned id would + * clobber the wrong design, so the save is skipped. + */ + getActiveDesignId: () => string | null + /** Persist the canvas under the given design id. */ + onSave: (designId: string) => void +} + +/** + * Debounced canvas autosave. Fires `onSave(designId)` after `delaySeconds` of + * inactivity when enabled and there are unsaved changes. Opt-in only — the + * caller decides whether `enabled` is set (see ADR: autosave defaults to off). + */ +export function useAutosave({ + enabled, + delaySeconds, + hasUnsavedChanges, + activeDesignId, + changeSignals, + getActiveDesignId, + onSave, +}: UseAutosaveOptions): void { + // Keep the latest callbacks in refs so the timer always calls the current + // versions without re-arming (which would reset the debounce) on every render. + const onSaveRef = useRef(onSave) + const getActiveDesignIdRef = useRef(getActiveDesignId) + useEffect(() => { + onSaveRef.current = onSave + getActiveDesignIdRef.current = getActiveDesignId + }) + + useEffect(() => { + if (!enabled || !hasUnsavedChanges || !activeDesignId) return + const designId = activeDesignId + const t = setTimeout(() => { + // Skip if the active design changed while the timer was pending. + if (getActiveDesignIdRef.current() !== designId) return + onSaveRef.current(designId) + }, delaySeconds * 1000) + return () => clearTimeout(t) + // changeSignals is spread so any canvas edit resets the debounce. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [enabled, delaySeconds, hasUnsavedChanges, activeDesignId, ...changeSignals]) +} diff --git a/frontend/src/utils/__tests__/autosaveSettings.test.ts b/frontend/src/utils/__tests__/autosaveSettings.test.ts new file mode 100644 index 0000000..3074c92 --- /dev/null +++ b/frontend/src/utils/__tests__/autosaveSettings.test.ts @@ -0,0 +1,58 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest' +import { + DEFAULT_AUTOSAVE_SETTINGS, + readAutosaveSettings, + writeAutosaveSettings, + subscribeAutosaveSettings, +} from '../autosaveSettings' + +describe('autosaveSettings', () => { + beforeEach(() => { + localStorage.clear() + }) + + it('defaults to disabled with a 5s delay', () => { + expect(DEFAULT_AUTOSAVE_SETTINGS).toEqual({ enabled: false, delay: 5 }) + }) + + it('returns defaults when nothing stored', () => { + expect(readAutosaveSettings()).toEqual(DEFAULT_AUTOSAVE_SETTINGS) + }) + + it('roundtrips through localStorage', () => { + writeAutosaveSettings({ enabled: true, delay: 30 }) + expect(readAutosaveSettings()).toEqual({ enabled: true, delay: 30 }) + }) + + it('falls back to defaults when stored value is corrupted', () => { + localStorage.setItem('homelable.autosave', '{not json') + expect(readAutosaveSettings()).toEqual(DEFAULT_AUTOSAVE_SETTINGS) + }) + + it('fills missing fields from defaults', () => { + localStorage.setItem('homelable.autosave', JSON.stringify({ enabled: true })) + expect(readAutosaveSettings()).toEqual({ enabled: true, delay: DEFAULT_AUTOSAVE_SETTINGS.delay }) + }) + + it('rejects a non-positive delay and falls back to default', () => { + localStorage.setItem('homelable.autosave', JSON.stringify({ enabled: true, delay: 0 })) + expect(readAutosaveSettings().delay).toBe(DEFAULT_AUTOSAVE_SETTINGS.delay) + localStorage.setItem('homelable.autosave', JSON.stringify({ enabled: true, delay: -10 })) + expect(readAutosaveSettings().delay).toBe(DEFAULT_AUTOSAVE_SETTINGS.delay) + }) + + it('rejects a non-numeric delay and falls back to default', () => { + localStorage.setItem('homelable.autosave', JSON.stringify({ enabled: true, delay: 'soon' })) + expect(readAutosaveSettings().delay).toBe(DEFAULT_AUTOSAVE_SETTINGS.delay) + }) + + it('notifies subscribers on write and stops after unsubscribe', () => { + const listener = vi.fn() + const unsubscribe = subscribeAutosaveSettings(listener) + writeAutosaveSettings({ enabled: true, delay: 10 }) + expect(listener).toHaveBeenCalledWith({ enabled: true, delay: 10 }) + unsubscribe() + writeAutosaveSettings({ enabled: false, delay: 3 }) + expect(listener).toHaveBeenCalledTimes(1) + }) +}) From 8d752e0bd816751f12f6d797aa91db892e6fdf7d Mon Sep 17 00:00:00 2001 From: Pouzor Date: Fri, 17 Jul 2026 11:43:42 +0200 Subject: [PATCH 3/5] fix(canvas): gate autosave on canvas provenance, not selection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous guard compared the pinned design against the live activeDesignId (the selection). On a design switch the selection flips synchronously while the new canvas loads asynchronously, so the on-screen nodes briefly still belong to the previous design — arming the timer with the newly-selected id would save those stale nodes under the wrong design. Track the design the canvas was actually loaded as (loadedDesignId provenance) and gate autosave on that instead: pin it when the timer is armed and re-check the live provenance at fire time, skipping the save if a different canvas has since loaded. Also validate the persisted `enabled` flag by type (mirror of `delay`), and add the matching regression tests (provenance switch skip, enabled type validation). ha-relevant: maybe --- frontend/src/App.tsx | 17 ++++++- .../src/hooks/__tests__/useAutosave.test.ts | 33 +++++++------ frontend/src/hooks/useAutosave.ts | 49 +++++++++++-------- .../utils/__tests__/autosaveSettings.test.ts | 5 ++ frontend/src/utils/autosaveSettings.ts | 2 +- 5 files changed, 67 insertions(+), 39 deletions(-) diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index fba646d..7a48958 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -66,6 +66,14 @@ export default function App() { const [autosave, setAutosave] = useState(readAutosaveSettings) useEffect(() => subscribeAutosaveSettings(setAutosave), []) + // Provenance: which design the in-memory canvas was loaded as. Differs from + // activeDesignId (the selection) during a switch, so autosave gates on this to + // avoid writing one design's canvas under another's id. Ref mirror for the + // fire-time guard, which must read the live value without re-arming the timer. + const [loadedDesignId, setLoadedDesignId] = useState(null) + const loadedDesignIdRef = useRef(loadedDesignId) + useEffect(() => { loadedDesignIdRef.current = loadedDesignId }, [loadedDesignId]) + const [themeModalOpen, setThemeModalOpen] = useState(false) const [styleEditorType, setStyleEditorType] = useState(null) const [searchOpen, setSearchOpen] = useState(false) @@ -133,9 +141,9 @@ export default function App() { enabled: autosave.enabled, delaySeconds: autosave.delay, hasUnsavedChanges, - activeDesignId, + designId: loadedDesignId, changeSignals: [nodes, edges], - getActiveDesignId: () => useDesignStore.getState().activeDesignId, + getLiveDesignId: () => loadedDesignIdRef.current, onSave: (designId) => { void handleSaveRef.current(designId, { silent: true }) }, }) @@ -168,6 +176,9 @@ export default function App() { } catch { setFloorMap(null) loadCanvas(demoNodes, demoEdges) + } finally { + // Record provenance so autosave writes back under the design just loaded. + setLoadedDesignId(designId ?? null) } }, [loadCanvas, setTheme, setCustomStyle, setFloorMap]) @@ -186,6 +197,8 @@ export default function App() { setFloorMap(null) loadCanvas(demoNodes, demoEdges) } + // Record provenance so autosave writes back under the design just loaded. + setLoadedDesignId(designId) }, [loadCanvas, setTheme, setCustomStyle, setFloorMap]) const loadDesignsAndCanvas = useCallback(async () => { diff --git a/frontend/src/hooks/__tests__/useAutosave.test.ts b/frontend/src/hooks/__tests__/useAutosave.test.ts index a299013..e114d14 100644 --- a/frontend/src/hooks/__tests__/useAutosave.test.ts +++ b/frontend/src/hooks/__tests__/useAutosave.test.ts @@ -6,21 +6,21 @@ interface Args { enabled?: boolean delaySeconds?: number hasUnsavedChanges?: boolean - activeDesignId?: string | null + designId?: string | null changeSignals?: unknown[] - getActiveDesignId?: () => string | null + getLiveDesignId?: () => string | null onSave?: (designId: string) => void } function args(over: Args = {}) { - const activeDesignId = 'activeDesignId' in over ? (over.activeDesignId ?? null) : 'design-a' + const designId = 'designId' in over ? (over.designId ?? null) : 'design-a' return { enabled: over.enabled ?? true, delaySeconds: over.delaySeconds ?? 5, hasUnsavedChanges: over.hasUnsavedChanges ?? true, - activeDesignId, + designId, changeSignals: over.changeSignals ?? [[], []], - getActiveDesignId: over.getActiveDesignId ?? (() => activeDesignId), + getLiveDesignId: over.getLiveDesignId ?? (() => designId), onSave: over.onSave ?? vi.fn(), } } @@ -36,7 +36,8 @@ describe('useAutosave', () => { vi.advanceTimersByTime(4999) expect(onSave).not.toHaveBeenCalled() vi.advanceTimersByTime(1) - expect(onSave).toHaveBeenCalledExactlyOnceWith('design-a') + expect(onSave).toHaveBeenCalledTimes(1) + expect(onSave).toHaveBeenCalledWith('design-a') }) it('does nothing when disabled', () => { @@ -53,9 +54,9 @@ describe('useAutosave', () => { expect(onSave).not.toHaveBeenCalled() }) - it('does nothing when there is no active design', () => { + it('does nothing when the canvas has no known provenance', () => { const onSave = vi.fn() - renderHook(() => useAutosave(args({ onSave, activeDesignId: null }))) + renderHook(() => useAutosave(args({ onSave, designId: null }))) vi.advanceTimersByTime(60_000) expect(onSave).not.toHaveBeenCalled() }) @@ -70,25 +71,27 @@ describe('useAutosave', () => { vi.advanceTimersByTime(4000) expect(onSave).not.toHaveBeenCalled() vi.advanceTimersByTime(1000) - expect(onSave).toHaveBeenCalledExactlyOnceWith('design-a') + expect(onSave).toHaveBeenCalledTimes(1) }) - it('skips the save if the active design changed while the timer was pending', () => { + it('skips the save if a different canvas loaded while the timer was pending', () => { const onSave = vi.fn() - // Pinned at arm time = 'design-a', but live id has moved on to 'design-b'. + // Pinned provenance at arm time = 'design-a', but a switch loaded 'design-b' + // before the timer fired — the on-screen canvas is no longer design-a's. renderHook(() => - useAutosave(args({ onSave, activeDesignId: 'design-a', getActiveDesignId: () => 'design-b' })), + useAutosave(args({ onSave, designId: 'design-a', getLiveDesignId: () => 'design-b' })), ) vi.advanceTimersByTime(5000) expect(onSave).not.toHaveBeenCalled() }) - it('saves under the pinned design id, not the live one, when they still match', () => { + it('saves under the pinned provenance id when it still matches at fire time', () => { const onSave = vi.fn() renderHook(() => - useAutosave(args({ onSave, activeDesignId: 'design-a', getActiveDesignId: () => 'design-a' })), + useAutosave(args({ onSave, designId: 'design-a', getLiveDesignId: () => 'design-a' })), ) vi.advanceTimersByTime(5000) - expect(onSave).toHaveBeenCalledExactlyOnceWith('design-a') + expect(onSave).toHaveBeenCalledTimes(1) + expect(onSave).toHaveBeenCalledWith('design-a') }) }) diff --git a/frontend/src/hooks/useAutosave.ts b/frontend/src/hooks/useAutosave.ts index e4ac35c..90b43c2 100644 --- a/frontend/src/hooks/useAutosave.ts +++ b/frontend/src/hooks/useAutosave.ts @@ -7,22 +7,28 @@ interface UseAutosaveOptions { delaySeconds: number /** True when the canvas has edits not yet persisted. */ hasUnsavedChanges: boolean - /** The design the in-memory canvas currently belongs to. */ - activeDesignId: string | null + /** + * The design the in-memory canvas actually belongs to — its *provenance*, set + * when a canvas is loaded, NOT the currently-selected design. These differ + * during a design switch: the selection flips synchronously while the new + * canvas loads asynchronously, so for a brief window the on-screen nodes still + * belong to the previous design. Gating on provenance (not selection) is what + * prevents saving one design's canvas under another design's id. + */ + designId: string | null /** * Values that represent canvas edits (e.g. nodes, edges). Any change to one * of these resets the debounce timer, so the save only fires after a quiet - * period. Kept separate from the trigger flags so the caller controls exactly - * what counts as "activity". + * period. Its length MUST stay constant across renders (React requires a + * stable dependency-array size) — pass a fixed-shape array like [nodes, edges]. */ - changeSignals: unknown[] + changeSignals: readonly unknown[] /** - * Reads the *live* active design id at fire time. Used to detect that the user - * switched designs while the timer was pending — if so, the in-memory canvas - * now belongs to a different design and saving it under the pinned id would - * clobber the wrong design, so the save is skipped. + * Reads the *live* canvas provenance at fire time. If it no longer matches the + * id pinned when the timer was armed, a different canvas has since loaded, so + * the save is skipped rather than written under the wrong (now-stale) id. */ - getActiveDesignId: () => string | null + getLiveDesignId: () => string | null /** Persist the canvas under the given design id. */ onSave: (designId: string) => void } @@ -36,30 +42,31 @@ export function useAutosave({ enabled, delaySeconds, hasUnsavedChanges, - activeDesignId, + designId, changeSignals, - getActiveDesignId, + getLiveDesignId, onSave, }: UseAutosaveOptions): void { // Keep the latest callbacks in refs so the timer always calls the current // versions without re-arming (which would reset the debounce) on every render. const onSaveRef = useRef(onSave) - const getActiveDesignIdRef = useRef(getActiveDesignId) + const getLiveDesignIdRef = useRef(getLiveDesignId) useEffect(() => { onSaveRef.current = onSave - getActiveDesignIdRef.current = getActiveDesignId + getLiveDesignIdRef.current = getLiveDesignId }) useEffect(() => { - if (!enabled || !hasUnsavedChanges || !activeDesignId) return - const designId = activeDesignId + if (!enabled || !hasUnsavedChanges || !designId) return + const pinnedDesignId = designId const t = setTimeout(() => { - // Skip if the active design changed while the timer was pending. - if (getActiveDesignIdRef.current() !== designId) return - onSaveRef.current(designId) + // Skip if a different canvas has loaded while the timer was pending. + if (getLiveDesignIdRef.current() !== pinnedDesignId) return + onSaveRef.current(pinnedDesignId) }, delaySeconds * 1000) return () => clearTimeout(t) - // changeSignals is spread so any canvas edit resets the debounce. + // changeSignals is spread so any canvas edit resets the debounce; its length + // must stay constant (documented on the option). // eslint-disable-next-line react-hooks/exhaustive-deps - }, [enabled, delaySeconds, hasUnsavedChanges, activeDesignId, ...changeSignals]) + }, [enabled, delaySeconds, hasUnsavedChanges, designId, ...changeSignals]) } diff --git a/frontend/src/utils/__tests__/autosaveSettings.test.ts b/frontend/src/utils/__tests__/autosaveSettings.test.ts index 3074c92..be31087 100644 --- a/frontend/src/utils/__tests__/autosaveSettings.test.ts +++ b/frontend/src/utils/__tests__/autosaveSettings.test.ts @@ -46,6 +46,11 @@ describe('autosaveSettings', () => { expect(readAutosaveSettings().delay).toBe(DEFAULT_AUTOSAVE_SETTINGS.delay) }) + it('rejects a non-boolean enabled and falls back to default', () => { + localStorage.setItem('homelable.autosave', JSON.stringify({ enabled: 'yes', delay: 10 })) + expect(readAutosaveSettings().enabled).toBe(DEFAULT_AUTOSAVE_SETTINGS.enabled) + }) + it('notifies subscribers on write and stops after unsubscribe', () => { const listener = vi.fn() const unsubscribe = subscribeAutosaveSettings(listener) diff --git a/frontend/src/utils/autosaveSettings.ts b/frontend/src/utils/autosaveSettings.ts index 3b54941..266e0b0 100644 --- a/frontend/src/utils/autosaveSettings.ts +++ b/frontend/src/utils/autosaveSettings.ts @@ -19,7 +19,7 @@ export function readAutosaveSettings(): AutosaveSettings { if (!raw) return DEFAULT_AUTOSAVE_SETTINGS const parsed = JSON.parse(raw) as Partial return { - enabled: parsed.enabled ?? DEFAULT_AUTOSAVE_SETTINGS.enabled, + enabled: typeof parsed.enabled === 'boolean' ? parsed.enabled : DEFAULT_AUTOSAVE_SETTINGS.enabled, delay: typeof parsed.delay === 'number' && parsed.delay > 0 ? parsed.delay : DEFAULT_AUTOSAVE_SETTINGS.delay, } } catch { From 71ab3bf3917bc8713bb5e97933b04a8f201b6ba2 Mon Sep 17 00:00:00 2001 From: Pouzor Date: Fri, 17 Jul 2026 13:02:14 +0200 Subject: [PATCH 4/5] fix(canvas): stop non-edits from marking the canvas unsaved MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Autosave saves whenever hasUnsavedChanges is set, so any source that dirties the canvas without a user edit turns into a spurious silent save. Two such sources: 1. Live status updates. useStatusPolling routed backend status pushes through updateNode, which always sets hasUnsavedChanges. With the 60s status cycle, an idle opted-in tab saved itself every cycle and could clobber edits made elsewhere with its stale in-memory canvas. Add setNodeStatus, a live-overlay action that updates a node's status/response_time/last_seen without dirtying (mirrors setServiceStatuses), and route polling through it. 2. Initial dimensions measure. onNodesChange treated React Flow's first-measure `dimensions` change as an edit, marking a freshly loaded canvas dirty before any interaction — autosave then saved on every load/switch. Only count a `dimensions` change as an edit when it is a real resize (resizing === true). Add regression tests: setNodeStatus updates status without dirtying; onNodesChange ignores initial-measure dimensions and select-only changes but still dirties on a user resize. ha-relevant: maybe --- .../hooks/__tests__/useStatusPolling.test.ts | 22 ++++----- frontend/src/hooks/useStatusPolling.ts | 8 ++-- .../__tests__/canvasStore/nodes.test.ts | 45 +++++++++++++++++++ frontend/src/stores/canvasStore.ts | 38 +++++++++++++++- 4 files changed, 98 insertions(+), 15 deletions(-) diff --git a/frontend/src/hooks/__tests__/useStatusPolling.test.ts b/frontend/src/hooks/__tests__/useStatusPolling.test.ts index e8df523..1a89280 100644 --- a/frontend/src/hooks/__tests__/useStatusPolling.test.ts +++ b/frontend/src/hooks/__tests__/useStatusPolling.test.ts @@ -7,7 +7,7 @@ import { useAuthStore } from '@/stores/authStore' vi.mock('@/stores/canvasStore') vi.mock('@/stores/authStore') -const mockUpdateNode = vi.fn() +const mockSetNodeStatus = vi.fn() const mockNotifyScanDeviceFound = vi.fn() const mockSetServiceStatuses = vi.fn() @@ -32,7 +32,7 @@ describe('useStatusPolling', () => { vi.stubGlobal('WebSocket', MockWebSocket) vi.mocked(useCanvasStore).mockReturnValue({ - updateNode: mockUpdateNode, + setNodeStatus: mockSetNodeStatus, notifyScanDeviceFound: mockNotifyScanDeviceFound, setServiceStatuses: mockSetServiceStatuses, } as ReturnType) @@ -50,7 +50,7 @@ describe('useStatusPolling', () => { afterEach(() => { vi.restoreAllMocks() - mockUpdateNode.mockClear() + mockSetNodeStatus.mockClear() mockNotifyScanDeviceFound.mockClear() mockSetServiceStatuses.mockClear() }) @@ -95,7 +95,7 @@ describe('useStatusPolling', () => { expect(ws.send).toHaveBeenCalledWith(JSON.stringify({ token: 'test-token' })) }) - it('calls updateNode with correct data on status message', () => { + it('calls setNodeStatus with correct data on status message', () => { renderHook(() => useStatusPolling()) const ws = MockWebSocket.instances[0] ws.onmessage?.({ @@ -106,7 +106,7 @@ describe('useStatusPolling', () => { response_time_ms: 42, }), }) - expect(mockUpdateNode).toHaveBeenCalledWith('node-1', { + expect(mockSetNodeStatus).toHaveBeenCalledWith('node-1', { status: 'online', response_time_ms: 42, last_seen: '2024-01-01T12:00:00Z', @@ -123,7 +123,7 @@ describe('useStatusPolling', () => { checked_at: '2024-01-01T12:00:00Z', }), }) - expect(mockUpdateNode).toHaveBeenCalledWith('node-1', { + expect(mockSetNodeStatus).toHaveBeenCalledWith('node-1', { status: 'offline', response_time_ms: undefined, last_seen: undefined, @@ -136,7 +136,7 @@ describe('useStatusPolling', () => { ws.onmessage?.({ data: JSON.stringify({ node_id: 'node-1', status: 'online', response_time_ms: null }), }) - expect(mockUpdateNode).toHaveBeenCalledWith( + expect(mockSetNodeStatus).toHaveBeenCalledWith( 'node-1', expect.objectContaining({ response_time_ms: undefined }), ) @@ -147,7 +147,7 @@ describe('useStatusPolling', () => { const ws = MockWebSocket.instances[0] ws.onmessage?.({ data: JSON.stringify({ type: 'scan_device_found' }) }) expect(mockNotifyScanDeviceFound).toHaveBeenCalledOnce() - expect(mockUpdateNode).not.toHaveBeenCalled() + expect(mockSetNodeStatus).not.toHaveBeenCalled() }) it('routes service_status messages to setServiceStatuses', () => { @@ -158,21 +158,21 @@ describe('useStatusPolling', () => { data: JSON.stringify({ type: 'service_status', node_id: 'node-9', services }), }) expect(mockSetServiceStatuses).toHaveBeenCalledWith('node-9', services) - expect(mockUpdateNode).not.toHaveBeenCalled() + expect(mockSetNodeStatus).not.toHaveBeenCalled() }) it('ignores malformed JSON without throwing', () => { renderHook(() => useStatusPolling()) const ws = MockWebSocket.instances[0] expect(() => ws.onmessage?.({ data: 'not-valid-json{{' })).not.toThrow() - expect(mockUpdateNode).not.toHaveBeenCalled() + expect(mockSetNodeStatus).not.toHaveBeenCalled() }) it('ignores messages with no node_id or status', () => { renderHook(() => useStatusPolling()) const ws = MockWebSocket.instances[0] ws.onmessage?.({ data: JSON.stringify({ some: 'unknown-field' }) }) - expect(mockUpdateNode).not.toHaveBeenCalled() + expect(mockSetNodeStatus).not.toHaveBeenCalled() }) it('closes WebSocket on unmount', () => { diff --git a/frontend/src/hooks/useStatusPolling.ts b/frontend/src/hooks/useStatusPolling.ts index defd2bc..b21b340 100644 --- a/frontend/src/hooks/useStatusPolling.ts +++ b/frontend/src/hooks/useStatusPolling.ts @@ -24,7 +24,7 @@ const STANDALONE = import.meta.env.VITE_STANDALONE === 'true' export function useStatusPolling() { const wsRef = useRef(null) - const { updateNode, notifyScanDeviceFound, setServiceStatuses } = useCanvasStore() + const { setNodeStatus, notifyScanDeviceFound, setServiceStatuses } = useCanvasStore() const { isAuthenticated, token } = useAuthStore() useEffect(() => { @@ -50,7 +50,9 @@ export function useStatusPolling() { } else if (msg.type === 'service_status' && msg.node_id && msg.services) { setServiceStatuses(msg.node_id, msg.services) } else if (msg.node_id && msg.status) { - updateNode(msg.node_id, { + // Live status is monitoring data, not a user edit — must not dirty the + // canvas (otherwise autosave rewrites an untouched canvas every cycle). + setNodeStatus(msg.node_id, { status: msg.status, response_time_ms: msg.response_time_ms ?? undefined, last_seen: msg.status === 'online' ? msg.checked_at : undefined, @@ -69,5 +71,5 @@ export function useStatusPolling() { ws.close() wsRef.current = null } - }, [isAuthenticated, token, updateNode, notifyScanDeviceFound, setServiceStatuses]) + }, [isAuthenticated, token, setNodeStatus, notifyScanDeviceFound, setServiceStatuses]) } diff --git a/frontend/src/stores/__tests__/canvasStore/nodes.test.ts b/frontend/src/stores/__tests__/canvasStore/nodes.test.ts index 50d99c7..7bc3412 100644 --- a/frontend/src/stores/__tests__/canvasStore/nodes.test.ts +++ b/frontend/src/stores/__tests__/canvasStore/nodes.test.ts @@ -52,6 +52,51 @@ describe('canvasStore — nodes', () => { expect(useCanvasStore.getState().hasUnsavedChanges).toBe(false) }) + it('setNodeStatus updates node status fields', () => { + useCanvasStore.getState().addNode(makeNode('n1')) + useCanvasStore.getState().setNodeStatus('n1', { + status: 'online', + response_time_ms: 42, + last_seen: '2024-01-01T12:00:00Z', + }) + const node = useCanvasStore.getState().nodes.find((n) => n.id === 'n1') + expect(node?.data.status).toBe('online') + expect(node?.data.response_time_ms).toBe(42) + expect(node?.data.last_seen).toBe('2024-01-01T12:00:00Z') + }) + + it('setNodeStatus does NOT mark the canvas unsaved (live status is not a user edit)', () => { + useCanvasStore.getState().addNode(makeNode('n1')) + useCanvasStore.setState({ hasUnsavedChanges: false }) + useCanvasStore.getState().setNodeStatus('n1', { status: 'offline' }) + expect(useCanvasStore.getState().hasUnsavedChanges).toBe(false) + }) + + it('onNodesChange does NOT dirty the canvas on an initial dimensions measure', () => { + useCanvasStore.getState().addNode(makeNode({ id: 'n1' })) + useCanvasStore.setState({ hasUnsavedChanges: false }) + useCanvasStore.getState().onNodesChange([ + { id: 'n1', type: 'dimensions', dimensions: { width: 120, height: 40 } }, + ]) + expect(useCanvasStore.getState().hasUnsavedChanges).toBe(false) + }) + + it('onNodesChange DOES dirty the canvas on a user resize (resizing: true)', () => { + useCanvasStore.getState().addNode(makeNode({ id: 'n1' })) + useCanvasStore.setState({ hasUnsavedChanges: false }) + useCanvasStore.getState().onNodesChange([ + { id: 'n1', type: 'dimensions', dimensions: { width: 200, height: 80 }, resizing: true }, + ]) + expect(useCanvasStore.getState().hasUnsavedChanges).toBe(true) + }) + + it('onNodesChange does NOT dirty the canvas on a select-only change', () => { + useCanvasStore.getState().addNode(makeNode({ id: 'n1' })) + useCanvasStore.setState({ hasUnsavedChanges: false }) + useCanvasStore.getState().onNodesChange([{ id: 'n1', type: 'select', selected: true }]) + expect(useCanvasStore.getState().hasUnsavedChanges).toBe(false) + }) + it('setEditingTextId sets and clears editing text id', () => { const { setEditingTextId } = useCanvasStore.getState() setEditingTextId('t1') diff --git a/frontend/src/stores/canvasStore.ts b/frontend/src/stores/canvasStore.ts index 419c3b1..c8c8c91 100644 --- a/frontend/src/stores/canvasStore.ts +++ b/frontend/src/stores/canvasStore.ts @@ -22,6 +22,22 @@ type Clipboard = { nodes: Node[]; edges: Edge[] } /** Resolve a node's effective parent id from either the RF field or domain data. */ const parentIdOf = (n: Node): string | undefined => n.parentId ?? n.data.parent_id ?? undefined +/** + * Whether a node change represents a real user edit that should dirty the canvas. + * Excludes: + * - 'select': selecting a node changes nothing persisted. + * - 'dimensions' without resizing: React Flow emits these when it first measures + * a node's size after mount/load. Counting them as edits marks a freshly + * loaded canvas dirty before the user touches anything (autosave would then + * save on every load). A user-driven resize sets `resizing === true` and still + * dirties. + */ +function isUserNodeEdit(c: NodeChange>): boolean { + if (c.type === 'select') return false + if (c.type === 'dimensions' && c.resizing !== true) return false + return true +} + /** * Keep manually-routed edge waypoints attached to their nodes on drag (#279). * @@ -127,6 +143,14 @@ interface CanvasState { setSelectedNode: (id: string | null) => void addNode: (node: Node) => void updateNode: (id: string, data: Partial) => void + /** + * Apply a live status update to a node WITHOUT marking the canvas unsaved. + * Status (online/offline, response time, last seen) is transient monitoring + * data pushed by the backend, not a user edit — dirtying the canvas here would + * make autosave rewrite an untouched canvas on every status cycle and could + * clobber edits made elsewhere. Mirrors setServiceStatuses' live-overlay rule. + */ + setNodeStatus: (id: string, status: Pick) => void deleteNode: (id: string) => void updateEdge: (id: string, data: Partial) => void reconnectEdge: (id: string, connection: Connection) => void @@ -327,7 +351,7 @@ export const useCanvasStore = create((set) => ({ nodes, edges, selectedNodeIds, - hasUnsavedChanges: state.hasUnsavedChanges || changes.some((c) => c.type !== 'select'), + hasUnsavedChanges: state.hasUnsavedChanges || changes.some(isUserNodeEdit), } }), @@ -468,6 +492,18 @@ export const useCanvasStore = create((set) => ({ return { nodes, edges, hasUnsavedChanges: true } }), + setNodeStatus: (id, status) => + set((state) => { + let changed = false + const nodes = state.nodes.map((n) => { + if (n.id !== id) return n + changed = true + return { ...n, data: { ...n.data, ...status } } + }) + // No hasUnsavedChanges: live status is monitoring data, not a user edit. + return changed ? { nodes } : {} + }), + deleteNode: (id) => set((state) => { const idsToRemove = new Set() From d63c9e64516cc72f5987ed2e8846b463a7771562 Mon Sep 17 00:00:00 2001 From: Pouzor Date: Fri, 17 Jul 2026 20:29:48 +0200 Subject: [PATCH 5/5] fix(canvas): drive autosave debounce off an edit counter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit useAutosave reset its debounce whenever the nodes/edges array reference changed. Live status polling (setNodeStatus) returns a fresh nodes array on every message without dirtying the canvas, so during active monitoring the status churn kept re-arming the timer faster than the delay and the silent save could be starved indefinitely — the user thinks autosave ran, but it never fired. Select-only changes reset it the same way. Add a monotonic editSeq counter that bumps only on real user edits. The store's set() is wrapped to increment editSeq whenever an update flips hasUnsavedChanges to true, so it stays centralised instead of touching every mutating action. onNodesChange/onEdgesChange now set the dirty flag only when a genuine edit occurred (not on select/measure), so those no longer bump the counter. App keys the autosave changeSignals on [editSeq] instead of [nodes, edges]. Add a regression test asserting editSeq bumps on add/update but not on setNodeStatus, select, or initial dimensions measure. ha-relevant: maybe --- frontend/src/App.tsx | 7 ++- .../__tests__/canvasStore/nodes.test.ts | 29 ++++++++++ frontend/src/stores/canvasStore.ts | 53 ++++++++++++++++--- 3 files changed, 80 insertions(+), 9 deletions(-) diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 2e9713e..d262891 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -55,7 +55,7 @@ import { buildProxmoxClusterEdges } from '@/components/proxmox/clusterEdges' const STANDALONE = import.meta.env.VITE_STANDALONE === 'true' export default function App() { - const { loadCanvas, applyLayout, markSaved, markUnsaved, hasUnsavedChanges, selectedNodeId, selectedNodeIds, addNode, updateNode, deleteNode, onConnect, updateEdge, deleteEdge, setProxmoxContainerMode, setNodeZIndex, editingGroupRectId, setEditingGroupRectId, editingTextId, setEditingTextId, nodes, edges, snapshotHistory, undo, redo, addToGroup, addToContainer, floorMap, setFloorMap } = useCanvasStore() + const { loadCanvas, applyLayout, markSaved, markUnsaved, hasUnsavedChanges, editSeq, selectedNodeId, selectedNodeIds, addNode, updateNode, deleteNode, onConnect, updateEdge, deleteEdge, setProxmoxContainerMode, setNodeZIndex, editingGroupRectId, setEditingGroupRectId, editingTextId, setEditingTextId, nodes, edges, snapshotHistory, undo, redo, addToGroup, addToContainer, floorMap, setFloorMap } = useCanvasStore() const canvasRef = useRef(null) const { isAuthenticated } = useAuthStore() const { activeTheme, setTheme, customStyle, setCustomStyle } = useThemeStore() @@ -142,7 +142,10 @@ export default function App() { delaySeconds: autosave.delay, hasUnsavedChanges, designId: loadedDesignId, - changeSignals: [nodes, edges], + // Debounce resets on each real user edit (editSeq), not on raw nodes/edges + // identity — live status polling churns those arrays without a user edit and + // would otherwise keep re-arming (and starving) the timer during monitoring. + changeSignals: [editSeq], getLiveDesignId: () => loadedDesignIdRef.current, onSave: (designId) => { void handleSaveRef.current(designId, { silent: true }) }, }) diff --git a/frontend/src/stores/__tests__/canvasStore/nodes.test.ts b/frontend/src/stores/__tests__/canvasStore/nodes.test.ts index 7bc3412..e2239f6 100644 --- a/frontend/src/stores/__tests__/canvasStore/nodes.test.ts +++ b/frontend/src/stores/__tests__/canvasStore/nodes.test.ts @@ -9,6 +9,7 @@ function resetStore() { nodes: [], edges: [], hasUnsavedChanges: false, + editSeq: 0, selectedNodeId: null, selectedNodeIds: [], editingGroupRectId: null, @@ -97,6 +98,34 @@ describe('canvasStore — nodes', () => { expect(useCanvasStore.getState().hasUnsavedChanges).toBe(false) }) + it('editSeq bumps on a real user edit but not on status/select/measure churn', () => { + const seq = () => useCanvasStore.getState().editSeq + + // Real edit bumps. + const before = seq() + useCanvasStore.getState().addNode(makeNode('n1')) + expect(seq()).toBe(before + 1) + + // Live status update: no bump (not a user edit). + const afterAdd = seq() + useCanvasStore.getState().setNodeStatus('n1', { status: 'online' }) + expect(seq()).toBe(afterAdd) + + // Select-only change: no bump. + useCanvasStore.getState().onNodesChange([{ id: 'n1', type: 'select', selected: true }]) + expect(seq()).toBe(afterAdd) + + // Initial dimensions measure: no bump. + useCanvasStore.getState().onNodesChange([ + { id: 'n1', type: 'dimensions', dimensions: { width: 120, height: 40 } }, + ]) + expect(seq()).toBe(afterAdd) + + // Another real edit bumps again. + useCanvasStore.getState().updateNode('n1', { label: 'renamed' }) + expect(seq()).toBe(afterAdd + 1) + }) + it('setEditingTextId sets and clears editing text id', () => { const { setEditingTextId } = useCanvasStore.getState() setEditingTextId('t1') diff --git a/frontend/src/stores/canvasStore.ts b/frontend/src/stores/canvasStore.ts index c8c8c91..519d54d 100644 --- a/frontend/src/stores/canvasStore.ts +++ b/frontend/src/stores/canvasStore.ts @@ -109,6 +109,14 @@ interface CanvasState { nodes: Node[] edges: Edge[] hasUnsavedChanges: boolean + /** + * Monotonic counter incremented on every real user edit (auto-bumped whenever + * an action sets hasUnsavedChanges to true). Consumers that need to react to + * *edits specifically* — e.g. the autosave debounce — key off this instead of + * the nodes/edges array identity, which also churns on live status updates and + * selection changes that must NOT reset the debounce. + */ + editSeq: number selectedNodeId: string | null selectedNodeIds: string[] scanEventTs: number @@ -187,10 +195,33 @@ interface CanvasState { applyAllCustomStyles: (def: CustomStyleDef) => void } -export const useCanvasStore = create((set) => ({ +export const useCanvasStore = create((rawSet) => { + // Wrap set so any update that flips hasUnsavedChanges to true also bumps + // editSeq. This centralises the "an edit happened" signal instead of touching + // every one of the ~two dozen mutating actions. Actions that update state + // without dirtying (setNodeStatus, markSaved, loadCanvas, selection) omit + // hasUnsavedChanges or set it false, so they never bump the counter. + const set: typeof rawSet = ((partial, replace) => { + rawSet((state) => { + const next = typeof partial === 'function' + ? (partial as (s: CanvasState) => Partial)(state) + : partial + if ( + next && + typeof next === 'object' && + (next as Partial).hasUnsavedChanges === true && + !('editSeq' in next) + ) { + return { ...next, editSeq: state.editSeq + 1 } + } + return next + }, replace as false | undefined) + }) as typeof rawSet + return { nodes: [], edges: [], hasUnsavedChanges: false, + editSeq: 0, selectedNodeId: null, selectedNodeIds: [], editingGroupRectId: null, @@ -347,19 +378,26 @@ export const useCanvasStore = create((set) => ({ // they don't follow a moved node on their own. Translate them by the same // delta the node moved so a clean routing stays clean after a drag (#279). const edges = translateWaypointsForMovedNodes(changes, state.nodes, nodes, state.edges) + // Only set hasUnsavedChanges when a real edit occurred, so the set() wrapper + // bumps editSeq only then. Selection- or measure-only changes leave the flag + // untouched (carried over) and must not reset the autosave debounce. + const edited = changes.some(isUserNodeEdit) return { nodes, edges, selectedNodeIds, - hasUnsavedChanges: state.hasUnsavedChanges || changes.some(isUserNodeEdit), + ...(edited ? { hasUnsavedChanges: true } : {}), } }), onEdgesChange: (changes) => - set((state) => ({ - edges: applyEdgeChanges(changes, state.edges), - hasUnsavedChanges: state.hasUnsavedChanges || changes.some((c) => c.type !== 'select'), - })), + set((state) => { + const edited = changes.some((c) => c.type !== 'select') + return { + edges: applyEdgeChanges(changes, state.edges), + ...(edited ? { hasUnsavedChanges: true } : {}), + } + }), onConnect: (connection) => set((state) => { @@ -1013,4 +1051,5 @@ export const useCanvasStore = create((set) => ({ }) return { nodes, edges, hasUnsavedChanges: true } }), -})) + } +})