Compare commits

..

1 Commits

Author SHA1 Message Date
Claude Bot
04387d66a9 fix: sync tree node title on graph node rename (closes #10) 2026-03-31 07:26:09 +00:00
3 changed files with 20 additions and 89 deletions

View File

@@ -104,90 +104,3 @@ test.describe('Breadcrumb navigation', () => {
await expect(page.locator('.ant-breadcrumb')).not.toContainText('Start'); await expect(page.locator('.ant-breadcrumb')).not.toContainText('Start');
}); });
}); });
// Regression tests for issue #2: stale closure caused breadcrumbs to disappear on back-navigation.
//
// The bug: createPathSegment's onClick closed over `graphsPath` from the render in which
// the segment was created. After further navigation the closure was stale, so findIndex
// returned -1 and splice(0) wiped the entire breadcrumb array.
//
// The fix: use the functional updater form of setGraphsPath so findIndex always runs
// against the *current* state rather than a captured snapshot.
test.describe('Stale-closure regression (issue #2)', () => {
test.beforeEach(async ({ page }) => {
await page.goto('/');
await waitForGraph(page);
});
test('root breadcrumb remains functional after navigating three levels deep', async ({ page }) => {
// Navigate 3 levels deep — each additional level re-renders breadcrumbs with new
// closures; before the fix, clicking the root would hit a stale closure from level 1
// and wipe everything.
await openSubgraph(page, 'Start'); // level 1
await openSubgraph(page, 'Start'); // level 2
await openSubgraph(page, 'Start'); // level 3
await expect(breadcrumbLinks(page)).toHaveCount(4);
// Click the root ("Main") breadcrumb
await breadcrumbLinks(page).first().click();
// Breadcrumbs must NOT disappear — exactly one "Main" segment remains
await expect(breadcrumbLinks(page)).toHaveCount(1, { timeout: 3000 });
await expect(page.locator('.ant-breadcrumb')).toContainText('Main');
await waitForGraph(page);
// The root graph is rendered correctly
await expect(nodeByLabel(page, 'Start')).toBeVisible();
await expect(nodeByLabel(page, 'End')).toBeVisible();
});
test('level-1 breadcrumb remains functional when clicked from level 3', async ({ page }) => {
// This is the core regression scenario: a breadcrumb segment created at level 1
// must still resolve correctly after two more navigations have updated the path state.
await openSubgraph(page, 'Start'); // level 1
await openSubgraph(page, 'Start'); // level 2
await openSubgraph(page, 'Start'); // level 3
await expect(breadcrumbLinks(page)).toHaveCount(4);
// Click the level-1 breadcrumb (second item)
await breadcrumbLinks(page).nth(1).click();
// Should trim to exactly 2 items — NOT clear everything
await expect(breadcrumbLinks(page)).toHaveCount(2, { timeout: 3000 });
await waitForGraph(page);
// The graph at level 1 is shown
await expect(nodeByLabel(page, 'Start')).toBeVisible();
});
test('successive back-clicks each trim exactly one breadcrumb level', async ({ page }) => {
await openSubgraph(page, 'Start'); // level 1
await openSubgraph(page, 'Start'); // level 2
await openSubgraph(page, 'Start'); // level 3
await expect(breadcrumbLinks(page)).toHaveCount(4);
// Click level-2 breadcrumb
await breadcrumbLinks(page).nth(2).click();
await expect(breadcrumbLinks(page)).toHaveCount(3, { timeout: 3000 });
// Then click level-1 breadcrumb
await breadcrumbLinks(page).nth(1).click();
await expect(breadcrumbLinks(page)).toHaveCount(2, { timeout: 3000 });
// Then click root breadcrumb
await breadcrumbLinks(page).first().click();
await expect(breadcrumbLinks(page)).toHaveCount(1, { timeout: 3000 });
await expect(page.locator('.ant-breadcrumb')).toContainText('Main');
});
test('clicking the active (last) breadcrumb does not alter the path', async ({ page }) => {
// Clicking the segment you are already on should be a no-op — it must not clear
// breadcrumbs by accidentally slicing at index -1.
await openSubgraph(page, 'Start'); // level 1
await openSubgraph(page, 'Start'); // level 2
await expect(breadcrumbLinks(page)).toHaveCount(3);
// The last breadcrumb link is the currently active level
await breadcrumbLinks(page).last().click();
await expect(breadcrumbLinks(page)).toHaveCount(3, { timeout: 3000 });
});
});

View File

@@ -1,6 +1,7 @@
import { Input, Modal } from "antd"; import { Input, Modal } from "antd";
import { graphContext, type NodeContext } from "./Graph"; import { graphContext, type NodeContext } from "./Graph";
import { useContext, useState } from "react"; import { useContext, useState } from "react";
import { useGraphLayersTreeStore } from "../stores/TreeStore";
export default function NodeRenameModal({ export default function NodeRenameModal({
nodeContext, nodeContext,
@@ -16,6 +17,7 @@ export default function NodeRenameModal({
} }
const [nodeName, setSelectedNodeName] = useState(nodeContext.nodeName); const [nodeName, setSelectedNodeName] = useState(nodeContext.nodeName);
const graphContextValue = useContext(graphContext)!; const graphContextValue = useContext(graphContext)!;
const renameTreeNode = useGraphLayersTreeStore(state => state.rename);
function renameNode() { function renameNode() {
const node = graphContextValue.graph.nodes.find(n => n.id === nodeContext.nodeId); const node = graphContextValue.graph.nodes.find(n => n.id === nodeContext.nodeId);
@@ -24,6 +26,7 @@ export default function NodeRenameModal({
} }
node.label = nodeName; node.label = nodeName;
graphContextValue.setGraph(prev => ({ ...prev, nodes: graphContextValue.graph.nodes })); graphContextValue.setGraph(prev => ({ ...prev, nodes: graphContextValue.graph.nodes }));
renameTreeNode(nodeContext.nodeId, nodeName);
openRenameModal(false); openRenameModal(false);
} }

View File

@@ -10,6 +10,7 @@ export interface TreeStore {
tree: TreeDataNode[]; tree: TreeDataNode[];
add: (childNode: NodeContext, parentNodeId: string | undefined) => void; add: (childNode: NodeContext, parentNodeId: string | undefined) => void;
remove: (nodeId: string) => void; remove: (nodeId: string) => void;
rename: (nodeId: string, newName: string) => void;
reset: () => void; reset: () => void;
} }
@@ -90,6 +91,20 @@ export const useGraphLayersTreeStore = create<TreeStore>()((set) => ({
tree: createTree([...state.rootNodes], nodesFlatById) tree: createTree([...state.rootNodes], nodesFlatById)
} }
}), }),
rename: (nodeId, newName) => set((state) => {
const node = state.nodesFlatById.get(nodeId);
if (!node) {
return state;
}
const nodesFlatById = new Map(state.nodesFlatById);
nodesFlatById.set(nodeId, { ...node, title: newName });
return {
...state,
nodesFlatById,
rootNodes: [...state.rootNodes],
tree: createTree([...state.rootNodes], nodesFlatById),
};
}),
reset: () => set({ reset: () => set({
nodesFlatById: new Map<React.Key, TreeDataNode>(), nodesFlatById: new Map<React.Key, TreeDataNode>(),
parentIdByChildId: new Map<React.Key, string>(), parentIdByChildId: new Map<React.Key, string>(),