Skip to content

Commit 88d8bf3

Browse files
milanofthemilanofthe
authored
Merge pull request #245 from pathsim/fix/history-coverage
Wrap all graph mutations in historyStore.mutate() for full undo/redo
2 parents f4f7b06 + d280072 commit 88d8bf3

9 files changed

Lines changed: 151 additions & 116 deletions

File tree

‎src/lib/components/FlowCanvas.svelte‎

Lines changed: 51 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -166,21 +166,23 @@
166166
return n;
167167
});
168168
169-
// Sync positions to appropriate stores
170-
nodes.forEach(n => {
171-
if (n.selected) {
172-
if (n.type === 'eventNode') {
173-
if (graphStore.isAtRoot()) {
174-
eventStore.updateEventPosition(n.id, n.position);
169+
// Sync positions to appropriate stores (as a single undoable action)
170+
historyStore.mutate(() => {
171+
nodes.forEach(n => {
172+
if (n.selected) {
173+
if (n.type === 'eventNode') {
174+
if (graphStore.isAtRoot()) {
175+
eventStore.updateEventPosition(n.id, n.position);
176+
} else {
177+
graphStore.updateSubsystemEventPosition(n.id, n.position);
178+
}
179+
} else if (n.type === 'annotation') {
180+
graphStore.updateAnnotationPosition(n.id, n.position);
175181
} else {
176-
graphStore.updateSubsystemEventPosition(n.id, n.position);
182+
graphStore.updateNodePosition(n.id, n.position);
177183
}
178-
} else if (n.type === 'annotation') {
179-
graphStore.updateAnnotationPosition(n.id, n.position);
180-
} else {
181-
graphStore.updateNodePosition(n.id, n.position);
182184
}
183-
}
185+
});
184186
});
185187
}
186188
});
@@ -802,47 +804,49 @@
802804
const targetMatch = connection.targetHandle.match(/-input-(\d+)$/);
803805
804806
if (sourceMatch && targetMatch) {
805-
const sourcePortIndex = parseInt(sourceMatch[1], 10);
806-
let targetPortIndex = parseInt(targetMatch[1], 10);
807+
historyStore.mutate(() => {
808+
const sourcePortIndex = parseInt(sourceMatch[1], 10);
809+
let targetPortIndex = parseInt(targetMatch[1], 10);
807810
808-
// Check if the target port is already connected
809-
const currentConnections = get(graphStore.connections);
810-
const isPortOccupied = currentConnections.some(
811-
(c) => c.targetNodeId === connection.target && c.targetPortIndex === targetPortIndex
812-
);
811+
// Check if the target port is already connected
812+
const currentConnections = get(graphStore.connections);
813+
const isPortOccupied = currentConnections.some(
814+
(c) => c.targetNodeId === connection.target && c.targetPortIndex === targetPortIndex
815+
);
813816
814-
if (isPortOccupied) {
815-
// Find the first available port instead
816-
const availablePort = findFirstAvailableInputPort(connection.target);
817+
if (isPortOccupied) {
818+
// Find the first available port instead
819+
const availablePort = findFirstAvailableInputPort(connection.target);
817820
818-
if (availablePort !== null) {
819-
// Use the first available port
820-
targetPortIndex = availablePort;
821-
} else {
822-
// No available port - check if we can create a new one
823-
const graphNodes = get(graphStore.nodesArray);
824-
const targetNode = graphNodes.find((n) => n.id === connection.target);
825-
if (!targetNode) return;
826-
827-
const typeDef = nodeRegistry.get(targetNode.type);
828-
if (!typeDef || typeDef.ports.maxInputs !== null) {
829-
// Can't create new port, abort
830-
return;
831-
}
821+
if (availablePort !== null) {
822+
// Use the first available port
823+
targetPortIndex = availablePort;
824+
} else {
825+
// No available port - check if we can create a new one
826+
const graphNodes = get(graphStore.nodesArray);
827+
const targetNode = graphNodes.find((n) => n.id === connection.target);
828+
if (!targetNode) return;
829+
830+
const typeDef = nodeRegistry.get(targetNode.type);
831+
if (!typeDef || typeDef.ports.maxInputs !== null) {
832+
// Can't create new port, abort
833+
return;
834+
}
832835
833-
// Create a new port and use it
834-
graphStore.addInputPort(connection.target);
835-
targetPortIndex = targetNode.inputs.length; // The new port index
836+
// Create a new port and use it
837+
graphStore.addInputPort(connection.target);
838+
targetPortIndex = targetNode.inputs.length; // The new port index
839+
}
836840
}
837-
}
838841
839-
// addConnection uses current navigation context automatically
840-
graphStore.addConnection(
841-
connection.source,
842-
sourcePortIndex,
843-
connection.target,
844-
targetPortIndex
845-
);
842+
// addConnection uses current navigation context automatically
843+
graphStore.addConnection(
844+
connection.source,
845+
sourcePortIndex,
846+
connection.target,
847+
targetPortIndex
848+
);
849+
});
846850
}
847851
}
848852

‎src/lib/components/FlowUpdater.svelte‎

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import { useSvelteFlow, useUpdateNodeInternals } from '@xyflow/svelte';
44
import { graphStore } from '$lib/stores/graph';
55
import { eventStore } from '$lib/stores/events';
6+
import { historyStore } from '$lib/stores/history';
67
import { eventRegistry } from '$lib/events/registry';
78
import type { EventInstance } from '$lib/events/types';
89
import { fitViewTrigger, fitViewPadding, type FitViewPadding, zoomInTrigger, zoomOutTrigger, panTrigger, focusNodeTrigger, registerScreenToFlowConverter } from '$lib/stores/viewActions';
@@ -131,9 +132,11 @@
131132
const nodeType = event.dataTransfer?.getData('application/pathview-node');
132133
if (nodeType) {
133134
// addNode uses current navigation context automatically
134-
graphStore.addNode(nodeType, {
135-
x: position.x - 80,
136-
y: position.y - 30
135+
historyStore.mutate(() => {
136+
graphStore.addNode(nodeType, {
137+
x: position.x - 80,
138+
y: position.y - 30
139+
});
137140
});
138141
return;
139142
}
@@ -146,23 +149,25 @@
146149
y: position.y - 40
147150
};
148151
149-
if (graphStore.isAtRoot()) {
150-
// Root level: use eventStore
151-
eventStore.addEvent(eventType, eventPos);
152-
} else {
153-
// Subsystem level: use graphStore
154-
const typeDef = eventRegistry.get(eventType);
155-
if (typeDef) {
156-
const newEvent: EventInstance = {
157-
id: crypto.randomUUID(),
158-
type: eventType,
159-
name: typeDef.name,
160-
position: eventPos,
161-
params: {}
162-
};
163-
graphStore.addSubsystemEvent(newEvent);
152+
historyStore.mutate(() => {
153+
if (graphStore.isAtRoot()) {
154+
// Root level: use eventStore
155+
eventStore.addEvent(eventType, eventPos);
156+
} else {
157+
// Subsystem level: use graphStore
158+
const typeDef = eventRegistry.get(eventType);
159+
if (typeDef) {
160+
const newEvent: EventInstance = {
161+
id: crypto.randomUUID(),
162+
type: eventType,
163+
name: typeDef.name,
164+
position: eventPos,
165+
params: {}
166+
};
167+
graphStore.addSubsystemEvent(newEvent);
168+
}
164169
}
165-
}
170+
});
166171
return;
167172
}
168173
});

