From 0c67bad5c5e433821cd9f5cd40ef9d0d0cd1f6ee Mon Sep 17 00:00:00 2001 From: Stan Date: Sat, 14 Feb 2026 08:58:22 +0000 Subject: [PATCH] Revert "fix: replace dual-store undo/redo with unified history store" This reverts commit c3f5df23ca451ce5d00759946eec7343d67a4332. --- packages/fossflow-lib/src/Isoflow.tsx | 12 +- .../__tests__/history-integration.test.tsx | 492 ------------- .../ContextMenu/ContextMenuManager.tsx | 18 +- .../src/components/Cursor/Cursor.tsx | 2 - .../src/components/DebugUtils/DebugUtils.tsx | 4 +- .../DebugUtils/__tests__/DebugUtils.test.tsx | 5 +- .../__tests__/SizeIndicator.test.tsx | 5 +- .../components/IsoTileArea/IsoTileArea.tsx | 2 - .../ConnectorControls/ConnectorControls.tsx | 4 - .../NodeSettings/NodeSettings.tsx | 12 +- .../TextBoxControls/TextBoxControls.tsx | 4 - .../src/components/MainMenu/MainMenu.tsx | 11 +- .../RichTextEditor/RichTextEditor.tsx | 6 - .../src/components/SceneLayer/SceneLayer.tsx | 2 - .../ConnectorLabels/ConnectorLabel.tsx | 2 - .../ConnectorLabels/ConnectorLabels.tsx | 36 +- .../SceneLayers/Connectors/Connector.tsx | 2 - .../SceneLayers/Connectors/Connectors.tsx | 12 +- .../SceneLayers/Nodes/Node/Node.tsx | 2 - .../components/SceneLayers/Nodes/Nodes.tsx | 12 +- .../SceneLayers/Rectangles/Rectangle.tsx | 2 - .../SceneLayers/Rectangles/Rectangles.tsx | 12 +- .../SceneLayers/TextBoxes/TextBox.tsx | 2 - .../SceneLayers/TextBoxes/TextBoxes.tsx | 12 +- .../src/hooks/__tests__/useHistory.test.tsx | 690 +++++++----------- packages/fossflow-lib/src/hooks/useHistory.ts | 173 +++-- .../src/hooks/useInitialDataManager.ts | 20 +- packages/fossflow-lib/src/hooks/useScene.ts | 109 +-- packages/fossflow-lib/src/hooks/useView.ts | 6 +- .../src/interaction/modes/Connector.ts | 16 +- .../src/interaction/modes/DragItems.ts | 56 +- .../src/interaction/modes/FreehandLasso.ts | 10 +- .../src/interaction/modes/Lasso.ts | 16 +- .../modes/Rectangle/DrawRectangle.ts | 6 +- .../modes/Rectangle/TransformRectangle.ts | 7 +- .../src/interaction/modes/TextBox.ts | 5 +- .../src/interaction/useInteractionManager.ts | 18 +- .../stores/__tests__/historyStore.test.tsx | 361 --------- .../fossflow-lib/src/stores/historyStore.tsx | 211 ------ .../fossflow-lib/src/stores/modelStore.tsx | 147 +++- .../fossflow-lib/src/stores/sceneStore.tsx | 142 +++- .../fossflow-lib/src/types/interactions.ts | 5 - packages/fossflow-lib/src/types/model.ts | 16 +- packages/fossflow-lib/src/types/scene.ts | 12 +- packages/fossflow-lib/src/types/ui.ts | 2 +- 45 files changed, 872 insertions(+), 1829 deletions(-) delete mode 100644 packages/fossflow-lib/src/__tests__/history-integration.test.tsx delete mode 100644 packages/fossflow-lib/src/stores/__tests__/historyStore.test.tsx delete mode 100644 packages/fossflow-lib/src/stores/historyStore.tsx diff --git a/packages/fossflow-lib/src/Isoflow.tsx b/packages/fossflow-lib/src/Isoflow.tsx index d8dbedd..2bfc9c3 100644 --- a/packages/fossflow-lib/src/Isoflow.tsx +++ b/packages/fossflow-lib/src/Isoflow.tsx @@ -1,13 +1,11 @@ import React, { useEffect } from 'react'; import { ThemeProvider } from '@mui/material/styles'; import { Box } from '@mui/material'; -import { shallow } from 'zustand/shallow'; import { theme } from 'src/styles/theme'; import { IsoflowProps } from 'src/types'; import { setWindowCursor, modelFromModelStore } from 'src/utils'; import { useModelStore, ModelProvider } from 'src/stores/modelStore'; import { SceneProvider } from 'src/stores/sceneStore'; -import { HistoryProvider } from 'src/stores/historyStore'; import { LocaleProvider } from 'src/stores/localeStore'; import { GlobalStyles } from 'src/styles/GlobalStyles'; import { Renderer } from 'src/components/Renderer/Renderer'; @@ -35,7 +33,7 @@ const App = ({ const initialDataManager = useInitialDataManager(); const model = useModelStore((state) => { return modelFromModelStore(state); - }, shallow); + }); const { load } = initialDataManager; @@ -101,11 +99,9 @@ export const Isoflow = (props: IsoflowProps) => { - - - - - + + + diff --git a/packages/fossflow-lib/src/__tests__/history-integration.test.tsx b/packages/fossflow-lib/src/__tests__/history-integration.test.tsx deleted file mode 100644 index db325af..0000000 --- a/packages/fossflow-lib/src/__tests__/history-integration.test.tsx +++ /dev/null @@ -1,492 +0,0 @@ -import React from 'react'; -import { renderHook, act } from '@testing-library/react'; -import { ModelProvider, useModelStoreApi } from 'src/stores/modelStore'; -import { SceneProvider, useSceneStoreApi } from 'src/stores/sceneStore'; -import { HistoryProvider, useHistoryStoreApi } from 'src/stores/historyStore'; -import { UiStateProvider } from 'src/stores/uiStateStore'; -import { useHistory } from 'src/hooks/useHistory'; - -/** - * Integration tests using real providers (no mocks). - * Verifies the unified history system works end-to-end - * across model, scene, and history stores. - */ - -const AllProviders = ({ children }: { children: React.ReactNode }) => ( - - - - {children} - - - -); - -/** Hook that exposes history + imperative store APIs for testing */ -function useTestHarness() { - const history = useHistory(); - const modelApi = useModelStoreApi(); - const sceneApi = useSceneStoreApi(); - const historyApi = useHistoryStoreApi(); - return { history, modelApi, sceneApi, historyApi }; -} - -describe('history integration (real providers)', () => { - const renderTestHook = () => - renderHook(() => useTestHarness(), { wrapper: AllProviders }); - - describe('saveSnapshot + undo + redo round-trip', () => { - it('undo restores model state atomically', () => { - const { result } = renderTestHook(); - - // Save initial state as a snapshot - act(() => { - result.current.history.saveSnapshot(); - }); - - // Mutate model - act(() => { - result.current.modelApi.getState().actions.set({ title: 'Changed' }); - }); - - // Undo should restore original title - let success = false; - act(() => { - success = result.current.history.undo(); - }); - - expect(success).toBe(true); - expect(result.current.modelApi.getState().title).toBe('Untitled'); - }); - - it('undo restores scene state atomically', () => { - const { result } = renderTestHook(); - - act(() => { - result.current.history.saveSnapshot(); - }); - - // Mutate scene - act(() => { - result.current.sceneApi.getState().actions.set({ - connectors: { - 'conn-1': { - path: { - tiles: [{ x: 0, y: 0 }], - rectangle: { x: 0, y: 0, width: 1, height: 1 } - } - } - } - }); - }); - - let success = false; - act(() => { - success = result.current.history.undo(); - }); - - expect(success).toBe(true); - expect(result.current.sceneApi.getState().connectors).toEqual({}); - }); - - it('redo re-applies the undone state', () => { - const { result } = renderTestHook(); - - act(() => { - result.current.history.saveSnapshot(); - }); - - act(() => { - result.current.modelApi.getState().actions.set({ title: 'V2' }); - }); - - act(() => { - result.current.history.undo(); - }); - - expect(result.current.modelApi.getState().title).toBe('Untitled'); - - let success = false; - act(() => { - success = result.current.history.redo(); - }); - - expect(success).toBe(true); - // Redo restores the state that was current when we undid (the V2 state) - expect(result.current.modelApi.getState().title).toBe('V2'); - }); - - it('undo returns false when history is empty', () => { - const { result } = renderTestHook(); - - let success = true; - act(() => { - success = result.current.history.undo(); - }); - - expect(success).toBe(false); - }); - - it('redo returns false when future is empty', () => { - const { result } = renderTestHook(); - - let success = true; - act(() => { - success = result.current.history.redo(); - }); - - expect(success).toBe(false); - }); - }); - - describe('gesture lifecycle produces single history entry', () => { - it('beginGesture + multiple mutations + endGesture = single undo step', () => { - const { result } = renderTestHook(); - - act(() => { - result.current.history.beginGesture(); - }); - - // Multiple mutations during gesture - act(() => { - result.current.modelApi.getState().actions.set({ title: 'Step1' }); - }); - act(() => { - result.current.modelApi.getState().actions.set({ title: 'Step2' }); - }); - act(() => { - result.current.modelApi.getState().actions.set({ title: 'Step3' }); - }); - - act(() => { - result.current.history.endGesture(); - }); - - // Only 1 history entry (from beginGesture) - const historyState = result.current.historyApi.getState(); - expect(historyState.past).toHaveLength(1); - - // Undo restores pre-gesture state in one step - act(() => { - result.current.history.undo(); - }); - - expect(result.current.modelApi.getState().title).toBe('Untitled'); - }); - - it('saveSnapshot is blocked during gesture', () => { - const { result } = renderTestHook(); - - act(() => { - result.current.history.beginGesture(); - }); - - // These should be no-ops - act(() => { - result.current.history.saveSnapshot(); - result.current.history.saveSnapshot(); - result.current.history.saveSnapshot(); - }); - - act(() => { - result.current.history.endGesture(); - }); - - // Only the beginGesture entry - expect(result.current.historyApi.getState().past).toHaveLength(1); - }); - - it('beginGesture is idempotent when already in gesture', () => { - const { result } = renderTestHook(); - - act(() => { - result.current.history.beginGesture(); - }); - - act(() => { - result.current.modelApi.getState().actions.set({ title: 'Changed' }); - }); - - // Second beginGesture should be ignored - act(() => { - result.current.history.beginGesture(); - }); - - act(() => { - result.current.history.endGesture(); - }); - - // Only 1 entry - expect(result.current.historyApi.getState().past).toHaveLength(1); - // The saved state is from the FIRST beginGesture (pre-mutation) - expect(result.current.historyApi.getState().past[0].model.title).toBe( - 'Untitled' - ); - }); - }); - - describe('cancelGesture restores pre-gesture state', () => { - it('restores model state on cancel', () => { - const { result } = renderTestHook(); - - act(() => { - result.current.history.beginGesture(); - }); - - act(() => { - result.current.modelApi.getState().actions.set({ title: 'During gesture' }); - }); - - expect(result.current.modelApi.getState().title).toBe('During gesture'); - - act(() => { - result.current.history.cancelGesture(); - }); - - expect(result.current.modelApi.getState().title).toBe('Untitled'); - expect(result.current.historyApi.getState().gestureInProgress).toBe(false); - expect(result.current.historyApi.getState().past).toHaveLength(0); - }); - - it('restores scene state on cancel', () => { - const { result } = renderTestHook(); - - act(() => { - result.current.history.beginGesture(); - }); - - act(() => { - result.current.sceneApi.getState().actions.set({ - connectors: { - 'temp-conn': { - path: { - tiles: [{ x: 0, y: 0 }], - rectangle: { x: 0, y: 0, width: 1, height: 1 } - } - } - } - }); - }); - - act(() => { - result.current.history.cancelGesture(); - }); - - expect(result.current.sceneApi.getState().connectors).toEqual({}); - }); - - it('cancelGesture is a no-op when no gesture in progress', () => { - const { result } = renderTestHook(); - - // Should not throw - act(() => { - result.current.history.cancelGesture(); - }); - - expect(result.current.historyApi.getState().past).toHaveLength(0); - }); - }); - - describe('undo + redo restores model+scene atomically', () => { - it('combined model+scene mutation undoes as one unit', () => { - const { result } = renderTestHook(); - - // Save pre-change snapshot - act(() => { - result.current.history.saveSnapshot(); - }); - - // Mutate both stores - act(() => { - result.current.modelApi.getState().actions.set({ title: 'New Title' }); - result.current.sceneApi.getState().actions.set({ - textBoxes: { - 'tb-1': { size: { width: 100, height: 50 } } - } - }); - }); - - // Undo should restore both - act(() => { - result.current.history.undo(); - }); - - expect(result.current.modelApi.getState().title).toBe('Untitled'); - expect(result.current.sceneApi.getState().textBoxes).toEqual({}); - - // Redo should bring both back - act(() => { - result.current.history.redo(); - }); - - expect(result.current.modelApi.getState().title).toBe('New Title'); - expect(result.current.sceneApi.getState().textBoxes).toEqual({ - 'tb-1': { size: { width: 100, height: 50 } } - }); - }); - }); - - describe('clearHistory resets everything', () => { - it('clears past, future, and gestureInProgress', () => { - const { result } = renderTestHook(); - - // Build up some history - act(() => { - result.current.history.saveSnapshot(); - }); - act(() => { - result.current.modelApi.getState().actions.set({ title: 'V2' }); - result.current.history.saveSnapshot(); - }); - act(() => { - result.current.history.undo(); - }); - - // Verify we have past and future - expect(result.current.historyApi.getState().past.length).toBeGreaterThan(0); - expect(result.current.historyApi.getState().future.length).toBeGreaterThan(0); - - act(() => { - result.current.history.clearHistory(); - }); - - const state = result.current.historyApi.getState(); - expect(state.past).toHaveLength(0); - expect(state.future).toHaveLength(0); - expect(state.gestureInProgress).toBe(false); - }); - }); - - describe('new snapshot clears future (branching)', () => { - it('saving after undo discards redo stack', () => { - const { result } = renderTestHook(); - - act(() => { - result.current.history.saveSnapshot(); - }); - act(() => { - result.current.modelApi.getState().actions.set({ title: 'V2' }); - result.current.history.saveSnapshot(); - }); - - // Undo once - act(() => { - result.current.history.undo(); - }); - - expect(result.current.historyApi.getState().future.length).toBeGreaterThan(0); - - // New action should clear future - act(() => { - result.current.modelApi.getState().actions.set({ title: 'V3' }); - result.current.history.saveSnapshot(); - }); - - expect(result.current.historyApi.getState().future).toHaveLength(0); - }); - }); - - describe('multiple undo/redo traversals', () => { - it('can undo and redo through multiple states', () => { - const { result } = renderTestHook(); - - // Create 3 snapshots - act(() => { - result.current.history.saveSnapshot(); // saves "Untitled" - result.current.modelApi.getState().actions.set({ title: 'A' }); - }); - act(() => { - result.current.history.saveSnapshot(); // saves "A" - result.current.modelApi.getState().actions.set({ title: 'B' }); - }); - act(() => { - result.current.history.saveSnapshot(); // saves "B" - result.current.modelApi.getState().actions.set({ title: 'C' }); - }); - - // Current state is "C", past has [Untitled, A, B] - expect(result.current.modelApi.getState().title).toBe('C'); - - // Undo 3 times - act(() => { - result.current.history.undo(); // restores "B", pushes "C" to future - }); - expect(result.current.modelApi.getState().title).toBe('B'); - - act(() => { - result.current.history.undo(); // restores "A", pushes "B" to future - }); - expect(result.current.modelApi.getState().title).toBe('A'); - - act(() => { - result.current.history.undo(); // restores "Untitled", pushes "A" to future - }); - expect(result.current.modelApi.getState().title).toBe('Untitled'); - - // No more undo - let success = true; - act(() => { - success = result.current.history.undo(); - }); - expect(success).toBe(false); - - // Redo back through - act(() => { - result.current.history.redo(); - }); - expect(result.current.modelApi.getState().title).toBe('A'); - - act(() => { - result.current.history.redo(); - }); - expect(result.current.modelApi.getState().title).toBe('B'); - - act(() => { - result.current.history.redo(); - }); - expect(result.current.modelApi.getState().title).toBe('C'); - - // No more redo - success = true; - act(() => { - success = result.current.history.redo(); - }); - expect(success).toBe(false); - }); - }); - - describe('gesture + undo interaction', () => { - it('gesture followed by undo restores pre-gesture state', () => { - const { result } = renderTestHook(); - - // Do a gesture (simulates drag, draw, etc.) - act(() => { - result.current.history.beginGesture(); - }); - act(() => { - result.current.modelApi.getState().actions.set({ title: 'Dragged' }); - result.current.sceneApi.getState().actions.set({ - connectors: { - 'drag-conn': { - path: { - tiles: [{ x: 1, y: 1 }, { x: 2, y: 2 }], - rectangle: { x: 1, y: 1, width: 1, height: 1 } - } - } - } - }); - }); - act(() => { - result.current.history.endGesture(); - }); - - // Now undo the entire gesture as one step - act(() => { - result.current.history.undo(); - }); - - expect(result.current.modelApi.getState().title).toBe('Untitled'); - expect(result.current.sceneApi.getState().connectors).toEqual({}); - }); - }); -}); diff --git a/packages/fossflow-lib/src/components/ContextMenu/ContextMenuManager.tsx b/packages/fossflow-lib/src/components/ContextMenu/ContextMenuManager.tsx index 8b01cf6..029fbb3 100644 --- a/packages/fossflow-lib/src/components/ContextMenu/ContextMenuManager.tsx +++ b/packages/fossflow-lib/src/components/ContextMenu/ContextMenuManager.tsx @@ -2,7 +2,7 @@ import React, { useCallback } from 'react'; import { useUiStateStore } from 'src/stores/uiStateStore'; import { generateId, findNearestUnoccupiedTile } from 'src/utils'; import { useScene } from 'src/hooks/useScene'; -import { useModelStoreApi } from 'src/stores/modelStore'; +import { useModelStore } from 'src/stores/modelStore'; import { VIEW_ITEM_DEFAULTS } from 'src/config'; import { ContextMenu } from './ContextMenu'; @@ -12,7 +12,9 @@ interface Props { export const ContextMenuManager = ({ anchorEl }: Props) => { const scene = useScene(); - const modelStoreApi = useModelStoreApi(); + const model = useModelStore((state) => { + return state; + }); const contextMenu = useUiStateStore((state) => { return state.contextMenu; }); @@ -34,11 +36,10 @@ export const ContextMenuManager = ({ anchorEl }: Props) => { label: 'Add Node', onClick: () => { if (!contextMenu) return; - const { icons } = modelStoreApi.getState(); - if (icons.length > 0) { + if (model.icons.length > 0) { const modelItemId = generateId(); - const firstIcon = icons[0]; - + const firstIcon = model.icons[0]; + // Find nearest unoccupied tile (should return the same tile since context menu is for empty tiles) const targetTile = findNearestUnoccupiedTile(contextMenu.tile, scene) || contextMenu.tile; @@ -62,11 +63,10 @@ export const ContextMenuManager = ({ anchorEl }: Props) => { label: 'Add Rectangle', onClick: () => { if (!contextMenu) return; - const { colors } = modelStoreApi.getState(); - if (colors.length > 0) { + if (model.colors.length > 0) { scene.createRectangle({ id: generateId(), - color: colors[0].id, + color: model.colors[0].id, from: contextMenu.tile, to: contextMenu.tile }); diff --git a/packages/fossflow-lib/src/components/Cursor/Cursor.tsx b/packages/fossflow-lib/src/components/Cursor/Cursor.tsx index 8265eea..1e64547 100644 --- a/packages/fossflow-lib/src/components/Cursor/Cursor.tsx +++ b/packages/fossflow-lib/src/components/Cursor/Cursor.tsx @@ -22,5 +22,3 @@ export const Cursor = memo(() => { /> ); }); - -Cursor.displayName = 'Cursor'; diff --git a/packages/fossflow-lib/src/components/DebugUtils/DebugUtils.tsx b/packages/fossflow-lib/src/components/DebugUtils/DebugUtils.tsx index 7efb827..b19edef 100644 --- a/packages/fossflow-lib/src/components/DebugUtils/DebugUtils.tsx +++ b/packages/fossflow-lib/src/components/DebugUtils/DebugUtils.tsx @@ -1,6 +1,5 @@ import React from 'react'; import { Box } from '@mui/material'; -import { shallow } from 'zustand/shallow'; import { useUiStateStore } from 'src/stores/uiStateStore'; import { useResizeObserver } from 'src/hooks/useResizeObserver'; import { useScene } from 'src/hooks/useScene'; @@ -10,8 +9,7 @@ export const DebugUtils = () => { const uiState = useUiStateStore( ({ scroll, mouse, zoom, mode, rendererEl }) => { return { scroll, mouse, zoom, mode, rendererEl }; - }, - shallow + } ); const scene = useScene(); const { size: rendererSize } = useResizeObserver(uiState.rendererEl); diff --git a/packages/fossflow-lib/src/components/DebugUtils/__tests__/DebugUtils.test.tsx b/packages/fossflow-lib/src/components/DebugUtils/__tests__/DebugUtils.test.tsx index 65afad2..7b86b78 100644 --- a/packages/fossflow-lib/src/components/DebugUtils/__tests__/DebugUtils.test.tsx +++ b/packages/fossflow-lib/src/components/DebugUtils/__tests__/DebugUtils.test.tsx @@ -4,7 +4,6 @@ import { ThemeProvider } from '@mui/material/styles'; import { theme } from 'src/styles/theme'; import { ModelProvider } from 'src/stores/modelStore'; import { SceneProvider } from 'src/stores/sceneStore'; -import { HistoryProvider } from 'src/stores/historyStore'; import { UiStateProvider } from 'src/stores/uiStateStore'; import { DebugUtils } from '../DebugUtils'; @@ -14,9 +13,7 @@ describe('DebugUtils', () => { - - {children} - + {children} diff --git a/packages/fossflow-lib/src/components/DebugUtils/__tests__/SizeIndicator.test.tsx b/packages/fossflow-lib/src/components/DebugUtils/__tests__/SizeIndicator.test.tsx index 03b66fb..1d24cc8 100644 --- a/packages/fossflow-lib/src/components/DebugUtils/__tests__/SizeIndicator.test.tsx +++ b/packages/fossflow-lib/src/components/DebugUtils/__tests__/SizeIndicator.test.tsx @@ -4,7 +4,6 @@ import { ThemeProvider } from '@mui/material/styles'; import { theme } from 'src/styles/theme'; import { ModelProvider } from 'src/stores/modelStore'; import { SceneProvider } from 'src/stores/sceneStore'; -import { HistoryProvider } from 'src/stores/historyStore'; import { UiStateProvider } from 'src/stores/uiStateStore'; import { SizeIndicator } from '../SizeIndicator'; @@ -14,9 +13,7 @@ describe('SizeIndicator', () => { - - {children} - + {children} diff --git a/packages/fossflow-lib/src/components/IsoTileArea/IsoTileArea.tsx b/packages/fossflow-lib/src/components/IsoTileArea/IsoTileArea.tsx index fdb1ea1..44770fd 100644 --- a/packages/fossflow-lib/src/components/IsoTileArea/IsoTileArea.tsx +++ b/packages/fossflow-lib/src/components/IsoTileArea/IsoTileArea.tsx @@ -55,5 +55,3 @@ export const IsoTileArea = memo(({ ); }); - -IsoTileArea.displayName = 'IsoTileArea'; diff --git a/packages/fossflow-lib/src/components/ItemControls/ConnectorControls/ConnectorControls.tsx b/packages/fossflow-lib/src/components/ItemControls/ConnectorControls/ConnectorControls.tsx index c0261e5..bad45c9 100644 --- a/packages/fossflow-lib/src/components/ItemControls/ConnectorControls/ConnectorControls.tsx +++ b/packages/fossflow-lib/src/components/ItemControls/ConnectorControls/ConnectorControls.tsx @@ -24,7 +24,6 @@ import { ColorPicker } from 'src/components/ColorSelector/ColorPicker'; import { CustomColorInput } from 'src/components/ColorSelector/CustomColorInput'; import { useUiStateStore } from 'src/stores/uiStateStore'; import { useScene } from 'src/hooks/useScene'; -import { useHistory } from 'src/hooks/useHistory'; import { Close as CloseIcon, Add as AddIcon, @@ -45,7 +44,6 @@ export const ConnectorControls = ({ id }: Props) => { }); const connector = useConnector(id); const { updateConnector, deleteConnector } = useScene(); - const { beginGesture, endGesture } = useHistory(); const [useCustomColor, setUseCustomColor] = useState( !!connector?.customColor ); @@ -209,8 +207,6 @@ export const ConnectorControls = ({ id }: Props) => { { return handleUpdateLabel(label.id, { text: e.target.value diff --git a/packages/fossflow-lib/src/components/ItemControls/NodeControls/NodeSettings/NodeSettings.tsx b/packages/fossflow-lib/src/components/ItemControls/NodeControls/NodeSettings/NodeSettings.tsx index ad79783..191f6b2 100644 --- a/packages/fossflow-lib/src/components/ItemControls/NodeControls/NodeSettings/NodeSettings.tsx +++ b/packages/fossflow-lib/src/components/ItemControls/NodeControls/NodeSettings/NodeSettings.tsx @@ -4,7 +4,6 @@ import { ModelItem, ViewItem } from 'src/types'; import { RichTextEditor } from 'src/components/RichTextEditor/RichTextEditor'; import { useModelItem } from 'src/hooks/useModelItem'; import { useModelStore } from 'src/stores/modelStore'; -import { useHistory } from 'src/hooks/useHistory'; import { DeleteButton } from '../../components/DeleteButton'; import { Section } from '../../components/Section'; @@ -29,7 +28,6 @@ export const NodeSettings = ({ const modelItem = useModelItem(node.id); const modelActions = useModelStore((state) => state.actions); const icons = useModelStore((state) => state.icons); - const { beginGesture, endGesture, isGestureInProgress } = useHistory(); // Local state for smooth slider interaction const currentIcon = icons.find(icon => icon.id === modelItem?.icon); @@ -59,13 +57,10 @@ export const NodeSettings = ({ // Handle slider change with local state + debounced store update const handleScaleChange = useCallback((e: Event, newScale: number | number[]) => { - if (!isGestureInProgress()) { - beginGesture(); - } const scale = newScale as number; setLocalScale(scale); // Immediate UI update updateIconScale(scale); // Debounced store update - }, [updateIconScale, beginGesture, isGestureInProgress]); + }, [updateIconScale]); // Cleanup timeout on unmount useEffect(() => { @@ -85,8 +80,6 @@ export const NodeSettings = ({
beginGesture()} - onBlur={() => endGesture()} onChange={(e) => { const text = e.target.value as string; if (modelItem.name !== text) onModelItemUpdated({ name: text }); @@ -96,8 +89,6 @@ export const NodeSettings = ({
beginGesture()} - onBlur={() => endGesture()} onChange={(text) => { if (modelItem.description !== text) onModelItemUpdated({ description: text }); @@ -128,7 +119,6 @@ export const NodeSettings = ({ max={2.5} value={localScale} onChange={handleScaleChange} - onChangeCommitted={endGesture} />
diff --git a/packages/fossflow-lib/src/components/ItemControls/TextBoxControls/TextBoxControls.tsx b/packages/fossflow-lib/src/components/ItemControls/TextBoxControls/TextBoxControls.tsx index 5fdda7a..d7a90dc 100644 --- a/packages/fossflow-lib/src/components/ItemControls/TextBoxControls/TextBoxControls.tsx +++ b/packages/fossflow-lib/src/components/ItemControls/TextBoxControls/TextBoxControls.tsx @@ -16,7 +16,6 @@ import { useTextBox } from 'src/hooks/useTextBox'; import { useUiStateStore } from 'src/stores/uiStateStore'; import { getIsoProjectionCss } from 'src/utils'; import { useScene } from 'src/hooks/useScene'; -import { useHistory } from 'src/hooks/useHistory'; import { ControlsContainer } from '../components/ControlsContainer'; import { Section } from '../components/Section'; import { DeleteButton } from '../components/DeleteButton'; @@ -31,7 +30,6 @@ export const TextBoxControls = ({ id }: Props) => { }); const textBox = useTextBox(id); const { updateTextBox, deleteTextBox } = useScene(); - const { beginGesture, endGesture } = useHistory(); // If textBox doesn't exist, return null if (!textBox) { @@ -60,8 +58,6 @@ export const TextBoxControls = ({ id }: Props) => {
{ updateTextBox(textBox.id, { content: e.target.value as string }); }} diff --git a/packages/fossflow-lib/src/components/MainMenu/MainMenu.tsx b/packages/fossflow-lib/src/components/MainMenu/MainMenu.tsx index ccd7962..dab16bb 100644 --- a/packages/fossflow-lib/src/components/MainMenu/MainMenu.tsx +++ b/packages/fossflow-lib/src/components/MainMenu/MainMenu.tsx @@ -12,7 +12,6 @@ import { Settings as SettingsIcon, } from '@mui/icons-material'; -import { shallow } from 'zustand/shallow'; import { UiElement } from 'src/components/UiElement/UiElement'; import { IconButton } from 'src/components/IconButton/IconButton'; import { useUiStateStore } from 'src/stores/uiStateStore'; @@ -33,7 +32,7 @@ export const MainMenu = () => { const [anchorEl, setAnchorEl] = useState(null); const model = useModelStore((state) => { return modelFromModelStore(state); - }, shallow); + }); const isMainMenuOpen = useUiStateStore((state) => { return state.isMainMenuOpen; }); @@ -121,13 +120,13 @@ export const MainMenu = () => { }, [uiStateActions, clear, clearHistory]); const handleUndo = useCallback(() => { - const success = undo(); - if (success) uiStateActions.setIsMainMenuOpen(false); + undo(); + uiStateActions.setIsMainMenuOpen(false); }, [undo, uiStateActions]); const handleRedo = useCallback(() => { - const success = redo(); - if (success) uiStateActions.setIsMainMenuOpen(false); + redo(); + uiStateActions.setIsMainMenuOpen(false); }, [redo, uiStateActions]); const onOpenSettings = useCallback(() => { diff --git a/packages/fossflow-lib/src/components/RichTextEditor/RichTextEditor.tsx b/packages/fossflow-lib/src/components/RichTextEditor/RichTextEditor.tsx index 871909a..eb699d2 100644 --- a/packages/fossflow-lib/src/components/RichTextEditor/RichTextEditor.tsx +++ b/packages/fossflow-lib/src/components/RichTextEditor/RichTextEditor.tsx @@ -6,8 +6,6 @@ import RichTextEditorErrorBoundary from './RichTextEditorErrorBoundary'; interface Props { value?: string; onChange?: (value: string) => void; - onFocus?: () => void; - onBlur?: () => void; readOnly?: boolean; height?: number; styles?: React.CSSProperties; @@ -44,8 +42,6 @@ const formats = [ export const RichTextEditor = ({ value, onChange, - onFocus, - onBlur, readOnly, height = 120, styles @@ -96,8 +92,6 @@ export const RichTextEditor = ({ value={value ?? ''} readOnly={readOnly} onChange={onChange} - onFocus={onFocus} - onBlur={onBlur} formats={formats} modules={modules} /> diff --git a/packages/fossflow-lib/src/components/SceneLayer/SceneLayer.tsx b/packages/fossflow-lib/src/components/SceneLayer/SceneLayer.tsx index 769f623..3425ea6 100644 --- a/packages/fossflow-lib/src/components/SceneLayer/SceneLayer.tsx +++ b/packages/fossflow-lib/src/components/SceneLayer/SceneLayer.tsx @@ -60,5 +60,3 @@ export const SceneLayer = memo(({ ); }); - -SceneLayer.displayName = 'SceneLayer'; diff --git a/packages/fossflow-lib/src/components/SceneLayers/ConnectorLabels/ConnectorLabel.tsx b/packages/fossflow-lib/src/components/SceneLayers/ConnectorLabels/ConnectorLabel.tsx index 4c6bf19..8554c22 100644 --- a/packages/fossflow-lib/src/components/SceneLayers/ConnectorLabels/ConnectorLabel.tsx +++ b/packages/fossflow-lib/src/components/SceneLayers/ConnectorLabels/ConnectorLabel.tsx @@ -124,5 +124,3 @@ export const ConnectorLabel = memo(({ connector: sceneConnector }: Props) => { ); }); - -ConnectorLabel.displayName = 'ConnectorLabel'; diff --git a/packages/fossflow-lib/src/components/SceneLayers/ConnectorLabels/ConnectorLabels.tsx b/packages/fossflow-lib/src/components/SceneLayers/ConnectorLabels/ConnectorLabels.tsx index 7011cb8..a397243 100644 --- a/packages/fossflow-lib/src/components/SceneLayers/ConnectorLabels/ConnectorLabels.tsx +++ b/packages/fossflow-lib/src/components/SceneLayers/ConnectorLabels/ConnectorLabels.tsx @@ -1,4 +1,4 @@ -import { useMemo, memo } from 'react'; +import React from 'react'; import { useScene } from 'src/hooks/useScene'; import { ConnectorLabel } from './ConnectorLabel'; @@ -6,27 +6,21 @@ interface Props { connectors: ReturnType['connectors']; } -export const ConnectorLabels = memo(({ connectors }: Props) => { - const labeledConnectors = useMemo( - () => - connectors.filter((connector) => { - return Boolean( - connector.description || - connector.startLabel || - connector.endLabel || - (connector.labels && connector.labels.length > 0) - ); - }), - [connectors] - ); - +export const ConnectorLabels = ({ connectors }: Props) => { return ( <> - {labeledConnectors.map((connector) => { - return ; - })} + {connectors + .filter((connector) => { + return Boolean( + connector.description || + connector.startLabel || + connector.endLabel || + (connector.labels && connector.labels.length > 0) + ); + }) + .map((connector) => { + return ; + })} ); -}); - -ConnectorLabels.displayName = 'ConnectorLabels'; +}; diff --git a/packages/fossflow-lib/src/components/SceneLayers/Connectors/Connector.tsx b/packages/fossflow-lib/src/components/SceneLayers/Connectors/Connector.tsx index 27b7b1c..dff9394 100644 --- a/packages/fossflow-lib/src/components/SceneLayers/Connectors/Connector.tsx +++ b/packages/fossflow-lib/src/components/SceneLayers/Connectors/Connector.tsx @@ -327,5 +327,3 @@ export const Connector = memo(({ connector: _connector, isSelected }: Props) => ); }); - -Connector.displayName = 'Connector'; diff --git a/packages/fossflow-lib/src/components/SceneLayers/Connectors/Connectors.tsx b/packages/fossflow-lib/src/components/SceneLayers/Connectors/Connectors.tsx index 223ad12..15fb7d6 100644 --- a/packages/fossflow-lib/src/components/SceneLayers/Connectors/Connectors.tsx +++ b/packages/fossflow-lib/src/components/SceneLayers/Connectors/Connectors.tsx @@ -1,4 +1,4 @@ -import { useMemo, memo } from 'react'; +import React, { useMemo } from 'react'; import type { useScene } from 'src/hooks/useScene'; import { useUiStateStore } from 'src/stores/uiStateStore'; import { Connector } from './Connector'; @@ -7,7 +7,7 @@ interface Props { connectors: ReturnType['connectors']; } -export const Connectors = memo(({ connectors }: Props) => { +export const Connectors = ({ connectors }: Props) => { const itemControls = useUiStateStore((state) => { return state.itemControls; }); @@ -27,11 +27,9 @@ export const Connectors = memo(({ connectors }: Props) => { return null; }, [mode, itemControls]); - const reversedConnectors = useMemo(() => [...connectors].reverse(), [connectors]); - return ( <> - {reversedConnectors.map((connector) => { + {[...connectors].reverse().map((connector) => { return ( { })} ); -}); - -Connectors.displayName = 'Connectors'; +}; diff --git a/packages/fossflow-lib/src/components/SceneLayers/Nodes/Node/Node.tsx b/packages/fossflow-lib/src/components/SceneLayers/Nodes/Node/Node.tsx index 8b74ab7..b0db54f 100644 --- a/packages/fossflow-lib/src/components/SceneLayers/Nodes/Node/Node.tsx +++ b/packages/fossflow-lib/src/components/SceneLayers/Nodes/Node/Node.tsx @@ -94,5 +94,3 @@ export const Node = memo(({ node, order }: Props) => { ); }); - -Node.displayName = 'Node'; diff --git a/packages/fossflow-lib/src/components/SceneLayers/Nodes/Nodes.tsx b/packages/fossflow-lib/src/components/SceneLayers/Nodes/Nodes.tsx index c84fa56..f8e4f36 100644 --- a/packages/fossflow-lib/src/components/SceneLayers/Nodes/Nodes.tsx +++ b/packages/fossflow-lib/src/components/SceneLayers/Nodes/Nodes.tsx @@ -1,4 +1,4 @@ -import { useMemo, memo } from 'react'; +import React from 'react'; import { ViewItem } from 'src/types'; import { Node } from './Node/Node'; @@ -6,18 +6,14 @@ interface Props { nodes: ViewItem[]; } -export const Nodes = memo(({ nodes }: Props) => { - const reversedNodes = useMemo(() => [...nodes].reverse(), [nodes]); - +export const Nodes = ({ nodes }: Props) => { return ( <> - {reversedNodes.map((node) => { + {[...nodes].reverse().map((node) => { return ( ); })} ); -}); - -Nodes.displayName = 'Nodes'; +}; diff --git a/packages/fossflow-lib/src/components/SceneLayers/Rectangles/Rectangle.tsx b/packages/fossflow-lib/src/components/SceneLayers/Rectangles/Rectangle.tsx index 0716668..5cf187b 100644 --- a/packages/fossflow-lib/src/components/SceneLayers/Rectangles/Rectangle.tsx +++ b/packages/fossflow-lib/src/components/SceneLayers/Rectangles/Rectangle.tsx @@ -31,5 +31,3 @@ export const Rectangle = memo(({ from, to, color: colorId, customColor }: Props) /> ); }); - -Rectangle.displayName = 'Rectangle'; diff --git a/packages/fossflow-lib/src/components/SceneLayers/Rectangles/Rectangles.tsx b/packages/fossflow-lib/src/components/SceneLayers/Rectangles/Rectangles.tsx index 86b25e5..889d519 100644 --- a/packages/fossflow-lib/src/components/SceneLayers/Rectangles/Rectangles.tsx +++ b/packages/fossflow-lib/src/components/SceneLayers/Rectangles/Rectangles.tsx @@ -1,4 +1,4 @@ -import { useMemo, memo } from 'react'; +import React from 'react'; import { useScene } from 'src/hooks/useScene'; import { Rectangle } from './Rectangle'; @@ -6,16 +6,12 @@ interface Props { rectangles: ReturnType['rectangles']; } -export const Rectangles = memo(({ rectangles }: Props) => { - const reversedRectangles = useMemo(() => [...rectangles].reverse(), [rectangles]); - +export const Rectangles = ({ rectangles }: Props) => { return ( <> - {reversedRectangles.map((rectangle) => { + {[...rectangles].reverse().map((rectangle) => { return ; })} ); -}); - -Rectangles.displayName = 'Rectangles'; +}; diff --git a/packages/fossflow-lib/src/components/SceneLayers/TextBoxes/TextBox.tsx b/packages/fossflow-lib/src/components/SceneLayers/TextBoxes/TextBox.tsx index aff73bc..96defe6 100644 --- a/packages/fossflow-lib/src/components/SceneLayers/TextBoxes/TextBox.tsx +++ b/packages/fossflow-lib/src/components/SceneLayers/TextBoxes/TextBox.tsx @@ -50,5 +50,3 @@ export const TextBox = memo(({ textBox }: Props) => { ); }); - -TextBox.displayName = 'TextBox'; diff --git a/packages/fossflow-lib/src/components/SceneLayers/TextBoxes/TextBoxes.tsx b/packages/fossflow-lib/src/components/SceneLayers/TextBoxes/TextBoxes.tsx index 20f5745..efd9c4d 100644 --- a/packages/fossflow-lib/src/components/SceneLayers/TextBoxes/TextBoxes.tsx +++ b/packages/fossflow-lib/src/components/SceneLayers/TextBoxes/TextBoxes.tsx @@ -1,4 +1,4 @@ -import { useMemo, memo } from 'react'; +import React from 'react'; import { useScene } from 'src/hooks/useScene'; import { TextBox } from './TextBox'; @@ -6,16 +6,12 @@ interface Props { textBoxes: ReturnType['textBoxes']; } -export const TextBoxes = memo(({ textBoxes }: Props) => { - const reversedTextBoxes = useMemo(() => [...textBoxes].reverse(), [textBoxes]); - +export const TextBoxes = ({ textBoxes }: Props) => { return ( <> - {reversedTextBoxes.map((textBox) => { + {[...textBoxes].reverse().map((textBox) => { return ; })} ); -}); - -TextBoxes.displayName = 'TextBoxes'; +}; diff --git a/packages/fossflow-lib/src/hooks/__tests__/useHistory.test.tsx b/packages/fossflow-lib/src/hooks/__tests__/useHistory.test.tsx index 4d88c4e..24e8374 100644 --- a/packages/fossflow-lib/src/hooks/__tests__/useHistory.test.tsx +++ b/packages/fossflow-lib/src/hooks/__tests__/useHistory.test.tsx @@ -1,461 +1,329 @@ -import React from 'react'; import { renderHook, act } from '@testing-library/react'; -import { createStore } from 'zustand'; import { useHistory } from '../useHistory'; -import type { HistoryStoreState, HistoryEntry } from 'src/stores/historyStore'; -import type { Model, Scene } from 'src/types'; -// --- Minimal mock data --- - -const makeModel = (title: string): Model => ({ - title, - colors: [], - icons: [], - items: [], - views: [] -}); - -const makeScene = (extra?: Record): Scene => ({ - connectors: {}, - textBoxes: {}, - ...extra -}); - -const makeEntry = (title: string): HistoryEntry => ({ - model: makeModel(title), - scene: makeScene() -}); - -// --- Mock stores --- - -const createMockModelStore = (initial: Model) => { - return createStore<{ - title: string; - colors: Model['colors']; - icons: Model['icons']; - items: Model['items']; - views: Model['views']; - version?: string; - description?: string; - actions: { - get: () => any; - set: (updates: Partial) => void; - }; - }>((set, get) => ({ - ...initial, - actions: { - get, - set: (updates: Partial) => set((state) => ({ ...state, ...updates })) - } - })); +// Mock implementations +const mockModelStore = { + canUndo: jest.fn(), + canRedo: jest.fn(), + undo: jest.fn(), + redo: jest.fn(), + saveToHistory: jest.fn(), + clearHistory: jest.fn() }; -const createMockSceneStore = (initial: Scene) => { - return createStore<{ - connectors: Scene['connectors']; - textBoxes: Scene['textBoxes']; - actions: { - get: () => any; - set: (updates: Partial) => void; - }; - }>((set, get) => ({ - ...initial, - actions: { - get, - set: (updates: Partial) => set((state) => ({ ...state, ...updates })) - } - })); +const mockSceneStore = { + canUndo: jest.fn(), + canRedo: jest.fn(), + undo: jest.fn(), + redo: jest.fn(), + saveToHistory: jest.fn(), + clearHistory: jest.fn() }; -const MAX_HISTORY = 50; - -const createMockHistoryStore = () => { - return createStore((set, get) => ({ - past: [], - future: [], - gestureInProgress: false, - maxSize: MAX_HISTORY, - actions: { - saveSnapshot: (currentEntry: HistoryEntry) => { - const { gestureInProgress, maxSize } = get(); - if (gestureInProgress) return; - set((state) => { - const newPast = [...state.past, currentEntry]; - if (newPast.length > maxSize) newPast.shift(); - return { past: newPast, future: [] }; - }); - }, - undo: (currentEntry: HistoryEntry) => { - const { past } = get(); - if (past.length === 0) return null; - const previous = past[past.length - 1]; - const newPast = past.slice(0, -1); - set({ past: newPast, future: [currentEntry, ...get().future] }); - return previous; - }, - redo: (currentEntry: HistoryEntry) => { - const { future } = get(); - if (future.length === 0) return null; - const next = future[0]; - const newFuture = future.slice(1); - set((state) => ({ past: [...state.past, currentEntry], future: newFuture })); - return next; - }, - clearHistory: () => { - set({ past: [], future: [], gestureInProgress: false }); - }, - beginGesture: (currentEntry: HistoryEntry) => { - const { gestureInProgress, maxSize } = get(); - if (gestureInProgress) return; - set((state) => { - const newPast = [...state.past, currentEntry]; - if (newPast.length > maxSize) newPast.shift(); - return { past: newPast, future: [], gestureInProgress: true }; - }); - }, - endGesture: () => { - set({ gestureInProgress: false }); - }, - cancelGesture: () => { - const { past, gestureInProgress } = get(); - if (!gestureInProgress || past.length === 0) { - set({ gestureInProgress: false }); - return null; - } - const previous = past[past.length - 1]; - const newPast = past.slice(0, -1); - set({ past: newPast, gestureInProgress: false }); - return previous; - } - } - })); -}; - -// --- Mock context providers --- - -let mockModelStore: ReturnType; -let mockSceneStore: ReturnType; -let mockHistoryStore: ReturnType; - +// Mock the store hooks jest.mock('../../stores/modelStore', () => ({ - useModelStoreApi: () => mockModelStore + useModelStore: jest.fn((selector) => { + const state = { + actions: mockModelStore + }; + return selector ? selector(state) : state; + }) })); jest.mock('../../stores/sceneStore', () => ({ - useSceneStoreApi: () => mockSceneStore -})); - -jest.mock('../../stores/historyStore', () => ({ - useHistoryStore: (selector: (state: HistoryStoreState) => any) => { - // Need to subscribe to trigger re-renders, but in tests we just call getState - return selector(mockHistoryStore.getState()); - }, - useHistoryStoreApi: () => mockHistoryStore, - extractModelData: (state: any) => ({ - version: state.version, - title: state.title, - description: state.description, - colors: state.colors, - icons: state.icons, - items: state.items, - views: state.views - }), - extractSceneData: (state: any) => ({ - connectors: state.connectors, - textBoxes: state.textBoxes + useSceneStore: jest.fn((selector) => { + const state = { + actions: mockSceneStore + }; + return selector ? selector(state) : state; }) })); describe('useHistory', () => { beforeEach(() => { - mockModelStore = createMockModelStore(makeModel('initial')); - mockSceneStore = createMockSceneStore(makeScene()); - mockHistoryStore = createMockHistoryStore(); + jest.clearAllMocks(); + + // Reset mock implementations + mockModelStore.canUndo.mockReturnValue(false); + mockModelStore.canRedo.mockReturnValue(false); + mockModelStore.undo.mockReturnValue(true); + mockModelStore.redo.mockReturnValue(true); + + mockSceneStore.canUndo.mockReturnValue(false); + mockSceneStore.canRedo.mockReturnValue(false); + mockSceneStore.undo.mockReturnValue(true); + mockSceneStore.redo.mockReturnValue(true); }); - describe('canUndo / canRedo', () => { + describe('undo/redo basic functionality', () => { it('should initialize with no undo/redo capability', () => { const { result } = renderHook(() => useHistory()); + expect(result.current.canUndo).toBe(false); expect(result.current.canRedo).toBe(false); }); - it('canUndo should be true after saveSnapshot', () => { + it('should call saveToHistory on both stores', () => { const { result } = renderHook(() => useHistory()); - + act(() => { - result.current.saveSnapshot(); + result.current.saveToHistory(); }); + + expect(mockModelStore.saveToHistory).toHaveBeenCalled(); + expect(mockSceneStore.saveToHistory).toHaveBeenCalled(); + }); - // Re-render to pick up store changes - const { result: result2 } = renderHook(() => useHistory()); - expect(result2.current.canUndo).toBe(true); - expect(result2.current.canRedo).toBe(false); + it('should perform undo when model store has history', () => { + mockModelStore.canUndo.mockReturnValue(true); + const { result } = renderHook(() => useHistory()); + + expect(result.current.canUndo).toBe(true); + + act(() => { + const success = result.current.undo(); + expect(success).toBe(true); + }); + + expect(mockModelStore.undo).toHaveBeenCalled(); + }); + + it('should perform undo when scene store has history', () => { + mockSceneStore.canUndo.mockReturnValue(true); + const { result } = renderHook(() => useHistory()); + + expect(result.current.canUndo).toBe(true); + + act(() => { + const success = result.current.undo(); + expect(success).toBe(true); + }); + + expect(mockSceneStore.undo).toHaveBeenCalled(); + }); + + it('should perform redo when model store has future', () => { + mockModelStore.canRedo.mockReturnValue(true); + const { result } = renderHook(() => useHistory()); + + expect(result.current.canRedo).toBe(true); + + act(() => { + const success = result.current.redo(); + expect(success).toBe(true); + }); + + expect(mockModelStore.redo).toHaveBeenCalled(); + }); + + it('should return false when undo is called with no history', () => { + mockModelStore.undo.mockReturnValue(false); + mockSceneStore.undo.mockReturnValue(false); + + const { result } = renderHook(() => useHistory()); + + act(() => { + const success = result.current.undo(); + expect(success).toBe(false); + }); + }); + + it('should return false when redo is called with no future', () => { + mockModelStore.redo.mockReturnValue(false); + mockSceneStore.redo.mockReturnValue(false); + + const { result } = renderHook(() => useHistory()); + + act(() => { + const success = result.current.redo(); + expect(success).toBe(false); + }); }); }); - describe('saveSnapshot', () => { - it('should capture model + scene and delegate to history store', () => { + describe('transaction functionality', () => { + it('should save history before transaction and not during', () => { const { result } = renderHook(() => useHistory()); - + act(() => { - result.current.saveSnapshot(); + result.current.transaction(() => { + // This should not trigger saveToHistory due to transaction + result.current.saveToHistory(); + }); }); - - const state = mockHistoryStore.getState(); - expect(state.past).toHaveLength(1); - expect(state.past[0].model.title).toBe('initial'); - expect(state.future).toHaveLength(0); + + // Should save once before transaction starts + expect(mockModelStore.saveToHistory).toHaveBeenCalledTimes(1); + expect(mockSceneStore.saveToHistory).toHaveBeenCalledTimes(1); }); - it('should clear future on new snapshot', () => { - // Seed some past and future - const entry1 = makeEntry('entry1'); - const entry2 = makeEntry('entry2'); - mockHistoryStore.setState({ - past: [entry1], - future: [entry2] - }); - + it('should track transaction state correctly', () => { const { result } = renderHook(() => useHistory()); - + + expect(result.current.isInTransaction()).toBe(false); + act(() => { - result.current.saveSnapshot(); + result.current.transaction(() => { + expect(result.current.isInTransaction()).toBe(true); + }); }); + + expect(result.current.isInTransaction()).toBe(false); + }); - const state = mockHistoryStore.getState(); - expect(state.future).toHaveLength(0); - expect(state.past).toHaveLength(2); + it('should prevent nested transactions', () => { + const { result } = renderHook(() => useHistory()); + + act(() => { + result.current.transaction(() => { + // First transaction saves history + expect(mockModelStore.saveToHistory).toHaveBeenCalledTimes(1); + + // Nested transaction should not save again + result.current.transaction(() => { + // Still in transaction + expect(result.current.isInTransaction()).toBe(true); + }); + + // Should still be 1 save + expect(mockModelStore.saveToHistory).toHaveBeenCalledTimes(1); + }); + }); + }); + + it('should handle transaction errors gracefully', () => { + const { result } = renderHook(() => useHistory()); + + expect(() => { + act(() => { + result.current.transaction(() => { + throw new Error('Test error'); + }); + }); + }).toThrow('Test error'); + + // Transaction should be cleaned up + expect(result.current.isInTransaction()).toBe(false); }); }); - describe('undo', () => { - it('should return previous entry and apply to both stores atomically', () => { - const previousEntry = makeEntry('previous'); - mockHistoryStore.setState({ past: [previousEntry] }); - + describe('history management', () => { + it('should clear all history', () => { const { result } = renderHook(() => useHistory()); - - let success: boolean = false; - act(() => { - success = result.current.undo(); - }); - - expect(success).toBe(true); - // Model store should now have the previous entry's data - expect(mockModelStore.getState().title).toBe('previous'); - }); - - it('should return false when no history available', () => { - const { result } = renderHook(() => useHistory()); - - let success: boolean = true; - act(() => { - success = result.current.undo(); - }); - - expect(success).toBe(false); - }); - - it('should push current state to future on undo', () => { - const previousEntry = makeEntry('previous'); - mockHistoryStore.setState({ past: [previousEntry] }); - - const { result } = renderHook(() => useHistory()); - - act(() => { - result.current.undo(); - }); - - const state = mockHistoryStore.getState(); - expect(state.past).toHaveLength(0); - expect(state.future).toHaveLength(1); - expect(state.future[0].model.title).toBe('initial'); - }); - }); - - describe('redo', () => { - it('should return next entry and apply', () => { - const futureEntry = makeEntry('future'); - mockHistoryStore.setState({ future: [futureEntry] }); - - const { result } = renderHook(() => useHistory()); - - let success: boolean = false; - act(() => { - success = result.current.redo(); - }); - - expect(success).toBe(true); - expect(mockModelStore.getState().title).toBe('future'); - }); - - it('should return false when no future available', () => { - const { result } = renderHook(() => useHistory()); - - let success: boolean = true; - act(() => { - success = result.current.redo(); - }); - - expect(success).toBe(false); - }); - - it('should push current state to past on redo (not the redo entry)', () => { - const futureEntry = makeEntry('future'); - mockHistoryStore.setState({ future: [futureEntry] }); - - const { result } = renderHook(() => useHistory()); - - act(() => { - result.current.redo(); - }); - - const state = mockHistoryStore.getState(); - // past should contain what was current before redo (title='initial'), not 'future' - expect(state.past).toHaveLength(1); - expect(state.past[0].model.title).toBe('initial'); - }); - - it('undo+redo+undo cycle should not cause data loss', () => { - const { result } = renderHook(() => useHistory()); - - // Save snapshot (captures 'initial'), then change model to 'changed' - act(() => { - result.current.saveSnapshot(); - }); - mockModelStore.getState().actions.set({ title: 'changed' }); - - // Undo: should restore 'initial' - act(() => { - result.current.undo(); - }); - expect(mockModelStore.getState().title).toBe('initial'); - - // Redo: should restore 'changed' - act(() => { - result.current.redo(); - }); - expect(mockModelStore.getState().title).toBe('changed'); - - // Undo again: should restore 'initial' (not get stuck on 'changed') - act(() => { - result.current.undo(); - }); - expect(mockModelStore.getState().title).toBe('initial'); - }); - }); - - describe('gesture lifecycle', () => { - it('beginGesture should save snapshot and set gestureInProgress', () => { - const { result } = renderHook(() => useHistory()); - - act(() => { - result.current.beginGesture(); - }); - - const state = mockHistoryStore.getState(); - expect(state.gestureInProgress).toBe(true); - expect(state.past).toHaveLength(1); - expect(state.past[0].model.title).toBe('initial'); - }); - - it('endGesture should clear gestureInProgress', () => { - const { result } = renderHook(() => useHistory()); - - act(() => { - result.current.beginGesture(); - }); - - expect(mockHistoryStore.getState().gestureInProgress).toBe(true); - - act(() => { - result.current.endGesture(); - }); - - expect(mockHistoryStore.getState().gestureInProgress).toBe(false); - }); - - it('saveSnapshot should be blocked during gesture', () => { - const { result } = renderHook(() => useHistory()); - - act(() => { - result.current.beginGesture(); - }); - - // past has 1 entry from beginGesture - expect(mockHistoryStore.getState().past).toHaveLength(1); - - act(() => { - result.current.saveSnapshot(); - result.current.saveSnapshot(); - result.current.saveSnapshot(); - }); - - // Should still be 1 - saves blocked during gesture - expect(mockHistoryStore.getState().past).toHaveLength(1); - }); - - it('cancelGesture should restore pre-gesture state', () => { - // Set up initial state - mockModelStore.getState().actions.set({ title: 'before-gesture' }); - - const { result } = renderHook(() => useHistory()); - - act(() => { - result.current.beginGesture(); - }); - - // Simulate changes during gesture - mockModelStore.getState().actions.set({ title: 'during-gesture' }); - expect(mockModelStore.getState().title).toBe('during-gesture'); - - act(() => { - result.current.cancelGesture(); - }); - - // Should be restored to pre-gesture state - expect(mockModelStore.getState().title).toBe('before-gesture'); - expect(mockHistoryStore.getState().gestureInProgress).toBe(false); - }); - - it('isGestureInProgress should reflect state', () => { - const { result } = renderHook(() => useHistory()); - - expect(result.current.isGestureInProgress()).toBe(false); - - act(() => { - result.current.beginGesture(); - }); - - expect(result.current.isGestureInProgress()).toBe(true); - - act(() => { - result.current.endGesture(); - }); - - expect(result.current.isGestureInProgress()).toBe(false); - }); - }); - - describe('clearHistory', () => { - it('should reset all history state', () => { - mockHistoryStore.setState({ - past: [makeEntry('a'), makeEntry('b')], - future: [makeEntry('c')], - gestureInProgress: true - }); - - const { result } = renderHook(() => useHistory()); - + act(() => { result.current.clearHistory(); }); + + expect(mockModelStore.clearHistory).toHaveBeenCalled(); + expect(mockSceneStore.clearHistory).toHaveBeenCalled(); + }); - const state = mockHistoryStore.getState(); - expect(state.past).toHaveLength(0); - expect(state.future).toHaveLength(0); - expect(state.gestureInProgress).toBe(false); + it('should check both stores for undo capability', () => { + // Only model has undo + mockModelStore.canUndo.mockReturnValue(true); + mockSceneStore.canUndo.mockReturnValue(false); + + const { result: result1 } = renderHook(() => useHistory()); + expect(result1.current.canUndo).toBe(true); + + // Only scene has undo + mockModelStore.canUndo.mockReturnValue(false); + mockSceneStore.canUndo.mockReturnValue(true); + + const { result: result2 } = renderHook(() => useHistory()); + expect(result2.current.canUndo).toBe(true); + + // Both have undo + mockModelStore.canUndo.mockReturnValue(true); + mockSceneStore.canUndo.mockReturnValue(true); + + const { result: result3 } = renderHook(() => useHistory()); + expect(result3.current.canUndo).toBe(true); + + // Neither has undo + mockModelStore.canUndo.mockReturnValue(false); + mockSceneStore.canUndo.mockReturnValue(false); + + const { result: result4 } = renderHook(() => useHistory()); + expect(result4.current.canUndo).toBe(false); + }); + + it('should check both stores for redo capability', () => { + // Only model has redo + mockModelStore.canRedo.mockReturnValue(true); + mockSceneStore.canRedo.mockReturnValue(false); + + const { result: result1 } = renderHook(() => useHistory()); + expect(result1.current.canRedo).toBe(true); + + // Only scene has redo + mockModelStore.canRedo.mockReturnValue(false); + mockSceneStore.canRedo.mockReturnValue(true); + + const { result: result2 } = renderHook(() => useHistory()); + expect(result2.current.canRedo).toBe(true); }); }); -}); + + describe('edge cases', () => { + it('should handle missing store actions gracefully', () => { + // Mock stores returning undefined actions + const useModelStore = require('../../stores/modelStore').useModelStore; + const useSceneStore = require('../../stores/sceneStore').useSceneStore; + + useModelStore.mockImplementation((selector) => { + const state = { actions: undefined }; + return selector ? selector(state) : state; + }); + useSceneStore.mockImplementation((selector) => { + const state = { actions: undefined }; + return selector ? selector(state) : state; + }); + + const { result } = renderHook(() => useHistory()); + + // Should not throw and return safe defaults + expect(result.current.canUndo).toBe(false); + expect(result.current.canRedo).toBe(false); + + act(() => { + expect(result.current.undo()).toBe(false); + expect(result.current.redo()).toBe(false); + // These should not throw + result.current.saveToHistory(); + result.current.clearHistory(); + result.current.transaction(() => {}); + }); + + // Restore mocks for other tests + useModelStore.mockImplementation((selector) => { + const state = { actions: mockModelStore }; + return selector ? selector(state) : state; + }); + useSceneStore.mockImplementation((selector) => { + const state = { actions: mockSceneStore }; + return selector ? selector(state) : state; + }); + }); + + it('should not save history during active transaction', () => { + const { result } = renderHook(() => useHistory()); + + act(() => { + result.current.transaction(() => { + // Clear previous calls from transaction setup + mockModelStore.saveToHistory.mockClear(); + mockSceneStore.saveToHistory.mockClear(); + + // Try to save during transaction + result.current.saveToHistory(); + + // Should not have saved + expect(mockModelStore.saveToHistory).not.toHaveBeenCalled(); + expect(mockSceneStore.saveToHistory).not.toHaveBeenCalled(); + }); + }); + }); + }); +}); \ No newline at end of file diff --git a/packages/fossflow-lib/src/hooks/useHistory.ts b/packages/fossflow-lib/src/hooks/useHistory.ts index f2061e2..b9bad61 100644 --- a/packages/fossflow-lib/src/hooks/useHistory.ts +++ b/packages/fossflow-lib/src/hooks/useHistory.ts @@ -1,94 +1,129 @@ -import { useCallback } from 'react'; -import { useModelStoreApi } from 'src/stores/modelStore'; -import { useSceneStoreApi } from 'src/stores/sceneStore'; -import { - useHistoryStore, - useHistoryStoreApi, - extractModelData, - extractSceneData -} from 'src/stores/historyStore'; -import type { HistoryEntry } from 'src/stores/historyStore'; +import { useCallback, useRef } from 'react'; +import { useModelStore } from 'src/stores/modelStore'; +import { useSceneStore } from 'src/stores/sceneStore'; export const useHistory = () => { - const modelStoreApi = useModelStoreApi(); - const sceneStoreApi = useSceneStoreApi(); - const historyStoreApi = useHistoryStoreApi(); + // Track if we're in a transaction to prevent nested history saves + const transactionInProgress = useRef(false); - const canUndo = useHistoryStore((s) => s.past.length > 0); - const canRedo = useHistoryStore((s) => s.future.length > 0); + // Get store actions + const modelActions = useModelStore((state) => { + return state?.actions; + }); + const sceneActions = useSceneStore((state) => { + return state?.actions; + }); - const getCurrentEntry = useCallback((): HistoryEntry => { - const model = modelStoreApi.getState(); - const scene = sceneStoreApi.getState(); - return { - model: extractModelData(model), - scene: extractSceneData(scene) - }; - }, [modelStoreApi, sceneStoreApi]); + // Get history state + const modelCanUndo = useModelStore((state) => { + return state?.actions?.canUndo?.() ?? false; + }); + const sceneCanUndo = useSceneStore((state) => { + return state?.actions?.canUndo?.() ?? false; + }); + const modelCanRedo = useModelStore((state) => { + return state?.actions?.canRedo?.() ?? false; + }); + const sceneCanRedo = useSceneStore((state) => { + return state?.actions?.canRedo?.() ?? false; + }); - const applyEntry = useCallback( - (entry: HistoryEntry) => { - modelStoreApi.getState().actions.set(entry.model); - sceneStoreApi.getState().actions.set(entry.scene); + // Derived values + const canUndo = modelCanUndo || sceneCanUndo; + const canRedo = modelCanRedo || sceneCanRedo; + + // Transaction wrapper - groups multiple operations into single history entry + const transaction = useCallback( + (operations: () => void) => { + if (!modelActions || !sceneActions) return; + + // Prevent nested transactions + if (transactionInProgress.current) { + operations(); + return; + } + + // Save current state before transaction + modelActions.saveToHistory(); + sceneActions.saveToHistory(); + + // Mark transaction as in progress + transactionInProgress.current = true; + + try { + // Execute all operations without saving intermediate history + operations(); + } finally { + // Always reset transaction state + transactionInProgress.current = false; + } + + // Note: We don't save after transaction - the final state is already current }, - [modelStoreApi, sceneStoreApi] + [modelActions, sceneActions] ); - const saveSnapshot = useCallback(() => { - historyStoreApi.getState().actions.saveSnapshot(getCurrentEntry()); - }, [historyStoreApi, getCurrentEntry]); - const undo = useCallback(() => { - const entry = historyStoreApi.getState().actions.undo(getCurrentEntry()); - if (entry) { - applyEntry(entry); - return true; + if (!modelActions || !sceneActions) return false; + + let undoPerformed = false; + + // Try to undo model first, then scene + if (modelActions.canUndo()) { + undoPerformed = modelActions.undo() || undoPerformed; } - return false; - }, [historyStoreApi, getCurrentEntry, applyEntry]); + if (sceneActions.canUndo()) { + undoPerformed = sceneActions.undo() || undoPerformed; + } + + return undoPerformed; + }, [modelActions, sceneActions]); const redo = useCallback(() => { - const entry = historyStoreApi.getState().actions.redo(getCurrentEntry()); - if (entry) { - applyEntry(entry); - return true; + if (!modelActions || !sceneActions) return false; + + let redoPerformed = false; + + // Try to redo model first, then scene + if (modelActions.canRedo()) { + redoPerformed = modelActions.redo() || redoPerformed; } - return false; - }, [historyStoreApi, getCurrentEntry, applyEntry]); + if (sceneActions.canRedo()) { + redoPerformed = sceneActions.redo() || redoPerformed; + } + + return redoPerformed; + }, [modelActions, sceneActions]); + + const saveToHistory = useCallback(() => { + // Don't save during transactions + if (transactionInProgress.current) { + return; + } + + if (!modelActions || !sceneActions) return; + + modelActions.saveToHistory(); + sceneActions.saveToHistory(); + }, [modelActions, sceneActions]); const clearHistory = useCallback(() => { - historyStoreApi.getState().actions.clearHistory(); - }, [historyStoreApi]); + if (!modelActions || !sceneActions) return; - const beginGesture = useCallback(() => { - historyStoreApi.getState().actions.beginGesture(getCurrentEntry()); - }, [historyStoreApi, getCurrentEntry]); - - const endGesture = useCallback(() => { - historyStoreApi.getState().actions.endGesture(); - }, [historyStoreApi]); - - const cancelGesture = useCallback(() => { - const entry = historyStoreApi.getState().actions.cancelGesture(); - if (entry) { - applyEntry(entry); - } - }, [historyStoreApi, applyEntry]); - - const isGestureInProgress = useCallback(() => { - return historyStoreApi.getState().gestureInProgress; - }, [historyStoreApi]); + modelActions.clearHistory(); + sceneActions.clearHistory(); + }, [modelActions, sceneActions]); return { undo, redo, canUndo, canRedo, - saveSnapshot, + saveToHistory, clearHistory, - beginGesture, - endGesture, - cancelGesture, - isGestureInProgress + transaction, + isInTransaction: () => { + return transactionInProgress.current; + } }; }; diff --git a/packages/fossflow-lib/src/hooks/useInitialDataManager.ts b/packages/fossflow-lib/src/hooks/useInitialDataManager.ts index 10eec4d..10d1c61 100644 --- a/packages/fossflow-lib/src/hooks/useInitialDataManager.ts +++ b/packages/fossflow-lib/src/hooks/useInitialDataManager.ts @@ -1,5 +1,4 @@ import { useCallback, useState, useRef } from 'react'; -import { shallow } from 'zustand/shallow'; import { InitialData, IconCollectionState } from 'src/types'; import { INITIAL_DATA, INITIAL_SCENE_STATE } from 'src/config'; import { @@ -10,7 +9,7 @@ import { getItemByIdOrThrow } from 'src/utils'; import * as reducers from 'src/stores/reducers'; -import { useModelStore, useModelStoreApi } from 'src/stores/modelStore'; +import { useModelStore } from 'src/stores/modelStore'; import { useView } from 'src/hooks/useView'; import { useUiStateStore } from 'src/stores/uiStateStore'; import { modelSchema } from 'src/schemas/model'; @@ -18,12 +17,9 @@ import { modelSchema } from 'src/schemas/model'; export const useInitialDataManager = () => { const [isReady, setIsReady] = useState(false); const prevInitialData = useRef(undefined); - const modelActions = useModelStore((state) => state.actions); - const { icons, colors } = useModelStore( - (state) => ({ icons: state.icons, colors: state.colors }), - shallow - ); - const modelStoreApi = useModelStoreApi(); + const model = useModelStore((state) => { + return state; + }); const uiStateActions = useUiStateStore((state) => { return state.actions; }); @@ -109,7 +105,7 @@ export const useInitialDataManager = () => { } prevInitialData.current = initialData; - modelActions.set(initialData); + model.actions.set(initialData); const view = getItemByIdOrThrow( initialData.views, @@ -147,13 +143,13 @@ export const useInitialDataManager = () => { setIsReady(true); }, - [changeView, modelActions, rendererEl, uiStateActions, editorMode] + [changeView, model.actions, rendererEl, uiStateActions, editorMode] ); const clear = useCallback(() => { - load({ ...INITIAL_DATA, icons, colors }); + load({ ...INITIAL_DATA, icons: model.icons, colors: model.colors }); uiStateActions.resetUiState(); - }, [load, icons, colors, uiStateActions]); + }, [load, model.icons, model.colors, uiStateActions]); return { load, diff --git a/packages/fossflow-lib/src/hooks/useScene.ts b/packages/fossflow-lib/src/hooks/useScene.ts index a9a3807..49f2887 100644 --- a/packages/fossflow-lib/src/hooks/useScene.ts +++ b/packages/fossflow-lib/src/hooks/useScene.ts @@ -1,4 +1,4 @@ -import { useCallback, useMemo } from 'react'; +import { useCallback, useMemo, useRef } from 'react'; import { shallow } from 'zustand/shallow'; import { ModelItem, @@ -18,7 +18,6 @@ import { RECTANGLE_DEFAULTS, TEXTBOX_DEFAULTS } from 'src/config'; -import { useHistory } from 'src/hooks/useHistory'; export const useScene = () => { const { views, colors, icons, items, version, title, description } = @@ -43,10 +42,10 @@ export const useScene = () => { shallow ); const currentViewId = useUiStateStore((state) => state.view); + const transactionInProgress = useRef(false); const modelStoreApi = useModelStoreApi(); const sceneStoreApi = useSceneStoreApi(); - const { saveSnapshot, beginGesture, endGesture } = useHistory(); const currentView = useMemo(() => { if (!views || !currentViewId) { @@ -139,45 +138,59 @@ export const useScene = () => { const setState = useCallback( (newState: State) => { - modelStoreApi.getState().actions.set(newState.model); - sceneStoreApi.getState().actions.set(newState.scene); + modelStoreApi.getState().actions.set(newState.model, true); + sceneStoreApi.getState().actions.set(newState.scene, true); }, [modelStoreApi, sceneStoreApi] ); + const saveToHistoryBeforeChange = useCallback(() => { + if (transactionInProgress.current) { + return; + } + + modelStoreApi.getState().actions.saveToHistory(); + sceneStoreApi.getState().actions.saveToHistory(); + }, [modelStoreApi, sceneStoreApi]); + const createModelItem = useCallback( (newModelItem: ModelItem) => { - saveSnapshot(); + if (!transactionInProgress.current) { + saveToHistoryBeforeChange(); + } + const newState = reducers.createModelItem(newModelItem, getState()); setState(newState); return newState; }, - [getState, setState, saveSnapshot] + [getState, setState, saveToHistoryBeforeChange] ); const updateModelItem = useCallback( (id: string, updates: Partial) => { - saveSnapshot(); + saveToHistoryBeforeChange(); const newState = reducers.updateModelItem(id, updates, getState()); setState(newState); }, - [getState, setState, saveSnapshot] + [getState, setState, saveToHistoryBeforeChange] ); const deleteModelItem = useCallback( (id: string) => { - saveSnapshot(); + saveToHistoryBeforeChange(); const newState = reducers.deleteModelItem(id, getState()); setState(newState); }, - [getState, setState, saveSnapshot] + [getState, setState, saveToHistoryBeforeChange] ); const createViewItem = useCallback( (newViewItem: ViewItem, currentState?: State) => { if (!currentViewId) return; - saveSnapshot(); + if (!transactionInProgress.current) { + saveToHistoryBeforeChange(); + } const stateToUse = currentState || getState(); @@ -189,14 +202,16 @@ export const useScene = () => { setState(newState); return newState; }, - [getState, setState, currentViewId, saveSnapshot] + [getState, setState, currentViewId, saveToHistoryBeforeChange] ); const updateViewItem = useCallback( (id: string, updates: Partial, currentState?: State) => { if (!currentViewId) return getState(); - saveSnapshot(); + if (!transactionInProgress.current) { + saveToHistoryBeforeChange(); + } const stateToUse = currentState || getState(); const newState = reducers.view({ @@ -207,14 +222,14 @@ export const useScene = () => { setState(newState); return newState; }, - [getState, setState, currentViewId, saveSnapshot] + [getState, setState, currentViewId, saveToHistoryBeforeChange] ); const deleteViewItem = useCallback( (id: string) => { if (!currentViewId) return; - saveSnapshot(); + saveToHistoryBeforeChange(); const newState = reducers.view({ action: 'DELETE_VIEWITEM', payload: id, @@ -222,14 +237,14 @@ export const useScene = () => { }); setState(newState); }, - [getState, setState, currentViewId, saveSnapshot] + [getState, setState, currentViewId, saveToHistoryBeforeChange] ); const createConnector = useCallback( (newConnector: Connector) => { if (!currentViewId) return; - saveSnapshot(); + saveToHistoryBeforeChange(); const newState = reducers.view({ action: 'CREATE_CONNECTOR', payload: newConnector, @@ -237,14 +252,14 @@ export const useScene = () => { }); setState(newState); }, - [getState, setState, currentViewId, saveSnapshot] + [getState, setState, currentViewId, saveToHistoryBeforeChange] ); const updateConnector = useCallback( (id: string, updates: Partial) => { if (!currentViewId) return; - saveSnapshot(); + saveToHistoryBeforeChange(); const newState = reducers.view({ action: 'UPDATE_CONNECTOR', payload: { id, ...updates }, @@ -252,14 +267,14 @@ export const useScene = () => { }); setState(newState); }, - [getState, setState, currentViewId, saveSnapshot] + [getState, setState, currentViewId, saveToHistoryBeforeChange] ); const deleteConnector = useCallback( (id: string) => { if (!currentViewId) return; - saveSnapshot(); + saveToHistoryBeforeChange(); const newState = reducers.view({ action: 'DELETE_CONNECTOR', payload: id, @@ -267,14 +282,14 @@ export const useScene = () => { }); setState(newState); }, - [getState, setState, currentViewId, saveSnapshot] + [getState, setState, currentViewId, saveToHistoryBeforeChange] ); const createTextBox = useCallback( (newTextBox: TextBox) => { if (!currentViewId) return; - saveSnapshot(); + saveToHistoryBeforeChange(); const newState = reducers.view({ action: 'CREATE_TEXTBOX', payload: newTextBox, @@ -282,14 +297,16 @@ export const useScene = () => { }); setState(newState); }, - [getState, setState, currentViewId, saveSnapshot] + [getState, setState, currentViewId, saveToHistoryBeforeChange] ); const updateTextBox = useCallback( (id: string, updates: Partial, currentState?: State) => { if (!currentViewId) return currentState || getState(); - saveSnapshot(); + if (!transactionInProgress.current) { + saveToHistoryBeforeChange(); + } const stateToUse = currentState || getState(); const newState = reducers.view({ @@ -300,14 +317,14 @@ export const useScene = () => { setState(newState); return newState; }, - [getState, setState, currentViewId, saveSnapshot] + [getState, setState, currentViewId, saveToHistoryBeforeChange] ); const deleteTextBox = useCallback( (id: string) => { if (!currentViewId) return; - saveSnapshot(); + saveToHistoryBeforeChange(); const newState = reducers.view({ action: 'DELETE_TEXTBOX', payload: id, @@ -315,14 +332,14 @@ export const useScene = () => { }); setState(newState); }, - [getState, setState, currentViewId, saveSnapshot] + [getState, setState, currentViewId, saveToHistoryBeforeChange] ); const createRectangle = useCallback( (newRectangle: Rectangle) => { if (!currentViewId) return; - saveSnapshot(); + saveToHistoryBeforeChange(); const newState = reducers.view({ action: 'CREATE_RECTANGLE', payload: newRectangle, @@ -330,14 +347,16 @@ export const useScene = () => { }); setState(newState); }, - [getState, setState, currentViewId, saveSnapshot] + [getState, setState, currentViewId, saveToHistoryBeforeChange] ); const updateRectangle = useCallback( (id: string, updates: Partial, currentState?: State) => { if (!currentViewId) return currentState || getState(); - saveSnapshot(); + if (!transactionInProgress.current) { + saveToHistoryBeforeChange(); + } const stateToUse = currentState || getState(); const newState = reducers.view({ @@ -348,14 +367,14 @@ export const useScene = () => { setState(newState); return newState; }, - [getState, setState, currentViewId, saveSnapshot] + [getState, setState, currentViewId, saveToHistoryBeforeChange] ); const deleteRectangle = useCallback( (id: string) => { if (!currentViewId) return; - saveSnapshot(); + saveToHistoryBeforeChange(); const newState = reducers.view({ action: 'DELETE_RECTANGLE', payload: id, @@ -363,24 +382,32 @@ export const useScene = () => { }); setState(newState); }, - [getState, setState, currentViewId, saveSnapshot] + [getState, setState, currentViewId, saveToHistoryBeforeChange] ); const transaction = useCallback( (operations: () => void) => { - beginGesture(); + if (transactionInProgress.current) { + operations(); + return; + } + + saveToHistoryBeforeChange(); + transactionInProgress.current = true; + try { operations(); } finally { - endGesture(); + transactionInProgress.current = false; } }, - [beginGesture, endGesture] + [saveToHistoryBeforeChange] ); const placeIcon = useCallback( (params: { modelItem: ModelItem; viewItem: ViewItem }) => { - beginGesture(); + saveToHistoryBeforeChange(); + transactionInProgress.current = true; try { const stateAfterModelItem = createModelItem(params.modelItem); @@ -389,10 +416,10 @@ export const useScene = () => { createViewItem(params.viewItem, stateAfterModelItem); } } finally { - endGesture(); + transactionInProgress.current = false; } }, - [createModelItem, createViewItem, beginGesture, endGesture] + [createModelItem, createViewItem, saveToHistoryBeforeChange] ); return { diff --git a/packages/fossflow-lib/src/hooks/useView.ts b/packages/fossflow-lib/src/hooks/useView.ts index b175b69..87fa6ed 100644 --- a/packages/fossflow-lib/src/hooks/useView.ts +++ b/packages/fossflow-lib/src/hooks/useView.ts @@ -4,7 +4,6 @@ import { useSceneStore } from 'src/stores/sceneStore'; import * as reducers from 'src/stores/reducers'; import { Model } from 'src/types'; import { INITIAL_SCENE_STATE } from 'src/config'; -import { useHistory } from 'src/hooks/useHistory'; export const useView = () => { const uiStateActions = useUiStateStore((state) => { @@ -15,8 +14,6 @@ export const useView = () => { return state.actions; }); - const { clearHistory } = useHistory(); - const changeView = useCallback( (viewId: string, model: Model) => { const newState = reducers.view({ @@ -27,9 +24,8 @@ export const useView = () => { sceneActions.set(newState.scene); uiStateActions.setView(viewId); - clearHistory(); }, - [uiStateActions, sceneActions, clearHistory] + [uiStateActions, sceneActions] ); return { diff --git a/packages/fossflow-lib/src/interaction/modes/Connector.ts b/packages/fossflow-lib/src/interaction/modes/Connector.ts index b0ec770..7e11816 100644 --- a/packages/fossflow-lib/src/interaction/modes/Connector.ts +++ b/packages/fossflow-lib/src/interaction/modes/Connector.ts @@ -61,7 +61,7 @@ export const Connector: ModeActions = { } } }, - mousedown: ({ uiState, scene, isRendererInteraction, history }) => { + mousedown: ({ uiState, scene, isRendererInteraction }) => { if (uiState.mode.type !== 'CONNECTOR' || !isRendererInteraction) return; const itemAtTile = getItemAtTile({ @@ -72,9 +72,8 @@ export const Connector: ModeActions = { if (uiState.connectorInteractionMode === 'click') { // Click mode: handle first and second clicks if (!uiState.mode.startAnchor) { - // First click: begin gesture and store the start position - history.beginGesture(); - const startAnchor = itemAtTile?.type === 'ITEM' + // First click: store the start position + const startAnchor = itemAtTile?.type === 'ITEM' ? { itemId: itemAtTile.id } : { tile: uiState.mouse.position.tile }; @@ -145,8 +144,7 @@ export const Connector: ModeActions = { // Don't delete connectors to empty space - they're valid // Only validate minimum path length will be handled by the update - // End gesture and reset for next connection - history.endGesture(); + // Reset for next connection uiState.actions.setMode({ type: 'CONNECTOR', showCursor: true, @@ -157,8 +155,7 @@ export const Connector: ModeActions = { } } } else { - // Drag mode: begin gesture before creating - history.beginGesture(); + // Drag mode: original behavior const newConnector: ConnectorI = { id: generateId(), color: scene.colors[0].id, @@ -186,7 +183,7 @@ export const Connector: ModeActions = { }); } }, - mouseup: ({ uiState, scene, history }) => { + mouseup: ({ uiState, scene }) => { if (uiState.mode.type !== 'CONNECTOR' || !uiState.mode.id) return; // Only handle mouseup for drag mode @@ -194,7 +191,6 @@ export const Connector: ModeActions = { // Don't delete connectors to empty space - they're valid // Validation is handled in the reducer layer - history.endGesture(); uiState.actions.setMode({ type: 'CONNECTOR', showCursor: true, diff --git a/packages/fossflow-lib/src/interaction/modes/DragItems.ts b/packages/fossflow-lib/src/interaction/modes/DragItems.ts index 419c653..fcd5af1 100644 --- a/packages/fossflow-lib/src/interaction/modes/DragItems.ts +++ b/packages/fossflow-lib/src/interaction/modes/DragItems.ts @@ -50,36 +50,40 @@ const dragItems = ( const hasUpdates = newTiles || textBoxRefs.length > 0 || rectangleRefs.length > 0; if (hasUpdates) { - let currentState: State | undefined; + // Wrap ALL updates in a single transaction with state chaining + // This ensures each update builds on the previous one's state + scene.transaction(() => { + let currentState: State | undefined; - // 1. Update nodes - if (newTiles) { - itemRefs.forEach((item, index) => { - currentState = scene.updateViewItem(item.id, { - tile: newTiles[index] + // 1. Update nodes + if (newTiles) { + itemRefs.forEach((item, index) => { + currentState = scene.updateViewItem(item.id, { + tile: newTiles[index] + }, currentState); + }); + } + + // 2. Update textboxes (chained from node state) + textBoxRefs.forEach((item) => { + const textBox = getItemByIdOrThrow(scene.textBoxes, item.id).value; + currentState = scene.updateTextBox(item.id, { + tile: CoordsUtils.add(textBox.tile, delta) }, currentState); }); - } - // 2. Update textboxes (chained from node state) - textBoxRefs.forEach((item) => { - const textBox = getItemByIdOrThrow(scene.textBoxes, item.id).value; - currentState = scene.updateTextBox(item.id, { - tile: CoordsUtils.add(textBox.tile, delta) - }, currentState); - }); - - // 3. Update rectangles (chained from textbox state) - rectangleRefs.forEach((item) => { - const rectangle = getItemByIdOrThrow(scene.rectangles, item.id).value; - currentState = scene.updateRectangle(item.id, { - from: CoordsUtils.add(rectangle.from, delta), - to: CoordsUtils.add(rectangle.to, delta) - }, currentState); + // 3. Update rectangles (chained from textbox state) + rectangleRefs.forEach((item) => { + const rectangle = getItemByIdOrThrow(scene.rectangles, item.id).value; + currentState = scene.updateRectangle(item.id, { + from: CoordsUtils.add(rectangle.from, delta), + to: CoordsUtils.add(rectangle.to, delta) + }, currentState); + }); }); } - // Handle connector anchors (gesture already in progress from entry) + // Handle connector anchors separately (they have different update logic) anchorRefs.forEach((item) => { const connector = getAnchorParent(item.id, scene.connectors); @@ -121,15 +125,13 @@ const dragItems = ( }; export const DragItems: ModeActions = { - entry: ({ uiState, rendererRef, history }) => { + entry: ({ uiState, rendererRef }) => { if (uiState.mode.type !== 'DRAG_ITEMS' || !uiState.mouse.mousedown) return; - history.beginGesture(); const renderer = rendererRef; renderer.style.userSelect = 'none'; }, - exit: ({ rendererRef, history }) => { - history.endGesture(); + exit: ({ rendererRef }) => { const renderer = rendererRef; renderer.style.userSelect = 'auto'; }, diff --git a/packages/fossflow-lib/src/interaction/modes/FreehandLasso.ts b/packages/fossflow-lib/src/interaction/modes/FreehandLasso.ts index f765f95..e5ddda7 100644 --- a/packages/fossflow-lib/src/interaction/modes/FreehandLasso.ts +++ b/packages/fossflow-lib/src/interaction/modes/FreehandLasso.ts @@ -1,25 +1,25 @@ import { produce } from 'immer'; -import { ModeActions, ItemReference, Coords, SceneItemsSnapshot } from 'src/types'; +import { ModeActions, ItemReference, Coords } from 'src/types'; import { screenToIso, isPointInPolygon } from 'src/utils'; // Helper to find all items whose centers are within the freehand polygon const getItemsInFreehandBounds = ( pathTiles: Coords[], - scene: SceneItemsSnapshot + scene: any ): ItemReference[] => { const items: ItemReference[] = []; if (pathTiles.length < 3) return items; // Check all nodes/items - scene.items.forEach((item) => { + scene.items.forEach((item: any) => { if (isPointInPolygon(item.tile, pathTiles)) { items.push({ type: 'ITEM', id: item.id }); } }); // Check all rectangles - they must be FULLY enclosed (all 4 corners inside) - scene.rectangles.forEach((rectangle) => { + scene.rectangles.forEach((rectangle: any) => { const corners = [ rectangle.from, { x: rectangle.to.x, y: rectangle.from.y }, @@ -36,7 +36,7 @@ const getItemsInFreehandBounds = ( }); // Check all text boxes - scene.textBoxes.forEach((textBox) => { + scene.textBoxes.forEach((textBox: any) => { if (isPointInPolygon(textBox.tile, pathTiles)) { items.push({ type: 'TEXTBOX', id: textBox.id }); } diff --git a/packages/fossflow-lib/src/interaction/modes/Lasso.ts b/packages/fossflow-lib/src/interaction/modes/Lasso.ts index 127ac5f..bf7b0db 100644 --- a/packages/fossflow-lib/src/interaction/modes/Lasso.ts +++ b/packages/fossflow-lib/src/interaction/modes/Lasso.ts @@ -1,24 +1,24 @@ import { produce } from 'immer'; -import { ModeActions, ItemReference, Coords, SceneItemsSnapshot } from 'src/types'; -import { isWithinBounds, hasMovedTile } from 'src/utils'; +import { ModeActions, ItemReference } from 'src/types'; +import { CoordsUtils, isWithinBounds, hasMovedTile } from 'src/utils'; // Helper to find all items within the lasso bounds const getItemsInBounds = ( - startTile: Coords, - endTile: Coords, - scene: SceneItemsSnapshot + startTile: { x: number; y: number }, + endTile: { x: number; y: number }, + scene: any ): ItemReference[] => { const items: ItemReference[] = []; // Check all nodes/items - scene.items.forEach((item) => { + scene.items.forEach((item: any) => { if (isWithinBounds(item.tile, [startTile, endTile])) { items.push({ type: 'ITEM', id: item.id }); } }); // Check all rectangles - they must be FULLY enclosed (all 4 corners inside) - scene.rectangles.forEach((rectangle) => { + scene.rectangles.forEach((rectangle: any) => { const corners = [ rectangle.from, { x: rectangle.to.x, y: rectangle.from.y }, @@ -37,7 +37,7 @@ const getItemsInBounds = ( }); // Check all text boxes - scene.textBoxes.forEach((textBox) => { + scene.textBoxes.forEach((textBox: any) => { if (isWithinBounds(textBox.tile, [startTile, endTile])) { items.push({ type: 'TEXTBOX', id: textBox.id }); } diff --git a/packages/fossflow-lib/src/interaction/modes/Rectangle/DrawRectangle.ts b/packages/fossflow-lib/src/interaction/modes/Rectangle/DrawRectangle.ts index 97d6b35..8f3ec2f 100644 --- a/packages/fossflow-lib/src/interaction/modes/Rectangle/DrawRectangle.ts +++ b/packages/fossflow-lib/src/interaction/modes/Rectangle/DrawRectangle.ts @@ -22,11 +22,10 @@ export const DrawRectangle: ModeActions = { to: uiState.mouse.position.tile }); }, - mousedown: ({ uiState, scene, isRendererInteraction, history }) => { + mousedown: ({ uiState, scene, isRendererInteraction }) => { if (uiState.mode.type !== 'RECTANGLE.DRAW' || !isRendererInteraction) return; - history.beginGesture(); const newRectangleId = generateId(); scene.createRectangle({ @@ -42,10 +41,9 @@ export const DrawRectangle: ModeActions = { uiState.actions.setMode(newMode); }, - mouseup: ({ uiState, history }) => { + mouseup: ({ uiState }) => { if (uiState.mode.type !== 'RECTANGLE.DRAW' || !uiState.mode.id) return; - history.endGesture(); uiState.actions.setMode({ type: 'CURSOR', showCursor: true, diff --git a/packages/fossflow-lib/src/interaction/modes/Rectangle/TransformRectangle.ts b/packages/fossflow-lib/src/interaction/modes/Rectangle/TransformRectangle.ts index a485e57..e487099 100644 --- a/packages/fossflow-lib/src/interaction/modes/Rectangle/TransformRectangle.ts +++ b/packages/fossflow-lib/src/interaction/modes/Rectangle/TransformRectangle.ts @@ -7,9 +7,7 @@ import { import { ModeActions } from 'src/types'; export const TransformRectangle: ModeActions = { - entry: ({ history }) => { - history.beginGesture(); - }, + entry: () => {}, exit: () => {}, mousemove: ({ uiState, scene }) => { if ( @@ -65,10 +63,9 @@ export const TransformRectangle: ModeActions = { mousedown: () => { // MOUSE_DOWN is triggered by the anchor iteself (see `TransformAnchor.tsx`) }, - mouseup: ({ uiState, history }) => { + mouseup: ({ uiState }) => { if (uiState.mode.type !== 'RECTANGLE.TRANSFORM') return; - history.endGesture(); uiState.actions.setMode({ type: 'CURSOR', mousedownItem: null, diff --git a/packages/fossflow-lib/src/interaction/modes/TextBox.ts b/packages/fossflow-lib/src/interaction/modes/TextBox.ts index 29c2504..6cb430e 100644 --- a/packages/fossflow-lib/src/interaction/modes/TextBox.ts +++ b/packages/fossflow-lib/src/interaction/modes/TextBox.ts @@ -15,13 +15,12 @@ export const TextBox: ModeActions = { tile: uiState.mouse.position.tile }); }, - mouseup: ({ uiState, scene, isRendererInteraction, history }) => { + mouseup: ({ uiState, scene, isRendererInteraction }) => { if (uiState.mode.type !== 'TEXTBOX' || !uiState.mode.id) return; if (!isRendererInteraction) { - history.cancelGesture(); + scene.deleteTextBox(uiState.mode.id); } else { - history.endGesture(); uiState.actions.setItemControls({ type: 'TEXTBOX', id: uiState.mode.id diff --git a/packages/fossflow-lib/src/interaction/useInteractionManager.ts b/packages/fossflow-lib/src/interaction/useInteractionManager.ts index 5f8bffb..5330b96 100644 --- a/packages/fossflow-lib/src/interaction/useInteractionManager.ts +++ b/packages/fossflow-lib/src/interaction/useInteractionManager.ts @@ -106,7 +106,7 @@ export const useInteractionManager = () => { const modelStoreApi = useModelStoreApi(); const scene = useScene(); const { size: rendererSize } = useResizeObserver(rendererEl); - const { undo, redo, canUndo, canRedo, beginGesture, endGesture, cancelGesture, isGestureInProgress } = useHistory(); + const { undo, redo, canUndo, canRedo } = useHistory(); const { createTextBox } = scene; const { handleMouseDown: handlePanMouseDown, handleMouseUp: handlePanMouseUp } = usePanHandlers(); const { scheduleUpdate, flushUpdate, cleanup } = useRAFThrottle(); @@ -131,7 +131,7 @@ export const useInteractionManager = () => { (uiState.connectorInteractionMode === 'drag' && connectorMode.id !== null); if (isConnectionInProgress && connectorMode.id) { - cancelGesture(); + scene.deleteConnector(connectorMode.id); uiState.actions.setMode({ type: 'CONNECTOR', @@ -158,13 +158,9 @@ export const useInteractionManager = () => { const isCtrlOrCmd = e.ctrlKey || e.metaKey; - // Block undo/redo during active interactions or gestures - const blockingModes = ['CONNECTOR', 'RECTANGLE.DRAW', 'RECTANGLE.TRANSFORM', 'TEXTBOX', 'DRAG_ITEMS']; - const isUndoBlocked = blockingModes.includes(uiState.mode.type) || isGestureInProgress(); - if (isCtrlOrCmd && e.key.toLowerCase() === 'z' && !e.shiftKey) { e.preventDefault(); - if (canUndo && !isUndoBlocked) { + if (canUndo) { undo(); } } @@ -175,7 +171,7 @@ export const useInteractionManager = () => { (e.key.toLowerCase() === 'z' && e.shiftKey)) ) { e.preventDefault(); - if (canRedo && !isUndoBlocked) { + if (canRedo) { redo(); } } @@ -234,7 +230,6 @@ export const useInteractionManager = () => { }); } else if (hotkeyMapping.text && key === hotkeyMapping.text) { e.preventDefault(); - beginGesture(); const textBoxId = generateId(); createTextBox({ ...TEXTBOX_DEFAULTS, @@ -270,7 +265,7 @@ export const useInteractionManager = () => { return () => { return window.removeEventListener('keydown', handleKeyDown); }; - }, [undo, redo, canUndo, canRedo, uiStateApi, createTextBox, scene, beginGesture, cancelGesture, isGestureInProgress]); + }, [undo, redo, canUndo, canRedo, uiStateApi, createTextBox, scene]); const processMouseUpdate = useCallback( (nextMouse: Mouse, e: SlimMouseEvent) => { @@ -289,7 +284,6 @@ export const useInteractionManager = () => { const baseState: State = { model, scene, - history: { beginGesture, endGesture, cancelGesture, isGestureInProgress }, uiState, rendererRef: rendererRef.current, rendererSize, @@ -313,7 +307,7 @@ export const useInteractionManager = () => { modeFunction(baseState); reducerTypeRef.current = uiState.mode.type; }, - [uiStateApi, modelStoreApi, scene, rendererSize, beginGesture, endGesture, cancelGesture, isGestureInProgress] + [uiStateApi, modelStoreApi, scene, rendererSize] ); const onMouseEvent = useCallback( diff --git a/packages/fossflow-lib/src/stores/__tests__/historyStore.test.tsx b/packages/fossflow-lib/src/stores/__tests__/historyStore.test.tsx deleted file mode 100644 index 645c16d..0000000 --- a/packages/fossflow-lib/src/stores/__tests__/historyStore.test.tsx +++ /dev/null @@ -1,361 +0,0 @@ -import React from 'react'; -import { renderHook, act } from '@testing-library/react'; -import { - HistoryProvider, - useHistoryStore, - useHistoryStoreApi -} from '../historyStore'; -import type { HistoryEntry } from '../historyStore'; - -const makeEntry = (id: number): HistoryEntry => ({ - model: { - version: `v${id}`, - title: `Model ${id}`, - colors: [], - icons: [], - items: [], - views: [] - }, - scene: { - connectors: {}, - textBoxes: {} - } -}); - -const wrapper = ({ children }: { children: React.ReactNode }) => ( - {children} -); - -describe('historyStore', () => { - describe('saveSnapshot', () => { - it('adds entry to past and clears future', () => { - const { result } = renderHook( - () => ({ - api: useHistoryStoreApi(), - past: useHistoryStore((s) => s.past), - future: useHistoryStore((s) => s.future) - }), - { wrapper } - ); - - act(() => { - result.current.api.getState().actions.saveSnapshot(makeEntry(1)); - }); - - expect(result.current.past).toHaveLength(1); - expect(result.current.past[0].model.title).toBe('Model 1'); - expect(result.current.future).toHaveLength(0); - }); - - it('enforces max size', () => { - const { result } = renderHook( - () => ({ - api: useHistoryStoreApi(), - past: useHistoryStore((s) => s.past) - }), - { wrapper } - ); - - act(() => { - for (let i = 0; i < 55; i++) { - result.current.api.getState().actions.saveSnapshot(makeEntry(i)); - } - }); - - expect(result.current.past).toHaveLength(50); - // Oldest entries should have been shifted off - expect(result.current.past[0].model.title).toBe('Model 5'); - }); - - it('skips save during gesture', () => { - const { result } = renderHook( - () => ({ - api: useHistoryStoreApi(), - past: useHistoryStore((s) => s.past) - }), - { wrapper } - ); - - act(() => { - result.current.api.getState().actions.beginGesture(makeEntry(0)); - }); - - act(() => { - result.current.api.getState().actions.saveSnapshot(makeEntry(1)); - }); - - // Only the beginGesture entry should be there - expect(result.current.past).toHaveLength(1); - expect(result.current.past[0].model.title).toBe('Model 0'); - }); - }); - - describe('undo', () => { - it('returns previous entry and moves current to future', () => { - const { result } = renderHook( - () => ({ - api: useHistoryStoreApi(), - past: useHistoryStore((s) => s.past), - future: useHistoryStore((s) => s.future) - }), - { wrapper } - ); - - act(() => { - result.current.api.getState().actions.saveSnapshot(makeEntry(1)); - }); - - let undone: HistoryEntry | null = null; - act(() => { - undone = result.current.api.getState().actions.undo(makeEntry(2)); - }); - - expect(undone).not.toBeNull(); - expect(undone!.model.title).toBe('Model 1'); - expect(result.current.past).toHaveLength(0); - expect(result.current.future).toHaveLength(1); - expect(result.current.future[0].model.title).toBe('Model 2'); - }); - - it('returns null when past is empty', () => { - const { result } = renderHook( - () => ({ - api: useHistoryStoreApi() - }), - { wrapper } - ); - - let undone: HistoryEntry | null = null; - act(() => { - undone = result.current.api.getState().actions.undo(makeEntry(1)); - }); - - expect(undone).toBeNull(); - }); - }); - - describe('redo', () => { - it('returns next entry and pushes current to past', () => { - const { result } = renderHook( - () => ({ - api: useHistoryStoreApi(), - past: useHistoryStore((s) => s.past), - future: useHistoryStore((s) => s.future) - }), - { wrapper } - ); - - // Build up: save then undo - act(() => { - result.current.api.getState().actions.saveSnapshot(makeEntry(1)); - }); - act(() => { - result.current.api.getState().actions.undo(makeEntry(2)); - }); - - // Now redo, passing current state (makeEntry(1) since undo restored it) - let redone: HistoryEntry | null = null; - act(() => { - redone = result.current.api.getState().actions.redo(makeEntry(1)); - }); - - expect(redone).not.toBeNull(); - expect(redone!.model.title).toBe('Model 2'); - expect(result.current.future).toHaveLength(0); - // past should contain the current state we passed, not the redo'd entry - expect(result.current.past).toHaveLength(1); - expect(result.current.past[0].model.title).toBe('Model 1'); - }); - - it('returns null when future is empty', () => { - const { result } = renderHook( - () => ({ - api: useHistoryStoreApi() - }), - { wrapper } - ); - - let redone: HistoryEntry | null = null; - act(() => { - redone = result.current.api.getState().actions.redo(makeEntry(99)); - }); - - expect(redone).toBeNull(); - }); - - it('undo+redo+undo cycle preserves correct entries', () => { - const { result } = renderHook( - () => ({ - api: useHistoryStoreApi(), - past: useHistoryStore((s) => s.past), - future: useHistoryStore((s) => s.future) - }), - { wrapper } - ); - - // State A is "current", save snapshot and change to B - act(() => { - result.current.api.getState().actions.saveSnapshot(makeEntry(1)); // past=[A] - }); - - // Undo: current is B(=2), want to go back to A(=1) - let undone: HistoryEntry | null = null; - act(() => { - undone = result.current.api.getState().actions.undo(makeEntry(2)); - }); - expect(undone!.model.title).toBe('Model 1'); - // past=[], future=[B(2)] - - // Redo: current is A(=1), want to go forward to B(=2) - let redone: HistoryEntry | null = null; - act(() => { - redone = result.current.api.getState().actions.redo(makeEntry(1)); - }); - expect(redone!.model.title).toBe('Model 2'); - // past=[A(1)], future=[] - - // Undo again: current is B(=2), should go back to A(=1) - act(() => { - undone = result.current.api.getState().actions.undo(makeEntry(2)); - }); - expect(undone!.model.title).toBe('Model 1'); - // past=[], future=[B(2)] - - expect(result.current.past).toHaveLength(0); - expect(result.current.future).toHaveLength(1); - expect(result.current.future[0].model.title).toBe('Model 2'); - }); - }); - - describe('gesture lifecycle', () => { - it('beginGesture saves state and sets flag', () => { - const { result } = renderHook( - () => ({ - api: useHistoryStoreApi(), - gestureInProgress: useHistoryStore((s) => s.gestureInProgress), - past: useHistoryStore((s) => s.past) - }), - { wrapper } - ); - - expect(result.current.gestureInProgress).toBe(false); - - act(() => { - result.current.api.getState().actions.beginGesture(makeEntry(1)); - }); - - expect(result.current.gestureInProgress).toBe(true); - expect(result.current.past).toHaveLength(1); - }); - - it('endGesture clears flag', () => { - const { result } = renderHook( - () => ({ - api: useHistoryStoreApi(), - gestureInProgress: useHistoryStore((s) => s.gestureInProgress) - }), - { wrapper } - ); - - act(() => { - result.current.api.getState().actions.beginGesture(makeEntry(1)); - }); - act(() => { - result.current.api.getState().actions.endGesture(); - }); - - expect(result.current.gestureInProgress).toBe(false); - }); - - it('cancelGesture pops past entry and clears flag', () => { - const { result } = renderHook( - () => ({ - api: useHistoryStoreApi(), - gestureInProgress: useHistoryStore((s) => s.gestureInProgress), - past: useHistoryStore((s) => s.past) - }), - { wrapper } - ); - - act(() => { - result.current.api.getState().actions.beginGesture(makeEntry(1)); - }); - - expect(result.current.past).toHaveLength(1); - - let cancelled: HistoryEntry | null = null; - act(() => { - cancelled = result.current.api.getState().actions.cancelGesture(); - }); - - expect(cancelled).not.toBeNull(); - expect(cancelled!.model.title).toBe('Model 1'); - expect(result.current.past).toHaveLength(0); - expect(result.current.gestureInProgress).toBe(false); - }); - - it('cancelGesture returns null when no gesture in progress', () => { - const { result } = renderHook( - () => ({ - api: useHistoryStoreApi() - }), - { wrapper } - ); - - let cancelled: HistoryEntry | null = null; - act(() => { - cancelled = result.current.api.getState().actions.cancelGesture(); - }); - - expect(cancelled).toBeNull(); - }); - - it('beginGesture is idempotent when already in gesture', () => { - const { result } = renderHook( - () => ({ - api: useHistoryStoreApi(), - past: useHistoryStore((s) => s.past) - }), - { wrapper } - ); - - act(() => { - result.current.api.getState().actions.beginGesture(makeEntry(1)); - }); - act(() => { - result.current.api.getState().actions.beginGesture(makeEntry(2)); - }); - - // Should only have one entry (second beginGesture was a no-op) - expect(result.current.past).toHaveLength(1); - expect(result.current.past[0].model.title).toBe('Model 1'); - }); - }); - - describe('clearHistory', () => { - it('resets past, future, and gestureInProgress', () => { - const { result } = renderHook( - () => ({ - api: useHistoryStoreApi(), - past: useHistoryStore((s) => s.past), - future: useHistoryStore((s) => s.future), - gestureInProgress: useHistoryStore((s) => s.gestureInProgress) - }), - { wrapper } - ); - - act(() => { - result.current.api.getState().actions.saveSnapshot(makeEntry(1)); - result.current.api.getState().actions.saveSnapshot(makeEntry(2)); - }); - - act(() => { - result.current.api.getState().actions.clearHistory(); - }); - - expect(result.current.past).toHaveLength(0); - expect(result.current.future).toHaveLength(0); - expect(result.current.gestureInProgress).toBe(false); - }); - }); -}); diff --git a/packages/fossflow-lib/src/stores/historyStore.tsx b/packages/fossflow-lib/src/stores/historyStore.tsx deleted file mode 100644 index 258c06d..0000000 --- a/packages/fossflow-lib/src/stores/historyStore.tsx +++ /dev/null @@ -1,211 +0,0 @@ -import React, { createContext, useRef, useContext } from 'react'; -import { createStore, useStore } from 'zustand'; -import { Model, Scene } from 'src/types'; - -export interface HistoryEntry { - model: Model; - scene: Scene; -} - -export interface HistoryStoreState { - past: HistoryEntry[]; - future: HistoryEntry[]; - gestureInProgress: boolean; - maxSize: number; - actions: { - saveSnapshot: (currentEntry: HistoryEntry) => void; - undo: (currentEntry: HistoryEntry) => HistoryEntry | null; - redo: (currentEntry: HistoryEntry) => HistoryEntry | null; - clearHistory: () => void; - beginGesture: (currentEntry: HistoryEntry) => void; - endGesture: () => void; - cancelGesture: () => HistoryEntry | null; - }; -} - -const MAX_HISTORY_SIZE = 50; - -const createHistoryStore = () => { - return createStore((set, get) => ({ - past: [], - future: [], - gestureInProgress: false, - maxSize: MAX_HISTORY_SIZE, - - actions: { - saveSnapshot: (currentEntry: HistoryEntry) => { - const { gestureInProgress, maxSize } = get(); - - if (gestureInProgress) return; - - set((state) => { - const newPast = [...state.past, currentEntry]; - - if (newPast.length > maxSize) { - newPast.shift(); - } - - return { - past: newPast, - future: [] - }; - }); - }, - - undo: (currentEntry: HistoryEntry) => { - const { past } = get(); - if (past.length === 0) return null; - - const previous = past[past.length - 1]; - const newPast = past.slice(0, -1); - - set({ - past: newPast, - future: [currentEntry, ...get().future] - }); - - return previous; - }, - - redo: (currentEntry: HistoryEntry) => { - const { future } = get(); - if (future.length === 0) return null; - - const next = future[0]; - const newFuture = future.slice(1); - - set((state) => ({ - past: [...state.past, currentEntry], - future: newFuture - })); - - return next; - }, - - clearHistory: () => { - set({ - past: [], - future: [], - gestureInProgress: false - }); - }, - - beginGesture: (currentEntry: HistoryEntry) => { - const { gestureInProgress, maxSize } = get(); - if (gestureInProgress) return; - - set((state) => { - const newPast = [...state.past, currentEntry]; - - if (newPast.length > maxSize) { - newPast.shift(); - } - - return { - past: newPast, - future: [], - gestureInProgress: true - }; - }); - }, - - endGesture: () => { - set({ gestureInProgress: false }); - }, - - cancelGesture: () => { - const { past, gestureInProgress } = get(); - if (!gestureInProgress || past.length === 0) { - set({ gestureInProgress: false }); - return null; - } - - const previous = past[past.length - 1]; - const newPast = past.slice(0, -1); - - set({ - past: newPast, - gestureInProgress: false - }); - - return previous; - } - } - })); -}; - -const HistoryContext = createContext | null>( - null -); - -interface ProviderProps { - children: React.ReactNode; -} - -export const HistoryProvider = ({ children }: ProviderProps) => { - const storeRef = useRef | undefined>(undefined); - - if (!storeRef.current) { - storeRef.current = createHistoryStore(); - } - - return ( - - {children} - - ); -}; - -export function useHistoryStore( - selector: (state: HistoryStoreState) => T, - equalityFn?: (left: T, right: T) => boolean -) { - const store = useContext(HistoryContext); - - if (store === null) { - throw new Error('Missing HistoryProvider in the tree'); - } - - const value = useStore(store, selector, equalityFn); - return value; -} - -export function useHistoryStoreApi() { - const store = useContext(HistoryContext); - - if (store === null) { - throw new Error('Missing HistoryProvider in the tree'); - } - - return store; -} - -export const extractModelData = (state: { - version?: string; - title: string; - description?: string; - colors: Model['colors']; - icons: Model['icons']; - items: Model['items']; - views: Model['views']; -}): Model => { - return { - version: state.version, - title: state.title, - description: state.description, - colors: state.colors, - icons: state.icons, - items: state.items, - views: state.views - }; -}; - -export const extractSceneData = (state: { - connectors: Scene['connectors']; - textBoxes: Scene['textBoxes']; -}): Scene => { - return { - connectors: state.connectors, - textBoxes: state.textBoxes - }; -}; diff --git a/packages/fossflow-lib/src/stores/modelStore.tsx b/packages/fossflow-lib/src/stores/modelStore.tsx index 3025af6..178d3d0 100644 --- a/packages/fossflow-lib/src/stores/modelStore.tsx +++ b/packages/fossflow-lib/src/stores/modelStore.tsx @@ -3,19 +3,158 @@ import { createStore, useStore } from 'zustand'; import { ModelStore, Model } from 'src/types'; import { INITIAL_DATA } from 'src/config'; +export interface HistoryState { + past: Model[]; + present: Model; + future: Model[]; + maxHistorySize: number; +} + +export interface ModelStoreWithHistory extends Omit { + history: HistoryState; + actions: { + get: () => ModelStoreWithHistory; + set: (model: Partial, skipHistory?: boolean) => void; + undo: () => boolean; + redo: () => boolean; + canUndo: () => boolean; + canRedo: () => boolean; + saveToHistory: () => void; + clearHistory: () => void; + }; +} + +const MAX_HISTORY_SIZE = 50; + +const createHistoryState = (initialModel: Model): HistoryState => { + return { + past: [], + present: initialModel, + future: [], + maxHistorySize: MAX_HISTORY_SIZE + }; +}; + +const extractModelData = (state: ModelStoreWithHistory): Model => { + return { + version: state.version, + title: state.title, + description: state.description, + colors: state.colors, + icons: state.icons, + items: state.items, + views: state.views + }; +}; + const initialState = () => { - return createStore((set, get) => { + return createStore((set, get) => { const initialModel = { ...INITIAL_DATA }; + const saveToHistory = () => { + set((state) => { + const currentModel = extractModelData(state); + const newPast = [...state.history.past, state.history.present]; + + // Limit history size to prevent memory issues + if (newPast.length > state.history.maxHistorySize) { + newPast.shift(); + } + + return { + ...state, + history: { + ...state.history, + past: newPast, + present: currentModel, + future: [] // Clear future when new action is performed + } + }; + }); + }; + + const undo = (): boolean => { + const { history } = get(); + if (history.past.length === 0) return false; + + const previous = history.past[history.past.length - 1]; + const newPast = history.past.slice(0, history.past.length - 1); + + set((state) => { + return { + ...previous, + history: { + ...state.history, + past: newPast, + present: previous, + future: [state.history.present, ...state.history.future] + } + }; + }); + + return true; + }; + + const redo = (): boolean => { + const { history } = get(); + if (history.future.length === 0) return false; + + const next = history.future[0]; + const newFuture = history.future.slice(1); + + set((state) => { + return { + ...next, + history: { + ...state.history, + past: [...state.history.past, state.history.present], + present: next, + future: newFuture + } + }; + }); + + return true; + }; + + const canUndo = () => { + return get().history.past.length > 0; + }; + const canRedo = () => { + return get().history.future.length > 0; + }; + + const clearHistory = () => { + const currentState = get(); + const currentModel = extractModelData(currentState); + + set((state) => { + return { + ...state, + history: createHistoryState(currentModel) + }; + }); + }; + return { ...initialModel, + history: createHistoryState(initialModel), actions: { get, - set: (updates: Partial) => { + set: (updates: Partial, skipHistory = false) => { + if (!skipHistory) { + saveToHistory(); + } set((state) => { return { ...state, ...updates }; }); - } + }, + undo, + redo, + canUndo, + canRedo, + saveToHistory, + clearHistory } }; }); @@ -44,7 +183,7 @@ export const ModelProvider = ({ children }: ProviderProps) => { }; export function useModelStore( - selector: (state: ModelStore) => T, + selector: (state: ModelStoreWithHistory) => T, equalityFn?: (left: T, right: T) => boolean ) { const store = useContext(ModelContext); diff --git a/packages/fossflow-lib/src/stores/sceneStore.tsx b/packages/fossflow-lib/src/stores/sceneStore.tsx index ffe4a5b..fa95eb8 100644 --- a/packages/fossflow-lib/src/stores/sceneStore.tsx +++ b/packages/fossflow-lib/src/stores/sceneStore.tsx @@ -2,22 +2,156 @@ import React, { createContext, useRef, useContext } from 'react'; import { createStore, useStore } from 'zustand'; import { SceneStore, Scene } from 'src/types'; +export interface SceneHistoryState { + past: Scene[]; + present: Scene; + future: Scene[]; + maxHistorySize: number; +} + +export interface SceneStoreWithHistory extends Omit { + history: SceneHistoryState; + actions: { + get: () => SceneStoreWithHistory; + set: (scene: Partial, skipHistory?: boolean) => void; + undo: () => boolean; + redo: () => boolean; + canUndo: () => boolean; + canRedo: () => boolean; + saveToHistory: () => void; + clearHistory: () => void; + }; +} + +const MAX_HISTORY_SIZE = 50; + +const createSceneHistoryState = (initialScene: Scene): SceneHistoryState => { + return { + past: [], + present: initialScene, + future: [], + maxHistorySize: MAX_HISTORY_SIZE + }; +}; + +const extractSceneData = (state: SceneStoreWithHistory): Scene => { + return { + connectors: state.connectors, + textBoxes: state.textBoxes + }; +}; + const initialState = () => { - return createStore((set, get) => { + return createStore((set, get) => { const initialScene: Scene = { connectors: {}, textBoxes: {} }; + const saveToHistory = () => { + set((state) => { + const currentScene = extractSceneData(state); + const newPast = [...state.history.past, state.history.present]; + + // Limit history size + if (newPast.length > state.history.maxHistorySize) { + newPast.shift(); + } + + return { + ...state, + history: { + ...state.history, + past: newPast, + present: currentScene, + future: [] + } + }; + }); + }; + + const undo = (): boolean => { + const { history } = get(); + if (history.past.length === 0) return false; + + const previous = history.past[history.past.length - 1]; + const newPast = history.past.slice(0, history.past.length - 1); + + set((state) => { + return { + ...previous, + history: { + ...state.history, + past: newPast, + present: previous, + future: [state.history.present, ...state.history.future] + } + }; + }); + + return true; + }; + + const redo = (): boolean => { + const { history } = get(); + if (history.future.length === 0) return false; + + const next = history.future[0]; + const newFuture = history.future.slice(1); + + set((state) => { + return { + ...next, + history: { + ...state.history, + past: [...state.history.past, state.history.present], + present: next, + future: newFuture + } + }; + }); + + return true; + }; + + const canUndo = () => { + return get().history.past.length > 0; + }; + const canRedo = () => { + return get().history.future.length > 0; + }; + + const clearHistory = () => { + const currentState = get(); + const currentScene = extractSceneData(currentState); + + set((state) => { + return { + ...state, + history: createSceneHistoryState(currentScene) + }; + }); + }; + return { ...initialScene, + history: createSceneHistoryState(initialScene), actions: { get, - set: (updates: Partial) => { + set: (updates: Partial, skipHistory = false) => { + if (!skipHistory) { + saveToHistory(); + } set((state) => { return { ...state, ...updates }; }); - } + }, + undo, + redo, + canUndo, + canRedo, + saveToHistory, + clearHistory } }; }); @@ -46,7 +180,7 @@ export const SceneProvider = ({ children }: ProviderProps) => { }; export function useSceneStore( - selector: (state: SceneStore) => T, + selector: (state: SceneStoreWithHistory) => T, equalityFn?: (left: T, right: T) => boolean ) { const store = useContext(SceneContext); diff --git a/packages/fossflow-lib/src/types/interactions.ts b/packages/fossflow-lib/src/types/interactions.ts index 6461666..17ee944 100644 --- a/packages/fossflow-lib/src/types/interactions.ts +++ b/packages/fossflow-lib/src/types/interactions.ts @@ -1,15 +1,10 @@ import { ModelStore, UiStateStore, Size } from 'src/types'; import { useScene } from 'src/hooks/useScene'; -import { useHistory } from 'src/hooks/useHistory'; export interface State { model: ModelStore; scene: ReturnType; uiState: UiStateStore; - history: Pick< - ReturnType, - 'beginGesture' | 'endGesture' | 'cancelGesture' | 'isGestureInProgress' - >; rendererRef: HTMLElement; rendererSize: Size; isRendererInteraction: boolean; diff --git a/packages/fossflow-lib/src/types/model.ts b/packages/fossflow-lib/src/types/model.ts index fb053d8..35f2217 100644 --- a/packages/fossflow-lib/src/types/model.ts +++ b/packages/fossflow-lib/src/types/model.ts @@ -17,6 +17,8 @@ import { connectorStyleOptions, connectorLineTypeOptions } from 'src/schemas'; +import { StoreApi } from 'zustand'; + export { connectorStyleOptions, connectorLineTypeOptions } from 'src/schemas'; export type Model = z.infer; export type ModelItems = z.infer; @@ -37,7 +39,17 @@ export type Rectangle = z.infer; export type ModelStore = Model & { actions: { - get: () => ModelStore; - set: (updates: Partial) => void; + get: StoreApi['getState']; + set: StoreApi['setState']; }; }; + +export type { + ModelStoreWithHistory, + HistoryState as ModelHistoryState +} from 'src/stores/modelStore'; + +export type { + SceneStoreWithHistory, + SceneHistoryState +} from 'src/stores/sceneStore'; diff --git a/packages/fossflow-lib/src/types/scene.ts b/packages/fossflow-lib/src/types/scene.ts index 6b7be56..b33654d 100644 --- a/packages/fossflow-lib/src/types/scene.ts +++ b/packages/fossflow-lib/src/types/scene.ts @@ -1,11 +1,5 @@ +import { StoreApi } from 'zustand'; import type { Coords, Rect, Size } from './common'; -import type { ViewItem, Rectangle, TextBox } from './model'; - -export interface SceneItemsSnapshot { - items: Pick[]; - rectangles: Pick[]; - textBoxes: Pick[]; -} export const tileOriginOptions = { CENTER: 'CENTER', @@ -56,7 +50,7 @@ export interface Scene { export type SceneStore = Scene & { actions: { - get: () => SceneStore; - set: (updates: Partial) => void; + get: StoreApi['getState']; + set: StoreApi['setState']; }; }; diff --git a/packages/fossflow-lib/src/types/ui.ts b/packages/fossflow-lib/src/types/ui.ts index 46c5386..9d84dff 100644 --- a/packages/fossflow-lib/src/types/ui.ts +++ b/packages/fossflow-lib/src/types/ui.ts @@ -44,7 +44,7 @@ export interface DragItemsMode { type: 'DRAG_ITEMS'; showCursor: boolean; items: ItemReference[]; - isInitialMovement: boolean; + isInitialMovement: Boolean; } export interface PanMode {