Skip to content

Commit 98f5c41

Browse files
Patel230claude
andcommitted
Fix synchronization between NodeDetailsModal and graph visualization
- Fixed stale closure in handleSave using useCallback with proper dependencies - Added dual protection (isSaving state + isSavingRef) to prevent multiple saves - Enhanced updateVisualizationData to update DOM elements (titles, types, descriptions) - Extracted getNodeDimensions as shared useCallback function for proper scoping - Added fresh node lookup by ID before passing to modal (synced with Apollo cache) - Track node property changes in useEffect dependencies for immediate updates - Resolves issues with graph not updating after modal saves Co-Authored-By: Claude <noreply@anthropic.com>
1 parent a30ac9d commit 98f5c41

2 files changed

Lines changed: 204 additions & 101 deletions

File tree

packages/web/src/components/InteractiveGraphVisualization.tsx

Lines changed: 160 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -1282,6 +1282,57 @@ export function InteractiveGraphVisualization({ onResetLayout }: InteractiveGrap
12821282
}
12831283
};
12841284

1285+
// Helper function to calculate node dimensions (shared between init and update)
1286+
const getNodeDimensions = useCallback((d: WorkItem) => {
1287+
// Larger base dimensions by type - increased for 3-line support
1288+
const baseDimensions = (() => {
1289+
switch (d.type) {
1290+
case 'EPIC': return { width: 200, height: 120 };
1291+
case 'MILESTONE': return { width: 190, height: 115 };
1292+
case 'FEATURE': return { width: 180, height: 110 };
1293+
case 'OUTCOME': return { width: 185, height: 115 };
1294+
case 'TASK': return { width: 170, height: 105 };
1295+
case 'BUG': return { width: 165, height: 100 };
1296+
case 'IDEA': return { width: 160, height: 95 };
1297+
default: return { width: 170, height: 105 };
1298+
}
1299+
})();
1300+
1301+
// Use base width for consistent layout
1302+
const width = baseDimensions.width;
1303+
1304+
// Calculate actual text wrapping based on word breaks - up to 3 lines
1305+
const maxCharsPerLine = Math.floor(width / 8); // ~8px per character for more conservative wrapping
1306+
const words = d.title.split(' ');
1307+
let lines = 1;
1308+
let currentLineLength = 0;
1309+
1310+
for (const word of words) {
1311+
const wordLength = word.length;
1312+
// Check if adding this word would exceed line length
1313+
if (currentLineLength + wordLength + 1 > maxCharsPerLine && currentLineLength > 0) {
1314+
lines++;
1315+
currentLineLength = wordLength;
1316+
if (lines >= 3) break; // Maximum 3 lines
1317+
} else {
1318+
currentLineLength += wordLength + (currentLineLength > 0 ? 1 : 0); // +1 for space
1319+
}
1320+
}
1321+
1322+
lines = Math.min(lines, 3); // Maximum 3 lines
1323+
1324+
// Calculate additional height needed for multiple lines (16px per extra line)
1325+
const additionalHeight = (lines - 1) * 16;
1326+
const finalHeight = baseDimensions.height + additionalHeight;
1327+
1328+
return {
1329+
width: width,
1330+
height: finalHeight,
1331+
titleLines: lines,
1332+
maxCharsPerLine: maxCharsPerLine
1333+
};
1334+
}, []);
1335+
12851336
// Smart data update function - only reinitializes when necessary, preserves camera position
12861337
const updateVisualizationData = useCallback(() => {
12871338
if (!simulationRef.current || !svgRef.current) return;
@@ -1313,21 +1364,93 @@ export function InteractiveGraphVisualization({ onResetLayout }: InteractiveGrap
13131364
return;
13141365
}
13151366

1316-
// If counts are the same, just update simulation data (for property changes)
1317-
console.log('[Graph Debug] Data counts unchanged - updating simulation data only');
1318-
1367+
// If counts are the same, update both simulation data AND DOM elements (for property changes)
1368+
console.log('[Graph Debug] Data counts unchanged - updating simulation data and DOM elements');
1369+
13191370
// Update simulation with current data (handles property changes)
13201371
simulation.nodes(nodes as any);
13211372
const linkForce = simulation.force('link') as d3.ForceLink<any, any>;
13221373
if (linkForce) {
13231374
linkForce.links(validatedEdges);
13241375
}
13251376

1377+
// UPDATE DOM ELEMENTS: Rebind data and update visual properties
1378+
// Update node titles
1379+
const nodeGroups = svg.select('.nodes-group').selectAll('.node');
1380+
nodeGroups.each(function(d: any) {
1381+
const nodeGroup = d3.select(this);
1382+
// Find the updated node data by ID
1383+
const updatedNode = nodes.find(n => n.id === d.id);
1384+
if (!updatedNode) return;
1385+
1386+
// Update title text elements
1387+
nodeGroup.selectAll('.node-title-text').remove();
1388+
const maxCharsPerLine = 18;
1389+
const maxLines = 2;
1390+
let lines: string[] = [];
1391+
const words = updatedNode.title.split(' ');
1392+
let currentLine = '';
1393+
1394+
for (const word of words) {
1395+
const testLine = currentLine ? `${currentLine} ${word}` : word;
1396+
if (testLine.length > maxCharsPerLine && currentLine) {
1397+
lines.push(currentLine);
1398+
currentLine = word;
1399+
if (lines.length >= maxLines) break;
1400+
} else {
1401+
currentLine = testLine;
1402+
}
1403+
}
1404+
if (currentLine && lines.length < maxLines) {
1405+
lines.push(currentLine);
1406+
}
1407+
if (lines.length === 0) {
1408+
lines.push(updatedNode.title.substring(0, maxCharsPerLine - 3) + '...');
1409+
}
1410+
1411+
const dimensions = getNodeDimensions(updatedNode);
1412+
const titleBarHeight = 32;
1413+
const startY = -dimensions.height / 2 + titleBarHeight + 18;
1414+
lines.forEach((line, index) => {
1415+
nodeGroup.append('text')
1416+
.attr('class', 'node-title-text')
1417+
.attr('x', 0)
1418+
.attr('y', startY + (index * 16))
1419+
.attr('text-anchor', 'middle')
1420+
.attr('dominant-baseline', 'middle')
1421+
.text(line)
1422+
.style('font-size', '14px')
1423+
.style('font-weight', '600')
1424+
.style('fill', () => {
1425+
const isCompleted = updatedNode.status === 'COMPLETED' || updatedNode.status === 'Completed' || updatedNode.status === 'Done' || updatedNode.status === 'DONE';
1426+
return isCompleted ? '#9ca3af' : '#ffffff';
1427+
})
1428+
.style('pointer-events', 'none');
1429+
});
1430+
1431+
// Update node type badge text
1432+
nodeGroup.select('.node-type-text')
1433+
.text(() => {
1434+
const config = getTypeConfig(updatedNode.type as WorkItemType);
1435+
return config.label.toUpperCase();
1436+
});
1437+
1438+
// Update description
1439+
nodeGroup.select('.node-description-text')
1440+
.text(() => {
1441+
if (!updatedNode.description) return '';
1442+
const maxDescChars = 25;
1443+
return updatedNode.description.length > maxDescChars
1444+
? updatedNode.description.substring(0, maxDescChars) + '...'
1445+
: updatedNode.description;
1446+
});
1447+
});
1448+
13261449
// Gentle restart to settle any property changes
13271450
simulation.alpha(0.1).restart();
1328-
1329-
console.log('[Graph Debug] Simulation data updated');
1330-
}, [nodes, validatedEdges]);
1451+
1452+
console.log('[Graph Debug] Simulation data and DOM elements updated');
1453+
}, [nodes, validatedEdges, getNodeDimensions]);
13311454