‎src/lib/components/contextMenuBuilders.ts‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { get } from 'svelte/store';
77
import type { MenuItemType } from './ContextMenu.svelte';
88
import type { ContextMenuTarget } from '$lib/stores/contextMenu';
99
import { graphStore, ANNOTATION_FONT_SIZE } from '$lib/stores/graph';
10+
import { historyStore } from '$lib/stores/history';
1011
import { routingStore } from '$lib/stores/routing';
1112
import { eventStore } from '$lib/stores/events';
1213
import { clipboardStore } from '$lib/stores/clipboard';
@@ -142,7 +143,7 @@ function buildNodeMenu(nodeId: string): MenuItemType[] {
142143
label: 'Delete',
143144
icon: 'trash',
144145
shortcut: 'Del',
145-
action: () => graphStore.removeNode(nodeId)
146+
action: () => historyStore.mutate(() => graphStore.removeNode(nodeId))
146147
}
147148
];
148149
}
@@ -191,7 +192,7 @@ function buildNodeMenu(nodeId: string): MenuItemType[] {
191192
shortcut: 'Ctrl+D',
192193
action: () => {
193194
graphStore.selectNode(nodeId, false);
194-
graphStore.duplicateSelected();
195+
historyStore.mutate(() => graphStore.duplicateSelected());
195196
}
196197
},
197198
{
@@ -208,7 +209,7 @@ function buildNodeMenu(nodeId: string): MenuItemType[] {
208209
label: 'Delete',
209210
icon: 'trash',
210211
shortcut: 'Del',
211-
action: () => graphStore.removeNode(nodeId)
212+
action: () => historyStore.mutate(() => graphStore.removeNode(nodeId))
212213
}
213214
);
214215

