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) + }) +})