13321455
// Define initializeVisualization function with access to nodes data
13331456
const initializeVisualization = useCallback(() => {
@@ -1870,56 +1993,8 @@ export function InteractiveGraphVisualization({ onResetLayout }: InteractiveGrap
18701993
}));
18711994

18721995
// Monopoly-style rectangular nodes with colored title bars
1873-
const getNodeDimensions = (d: WorkItem) => {
1874-
// Larger base dimensions by type - increased for 3-line support
1875-
const baseDimensions = (() => {
1876-
switch (d.type) {
1877-
case 'EPIC': return { width: 200, height: 120 };
1878-
case 'MILESTONE': return { width: 190, height: 115 };
1879-
case 'FEATURE': return { width: 180, height: 110 };
1880-
case 'OUTCOME': return { width: 185, height: 115 };
1881-
case 'TASK': return { width: 170, height: 105 };
1882-
case 'BUG': return { width: 165, height: 100 };
1883-
case 'IDEA': return { width: 160, height: 95 };
1884-
default: return { width: 170, height: 105 };
1885-
}
1886-
})();
1887-
1888-
// Use base width for consistent layout
1889-
const width = baseDimensions.width;
1890-
1891-
// Calculate actual text wrapping based on word breaks - up to 3 lines
1892-
const maxCharsPerLine = Math.floor(width / 8); // ~8px per character for more conservative wrapping
1893-
const words = d.title.split(' ');
1894-
let lines = 1;
1895-
let currentLineLength = 0;
1896-
1897-
for (const word of words) {
1898-
const wordLength = word.length;
1899-
// Check if adding this word would exceed line length
1900-
if (currentLineLength + wordLength + 1 > maxCharsPerLine && currentLineLength > 0) {
1901-
lines++;
1902-
currentLineLength = wordLength;
1903-
if (lines >= 3) break; // Maximum 3 lines
1904-
} else {
1905-
currentLineLength += wordLength + (currentLineLength > 0 ? 1 : 0); // +1 for space
1906-
}
1907-
}
1908-
1909-
lines = Math.min(lines, 3); // Maximum 3 lines
1910-
1911-
// Calculate additional height needed for multiple lines (16px per extra line)
1912-
const additionalHeight = (lines - 1) * 16;
1913-
const finalHeight = baseDimensions.height + additionalHeight;
1914-
1915-
return {
1916-
width: width,
1917-
height: finalHeight,
1918-
titleLines: lines,
1919-
maxCharsPerLine: maxCharsPerLine
1920-
};
1921-
};
1922-
1996+
// getNodeDimensions is now defined outside and shared with updateVisualizationData
1997+
19231998
// Main node rectangle (dark theme background)
19241999
nodeElements.append('rect')
19252000
.attr('class', (d: WorkItem) => {
@@ -3269,13 +3344,14 @@ export function InteractiveGraphVisualization({ onResetLayout }: InteractiveGrap
32693344
window.addEventListener('resize', handleResize);
32703345
return () => window.removeEventListener('resize', handleResize);
32713346
}, [
3272-
nodes.length,
3273-
validatedEdges.length,
3347+
nodes.length,
3348+
validatedEdges.length,
32743349
currentGraph?.id, // Re-init when graph changes
32753350
reinitTrigger, // Manual trigger
32763351
loading, // Re-init when loading completes
3277-
edgesLoading // Re-init when edges loading completes
3278-
// Removed JSON.stringify dependency - it was triggering reinitialization on every node property change
3352+
edgesLoading, // Re-init when edges loading completes
3353+
// Track node property changes for selective updates (only titles, descriptions, types)
3354+
nodes.map(n => `${n.id}:${n.title}:${n.description}:${n.type}:${n.status}`).join(',')
32793355
]);
32803356

32813357
// Manual reinitialization function (expose globally for debugging)
@@ -4045,26 +4121,30 @@ export function InteractiveGraphVisualization({ onResetLayout }: InteractiveGrap
40454121

40464122

40474123
{/* Work Item Details Modal */}
4048-
{showNodeDetailsModal && selectedNode && (
4049-
<NodeDetailsModal
4050-
isOpen={showNodeDetailsModal}
4051-
onClose={() => {
4052-
setShowNodeDetailsModal(false);
4053-
setSelectedNode(null);
4054-
}}
4055-
node={selectedNode}
4056-
edges={validatedEdges}
4057-
nodes={validatedNodes}
4058-
onConnectToExisting={(node) => {
4059-
setShowNodeDetailsModal(false);
4060-
handleConnectToExistingNodes(node);
4061-
}}
4062-
onDisconnect={(node) => {
4063-
setShowNodeDetailsModal(false);
4064-
handleDisconnectNodes(node);
4065-
}}
4066-
/>
4067-
)}
4124+
{showNodeDetailsModal && selectedNode && (() => {
4125+
// Always get fresh node data by ID from validatedNodes (synced with Apollo cache)
4126+
const freshNode = validatedNodes.find(n => n.id === selectedNode.id) || selectedNode;
4127+
return (
4128+
<NodeDetailsModal
4129+
isOpen={showNodeDetailsModal}
4130+
onClose={() => {
4131+
setShowNodeDetailsModal(false);
4132+
setSelectedNode(null);
4133+
}}
4134+
node={freshNode}
4135+
edges={validatedEdges}
4136+
nodes={validatedNodes}
4137+
onConnectToExisting={(node) => {
4138+
setShowNodeDetailsModal(false);
4139+
handleConnectToExistingNodes(node);
4140+
}}
4141+
onDisconnect={(node) => {
4142+
setShowNodeDetailsModal(false);
4143+
handleDisconnectNodes(node);
4144+
}}
4145+
/>
4146+
);
4147+
})()}
40684148

40694149
{/* Fullscreen toggle removed - fullscreen mode abandoned */}
40704150

0 commit comments

Comments
 (0)