@@ -228,7 +229,7 @@ function buildSelectionMenu(
228229
label: `Duplicate ${count} nodes`,
229230
icon: 'copy',
230231
shortcut: 'Ctrl+D',
231-
action: () => graphStore.duplicateSelected()
232+
action: () => historyStore.mutate(() => graphStore.duplicateSelected())
232233
},
233234
{
234235
label: `Copy ${count} nodes`,
@@ -270,7 +271,7 @@ function buildEventMenu(eventId: string): MenuItemType[] {
270271
shortcut: 'Ctrl+D',
271272
action: () => {
272273
eventStore.selectEvent(eventId, false);
273-
eventStore.duplicateSelected();
274+
historyStore.mutate(() => eventStore.duplicateSelected());
274275
}
275276
},
276277
{
@@ -287,7 +288,7 @@ function buildEventMenu(eventId: string): MenuItemType[] {
287288
label: 'Delete',
288289
icon: 'trash',
289290
shortcut: 'Del',
290-
action: () => eventStore.removeEvent(eventId)
291+
action: () => historyStore.mutate(() => eventStore.removeEvent(eventId))
291292
}
292293
];
293294
}
@@ -307,7 +308,7 @@ function buildEdgeMenu(edgeId: string): MenuItemType[] {
307308
label: 'Delete',
308309
icon: 'trash',
309310
shortcut: 'Del',
310-
action: () => graphStore.removeConnection(edgeId)
311+
action: () => historyStore.mutate(() => graphStore.removeConnection(edgeId))
311312
}
312313
];
313314
}
@@ -338,7 +339,7 @@ function buildCanvasMenu(
338339
icon: 'type',
339340
action: () => {
340341
const flowPos = screenToFlow(screenPosition);
341-
graphStore.addAnnotation(flowPos);
342+
historyStore.mutate(() => graphStore.addAnnotation(flowPos));
342343
}
343344
},
344345
DIVIDER,
@@ -413,7 +414,7 @@ function buildAnnotationMenu(annotationId: string): MenuItemType[] {
413414
icon: 'font-size-increase',
414415
action: () => {
415416
if (currentFontSize < ANNOTATION_FONT_SIZE.MAX) {
416-
graphStore.updateAnnotation(annotationId, { fontSize: currentFontSize + ANNOTATION_FONT_SIZE.STEP });
417+
historyStore.mutate(() => graphStore.updateAnnotation(annotationId, { fontSize: currentFontSize + ANNOTATION_FONT_SIZE.STEP }));
417418
}
418419
},
419420
disabled: currentFontSize >= ANNOTATION_FONT_SIZE.MAX
@@ -423,7 +424,7 @@ function buildAnnotationMenu(annotationId: string): MenuItemType[] {
423424
icon: 'font-size-decrease',
424425
action: () => {
425426
if (currentFontSize > ANNOTATION_FONT_SIZE.MIN) {
426-
graphStore.updateAnnotation(annotationId, { fontSize: currentFontSize - ANNOTATION_FONT_SIZE.STEP });
427+
historyStore.mutate(() => graphStore.updateAnnotation(annotationId, { fontSize: currentFontSize - ANNOTATION_FONT_SIZE.STEP }));
427428
}
428429
},
429430
disabled: currentFontSize <= ANNOTATION_FONT_SIZE.MIN
@@ -435,7 +436,7 @@ function buildAnnotationMenu(annotationId: string): MenuItemType[] {
435436
shortcut: 'Ctrl+D',
436437
action: () => {
437438
graphStore.selectNode(annotationId, false);
438-
graphStore.duplicateSelected();
439+
historyStore.mutate(() => graphStore.duplicateSelected());
439440
}
440441
},
441442
{
@@ -452,7 +453,7 @@ function buildAnnotationMenu(annotationId: string): MenuItemType[] {
452453
label: 'Delete',
453454
icon: 'trash',
454455
shortcut: 'Del',
455-
action: () => graphStore.removeAnnotation(annotationId)
456+
action: () => historyStore.mutate(() => graphStore.removeAnnotation(annotationId))
456457
}
457458
];
458459
}

‎src/lib/components/dialogs/BlockPropertiesDialog.svelte‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import { fade, scale } from 'svelte/transition';
44
import { cubicOut } from 'svelte/easing';
55
import { graphStore } from '$lib/stores/graph';
6+
import { historyStore } from '$lib/stores/history';
67
import { nodeDialogStore, closeNodeDialog } from '$lib/stores/nodeDialog';
78
import { nodeRegistry, type NodeInstance } from '$lib/nodes';
89
import { generateBlockCode } from '$lib/pyodide/pathsimRunner';
@@ -58,7 +59,8 @@
5859
// Handle color selection
5960
function handleColorSelect(color: string | undefined) {
6061
if (!node) return;
61-
graphStore.updateNodeColor(node.id, color);
62+
const nodeId = node.id;
63+
historyStore.mutate(() => graphStore.updateNodeColor(nodeId, color));
6264
}
6365
6466
// Code preview state
@@ -112,7 +114,8 @@
112114
// No parsing or type coercion - user writes valid Python syntax
113115
function handleParamChange(paramName: string, value: string) {
114116
if (!node) return;
115-
graphStore.updateNodeParams(node.id, { [paramName]: value });
117+
const nodeId = node.id;
118+
historyStore.mutate(() => graphStore.updateNodeParams(nodeId, { [paramName]: value }));
116119
}
117120
118121
// Check if a parameter is pinned to the node
@@ -123,17 +126,19 @@
123126
// Toggle pin state for a parameter
124127
function togglePinParam(paramName: string) {
125128
if (!node) return;
129+
const nodeId = node.id;
126130
const currentPinned = node.pinnedParams ?? [];
127131
const newPinned = currentPinned.includes(paramName)
128132
? currentPinned.filter(p => p !== paramName)
129133
: [...currentPinned, paramName];
130-
graphStore.updateNode(node.id, { pinnedParams: newPinned });
134+
historyStore.mutate(() => graphStore.updateNode(nodeId, { pinnedParams: newPinned }));
131135
}
132136
133137
// Handle name change
134138
function handleNameChange(name: string) {
135139
if (!node) return;
136-
graphStore.updateNodeName(node.id, name);
140+
const nodeId = node.id;
141+
historyStore.mutate(() => graphStore.updateNodeName(nodeId, name));
137142
}
138143
139144
// Format value for display

‎src/lib/components/dialogs/EventPropertiesDialog.svelte‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import { cubicOut } from 'svelte/easing';
55
import { get } from 'svelte/store';
66
import { eventStore } from '$lib/stores/events';
7+
import { historyStore } from '$lib/stores/history';
78
import { eventDialogStore, closeEventDialog } from '$lib/stores/eventDialog';
89
import { eventRegistry } from '$lib/events/registry';
910
import { graphStore } from '$lib/stores/graph';
@@ -53,7 +54,8 @@
5354
// Handle color selection
5455
function handleColorSelect(color: string | undefined) {
5556
if (!event) return;
56-
eventStore.updateEventColor(event.id, color);
57+
const eventId = event.id;
58+
historyStore.mutate(() => eventStore.updateEventColor(eventId, color));
5759
}
5860
5961
// Code preview state
@@ -91,13 +93,15 @@
9193
// Handle parameter change
9294
function handleParamChange(paramName: string, value: string) {
9395
if (!event) return;
94-
eventStore.updateEventParams(event.id, { [paramName]: value });
96+
const eventId = event.id;
97+
historyStore.mutate(() => eventStore.updateEventParams(eventId, { [paramName]: value }));
9598
}
9699
97100
// Handle name change
98101
function handleNameChange(name: string) {
99102
if (!event) return;
100-
eventStore.updateEventName(event.id, name);
103+
const eventId = event.id;
104+
historyStore.mutate(() => eventStore.updateEventName(eventId, name));
101105
}
102106
103107
// Format value for display

0 commit comments

Comments
 (0)