Skip to content

Commit 4ff5436

Browse files
committed
feat(spatial): enhance port handling and node hit-testing
- Introduce `PortType.both` for bidirectional ports. - Adjust `_isPointCoveredByOtherNode` to exclude GroupNode member nodes. - Replace logical `position` with `visualPosition` for precise hit-testing. - Refactor connection validation to use port type (`isInput`/`isOutput`) directly. - Ensure GroupNode preservation behavior is customizable when empty. - Update rendering for node and group colors to use 15% opacity for subtle styling. - Enhance visual position initialization in test factories.
1 parent 1095543 commit 4ff5436

12 files changed

Lines changed: 209 additions & 86 deletions

File tree

packages/vyuh_node_flow/lib/src/editor/controller/connection_api.dart

Lines changed: 21 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -800,18 +800,6 @@ extension ConnectionApi<T, C> on NodeFlowController<T, C> {
800800
);
801801
}
802802

803-
// Cannot connect same direction ports
804-
final targetIsOutput = targetNode.outputPorts.any(
805-
(p) => p.id == targetPortId,
806-
);
807-
if (temp.isStartFromOutput == targetIsOutput) {
808-
return ConnectionValidationResult.deny(
809-
reason: targetIsOutput
810-
? 'Cannot connect output to output'
811-
: 'Cannot connect input to input',
812-
);
813-
}
814-
815803
// Get source node and port
816804
final sourceNode = _nodes[temp.startNodeId];
817805
if (sourceNode == null) {
@@ -838,6 +826,25 @@ extension ConnectionApi<T, C> on NodeFlowController<T, C> {
838826
);
839827
}
840828

829+
// Cannot connect same direction ports
830+
// Use the port's actual type (isInput/isOutput) rather than which list it's in,
831+
// because PortType.both ports can act as either input or output.
832+
if (temp.isStartFromOutput) {
833+
// Connecting from output - target must accept input
834+
if (!targetPort.isInput) {
835+
return const ConnectionValidationResult.deny(
836+
reason: 'Cannot connect output to output',
837+
);
838+
}
839+
} else {
840+
// Connecting from input - target must accept output
841+
if (!targetPort.isOutput) {
842+
return const ConnectionValidationResult.deny(
843+
reason: 'Cannot connect input to input',
844+
);
845+
}
846+
}
847+
841848
// Both ports must be connectable
842849
if (!sourcePort.isConnectable) {
843850
return const ConnectionValidationResult.deny(
@@ -850,20 +857,8 @@ extension ConnectionApi<T, C> on NodeFlowController<T, C> {
850857
);
851858
}
852859

853-
// Direction compatibility
854-
if (temp.isStartFromOutput) {
855-
if (!targetPort.isInput) {
856-
return const ConnectionValidationResult.deny(
857-
reason: 'Target port cannot receive connections',
858-
);
859-
}
860-
} else {
861-
if (!targetPort.isOutput) {
862-
return const ConnectionValidationResult.deny(
863-
reason: 'Target port cannot emit connections',
864-
);
865-
}
866-
}
860+
// Note: Direction compatibility is already checked above using port.isInput/isOutput
861+
// which correctly handles PortType.both ports.
867862

868863
// Determine actual source/target
869864
final Node<T> actualSourceNode;

packages/vyuh_node_flow/lib/src/editor/controller/group_api.dart

Lines changed: 34 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -113,34 +113,50 @@ extension GroupApi<T, C> on NodeFlowController<T, C> {
113113
// ============================================================================
114114

115115
/// Moves nodes by a given delta, handling position and visual position with snapping.
116+
///
117+
/// If any of the nodes being moved is a GroupNode, its children are also
118+
/// moved recursively. This ensures nested groups work correctly - moving a
119+
/// parent group moves all descendants.
116120
void _moveNodesByDelta(Set<String> nodeIds, Offset delta) {
117-
// Check if already moving to prevent nested calls
118-
if (_isMovingGroupNodes) {
119-
return;
120-
}
121-
122121
if (nodeIds.isEmpty) return;
123122

124-
// Temporarily disable updates to prevent cycles
123+
// Set flag to prevent MobX reactions during movement
125124
_isMovingGroupNodes = true;
126125

127-
try {
128-
runInAction(() {
129-
for (final nodeId in nodeIds) {
130-
final node = _nodes[nodeId];
131-
if (node != null) {
132-
final newPosition = node.position.value + delta;
133-
134-
// Update both position and visual position
135-
node.position.value = newPosition;
136-
final snappedPosition = snapToGrid(newPosition);
137-
node.setVisualPosition(snappedPosition);
126+
// Track nodes we've already moved to prevent infinite loops
127+
final movedNodes = <String>{};
128+
129+
void moveRecursively(Set<String> ids) {
130+
for (final nodeId in ids) {
131+
// Skip if already moved
132+
if (movedNodes.contains(nodeId)) continue;
133+
movedNodes.add(nodeId);
134+
135+
final node = _nodes[nodeId];
136+
if (node == null) continue;
137+
138+
// Move this node
139+
final newPosition = node.position.value + delta;
140+
node.position.value = newPosition;
141+
final snappedPosition = snapToGrid(newPosition);
142+
node.setVisualPosition(snappedPosition);
143+
144+
// If this node is a GroupNode, recursively move its children
145+
if (node is GroupableMixin<T>) {
146+
if (node.isGroupable && node.groupedNodeIds.isNotEmpty) {
147+
moveRecursively(node.groupedNodeIds);
138148
}
139149
}
150+
}
151+
}
152+
153+
try {
154+
runInAction(() {
155+
moveRecursively(nodeIds);
140156
});
141157

142158
// Mark nodes dirty (deferred during drag)
143-
_markNodesDirty(nodeIds);
159+
_markNodesDirty(movedNodes);
144160
} finally {
145161
_isMovingGroupNodes = false;
146162
}

packages/vyuh_node_flow/lib/src/nodes/comment_node.dart

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,8 @@ class CommentNode<T> extends Node<T> with ResizableMixin<T> {
118118
Color? selectedBorderColor,
119119
double borderRadius = 4.0,
120120
}) {
121-
// Use the comment's own color with 25% opacity, filled, no outline
122-
final commentColor = this.color.withValues(alpha: 0.25);
121+
// Use the comment's own color (not the parameter) with 15% opacity
122+
final commentColor = this.color.withValues(alpha: 0.15);
123123
final rrect = RRect.fromRectAndRadius(
124124
bounds,
125125
Radius.circular(borderRadius),
@@ -138,8 +138,8 @@ class CommentNode<T> extends Node<T> with ResizableMixin<T> {
138138
required Color defaultColor,
139139
double borderRadius = 2.0,
140140
}) {
141-
// Use the comment's own color with 25% opacity, filled, no outline
142-
final commentColor = color.withValues(alpha: 0.25);
141+
// Use the comment's own color with 15% opacity for subtle appearance
142+
final commentColor = color.withValues(alpha: 0.15);
143143
final rrect = RRect.fromRectAndRadius(
144144
bounds,
145145
Radius.circular(borderRadius),

packages/vyuh_node_flow/lib/src/nodes/group_node.dart

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -131,10 +131,15 @@ class GroupNode<T> extends Node<T> with ResizableMixin<T>, GroupableMixin<T> {
131131
// Optional ports for subflow patterns
132132
super.inputPorts,
133133
super.outputPorts,
134+
// Custom widget builder for subclass rendering (e.g., LoopNode)
135+
super.widgetBuilder,
136+
// Override default auto-delete behavior when group becomes empty
137+
bool preserveWhenEmpty = false,
134138
}) : _nodeIds = ObservableSet.of(nodeIds ?? {}),
135139
_observableBehavior = Observable(behavior),
136140
_observableTitle = Observable(title),
137141
_observableColor = Observable(color),
142+
_preserveWhenEmpty = preserveWhenEmpty,
138143
super(
139144
type: 'group',
140145
layer: NodeRenderLayer.background,
@@ -143,6 +148,13 @@ class GroupNode<T> extends Node<T> with ResizableMixin<T>, GroupableMixin<T> {
143148
selectable: true,
144149
);
145150

151+
/// Whether to preserve this group when it becomes empty.
152+
///
153+
/// By default, groups with [GroupBehavior.explicit] are auto-deleted when
154+
/// all member nodes are removed. Set this to `true` to prevent auto-deletion.
155+
/// Useful for loop containers that should persist even without body nodes.
156+
final bool _preserveWhenEmpty;
157+
146158
final Observable<String> _observableTitle;
147159
final Observable<Color> _observableColor;
148160
final Observable<GroupBehavior> _observableBehavior;
@@ -321,7 +333,8 @@ class GroupNode<T> extends Node<T> with ResizableMixin<T>, GroupableMixin<T> {
321333
bool get isEmpty => behavior != GroupBehavior.bounds && _nodeIds.isEmpty;
322334

323335
@override
324-
bool get shouldRemoveWhenEmpty => behavior == GroupBehavior.explicit;
336+
bool get shouldRemoveWhenEmpty =>
337+
behavior == GroupBehavior.explicit && !_preserveWhenEmpty;
325338

326339
@override
327340
void onChildMoved(String nodeId, Offset newPosition) {
@@ -487,8 +500,8 @@ class GroupNode<T> extends Node<T> with ResizableMixin<T>, GroupableMixin<T> {
487500
Color? selectedBorderColor,
488501
double borderRadius = 4.0,
489502
}) {
490-
// Use the group's own color with 25% opacity, filled, no outline
491-
final groupColor = currentColor.withValues(alpha: 0.25);
503+
// Use the group's own color with 15% opacity for subtle appearance
504+
final groupColor = currentColor.withValues(alpha: 0.15);
492505
final rrect = RRect.fromRectAndRadius(
493506
bounds,
494507
Radius.circular(borderRadius),
@@ -507,8 +520,8 @@ class GroupNode<T> extends Node<T> with ResizableMixin<T>, GroupableMixin<T> {
507520
required Color defaultColor,
508521
double borderRadius = 2.0,
509522
}) {
510-
// Use the group's own color with 25% opacity, filled, no outline
511-
final groupColor = currentColor.withValues(alpha: 0.25);
523+
// Use the group's own color with 15% opacity for subtle appearance
524+
final groupColor = currentColor.withValues(alpha: 0.15);
512525
final rrect = RRect.fromRectAndRadius(
513526
bounds,
514527
Radius.circular(borderRadius),
@@ -539,6 +552,7 @@ class GroupNode<T> extends Node<T> with ResizableMixin<T>, GroupableMixin<T> {
539552
bool? locked,
540553
List<Port>? inputPorts,
541554
List<Port>? outputPorts,
555+
NodeWidgetBuilder<T>? widgetBuilder,
542556
}) {
543557
return GroupNode<T>(
544558
id: id ?? this.id,
@@ -555,6 +569,7 @@ class GroupNode<T> extends Node<T> with ResizableMixin<T>, GroupableMixin<T> {
555569
locked: locked ?? this.locked,
556570
inputPorts: inputPorts ?? List.from(this.inputPorts),
557571
outputPorts: outputPorts ?? List.from(this.outputPorts),
572+
widgetBuilder: widgetBuilder ?? this.widgetBuilder,
558573
);
559574
}
560575

packages/vyuh_node_flow/lib/src/nodes/node.dart

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -407,20 +407,26 @@ class Node<T> {
407407
// ===========================================================================
408408

409409
/// Checks if a point is within the node's rectangular bounds.
410+
///
411+
/// Uses [visualPosition] for accurate hit testing against where the node
412+
/// is actually rendered (which may differ from [position] due to grid snapping).
410413
bool containsPoint(Offset point) {
411414
return Rect.fromLTWH(
412-
position.value.dx,
413-
position.value.dy,
415+
visualPosition.value.dx,
416+
visualPosition.value.dy,
414417
size.value.width,
415418
size.value.height,
416419
).contains(point);
417420
}
418421

419422
/// Gets the node's bounding rectangle.
423+
///
424+
/// Uses [visualPosition] for accurate bounds that match where the node
425+
/// is actually rendered (which may differ from [position] due to grid snapping).
420426
Rect getBounds() {
421427
return Rect.fromLTWH(
422-
position.value.dx,
423-
position.value.dy,
428+
visualPosition.value.dx,
429+
visualPosition.value.dy,
424430
size.value.width,
425431
size.value.height,
426432
);

packages/vyuh_node_flow/lib/src/nodes/node_container.dart

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -187,15 +187,19 @@ class NodeContainer<T> extends StatelessWidget {
187187
),
188188

189189
// Input ports (only when LOD allows and ports exist)
190+
// Use port.isOutput to respect PortType (input, output, both)
191+
// rather than list membership
190192
if (lodVisibility.showPorts && node.inputPorts.isNotEmpty)
191193
...node.inputPorts.map(
192-
(port) => _buildPort(context, port, false),
194+
(port) => _buildPort(context, port, port.isOutput),
193195
),
194196

195197
// Output ports (only when LOD allows and ports exist)
198+
// Use port.isOutput to respect PortType (input, output, both)
199+
// rather than list membership
196200
if (lodVisibility.showPorts && node.outputPorts.isNotEmpty)
197201
...node.outputPorts.map(
198-
(port) => _buildPort(context, port, true),
202+
(port) => _buildPort(context, port, port.isOutput),
199203
),
200204

201205
// Resize handles (shown when selected and resizable)
@@ -227,7 +231,7 @@ class NodeContainer<T> extends StatelessWidget {
227231
Widget _buildPort(BuildContext context, Port port, bool isOutput) {
228232
final theme = controller.theme ?? NodeFlowTheme.light;
229233
final portTheme = theme.portTheme;
230-
final isConnected = _isPortConnected(port.id, isOutput);
234+
final isConnected = _isPortConnected(port);
231235

232236
// Get the visual position from the Node model
233237
final effectivePortSize = port.size ?? portTheme.size;
@@ -281,15 +285,26 @@ class NodeContainer<T> extends StatelessWidget {
281285
}
282286

283287
/// Checks if a port is connected by examining the connections list.
284-
bool _isPortConnected(String portId, bool isOutput) {
288+
///
289+
/// For [PortType.both] ports, checks both source and target connections
290+
/// since these ports can act as either input or output.
291+
bool _isPortConnected(Port port) {
285292
return connections.any((connection) {
286-
if (isOutput) {
293+
// For bidirectional ports (PortType.both), check both directions
294+
if (port.isInput && port.isOutput) {
295+
return (connection.sourceNodeId == node.id &&
296+
connection.sourcePortId == port.id) ||
297+
(connection.targetNodeId == node.id &&
298+
connection.targetPortId == port.id);
299+
}
300+
// For output-only ports, check as source
301+
if (port.isOutput) {
287302
return connection.sourceNodeId == node.id &&
288-
connection.sourcePortId == portId;
289-
} else {
290-
return connection.targetNodeId == node.id &&
291-
connection.targetPortId == portId;
303+
connection.sourcePortId == port.id;
292304
}
305+
// For input-only ports, check as target
306+
return connection.targetNodeId == node.id &&
307+
connection.targetPortId == port.id;
293308
});
294309
}
295310
}

0 commit comments

Comments
 (0)