feat(edges): aggregate cross-group edges into exit/bridge/entry paths by jpinsonneau · Pull Request #320 · patternfly/react-topology

jpinsonneau · 2026-07-23T16:52:02Z

Summary

Extends createAggregateEdges with a groupEdges option that splits cross-group leaf edges into exit → bridge → entry segments and merges parallel bridges between the same parent groups (addresses Topology - Aggregating edges between node groups #17 / PF-1994).
Preserves existing collapsedGroups aggregation behavior; adds leaf-id tracking (aggregatedEdgeIds, count, optional label carry onto the bridge).
Adds unit coverage and an Aggregate Edges demo (selection by leaf path, custom labels vs summed metric tags).

Test plan

yarn test packages/module/src/utils/__tests__/createAggregateEdges.spec.ts
Run demo app → Aggregate Edges
- Toggle Aggregate edges between groups: cross-group clutter collapses to shared bridges with stubs
- Click an exit stub (e.g. 2-3): selects that leaf’s path only, not sibling leaves on the same bridge
- Click the bridge: selects all stubs for leaves on that bridge
- Collapse Group 2 & Subgroup 3: remapping + aggregation still works
- Custom edge labels appear on the bridge without being replaced by merge count
- Metric tags (summed Bps) show a single summed rate on the bridge
Collapsible Groups demo still behaves as before

Made with Cursor

jeff-phillips-18

Thanks @jpinsonneau. Looking real good. A few suggestions.

jpinsonneau · 2026-07-27T13:20:51Z

Thanks for the feedback @jeff-phillips-18 🥳

Split demo into packages/demo-app-ts/src/demos/aggregateEdges/:

AggregateEdges.tsx - main demo
AggregateEdge.tsx - exit/bridge/entry rendering + selection
LabeledDefaultEdge.tsx - leaf edges with labels/tags
AggregateGroup.tsx - DefaultGroup with built-in collapse
model.ts, DemoContext.tsx

Replaced DemoDefaultGroup + “Collapse Group 2 & Subgroup 3” checkbox with collapsible DefaultGroup; collapse rebuilds aggregates from leaf edges via context

jeff-phillips-18 · 2026-07-27T13:56:39Z

@jpinsonneau I'm seeing a couple of issues:

installHook.js:1 [MobX] Since strict-mode is enabled, changing (observed) observable values without using an action is not allowed. Tried to modify: BaseNode@96.collapsed

BaseElement.ts:89 Uncaught Error: GraphElement with ID 'aggregate_entry_Group 2_21_Group 1__Group 2' has no controller.

For the first, it looks like an existing issue in DefaultGroupInner. The handleCollapse function should be wrapped in an action.

The second issue looks to be due to handling the collapsed state in the view. I don't think that is needed as it is held by by the collapsed value in the NodeModel.

However, after making those changes, I am seeing an odd edge arrow after collapsing:

jpinsonneau · 2026-07-27T14:06:42Z

Thanks @jeff-phillips-18 — addressed in the latest commit:

MobX strict mode — DefaultGroup handleCollapse now wraps setDimensions / setCollapsed in action().
No controller / view collapsed state — removed the React collapsedIds mirror. Collapse stays on the Node; onCollapseChange only rebuilds aggregates by reading live isCollapsed() from the controller.
Odd arrows after collapse — reused bridge/stub edges were keeping setStartPoint/setEndPoint overrides from the expanded hull. Those are cleared on rebuild and when bounds change so terminals re-anchor to the collapsed shape.

jeff-phillips-18 · 2026-07-27T21:43:36Z

When I select the edge from 2-1 that goes to 3-1 and 3-2, it highlights the aggregated edges showing the path from 2-2 to both 3-1 and 3-2 connected first to subgroup 3. But, it is highlighting the aggregated edge between Group 2 and Subgroup 3 which is bidirectional so the flow looks like it is coming back to Group 2 but it really isn't. That arrow head should not be highlighted, it makes it hard to understand the correct flow:

You can see that there is no flow back to 2-2 from Subgroup 3:

jpinsonneau · 2026-07-28T09:25:32Z

When I select the edge from 2-1 that goes to 3-1 and 3-2, it highlights the aggregated edges showing the path from 2-2 to both 3-1 and 3-2 connected first to subgroup 3. But, it is highlighting the aggregated edge between Group 2 and Subgroup 3 which is bidirectional so the flow looks like it is coming back to Group 2 but it really isn't. That arrow head should not be highlighted, it makes it hard to understand the correct flow:

Good catch @jeff-phillips-18

I have fixed that case in 4da96ea

jeff-phillips-18 · 2026-07-28T12:08:02Z

These new edges created with roles will rely on the application creating an edge component very much like the AggregateEdge component in the demo. I think we should create a DefaultAggregatedEdge component in the toolkit that handles much of what AggregateEdge is doing (keeping out the demo related things like useAggregateEdgesDemo and its values.

I'm still not exactly sure what the snapGeneration is doing and I'm not sure how that could be handled by the DefaultAggregatedEdge component.

jpinsonneau · 2026-07-28T12:39:19Z

These new edges created with roles will rely on the application creating an edge component very much like the AggregateEdge component in the demo. I think we should create a DefaultAggregatedEdge component in the toolkit that handles much of what AggregateEdge is doing (keeping out the demo related things like useAggregateEdgesDemo and its values.

Sure, added the DefaultAggregatedEdge and an util file in 7f38f8d

I'm still not exactly sure what the snapGeneration is doing and I'm not sure how that could be handled by the DefaultAggregatedEdge component.

Aggregate edges pin endpoints with setStartPoint / setEndPoint. After animation (such in Cola) finishes or after collapse, those pins are often wrong, and bounds may not change enough for the snap effect to re-run.

Layout end: handled inside DefaultAggregatedEdge via GRAPH_LAYOUT_END_EVENT.
Collapse / model rebuild: app clears stale endpoints, then bumps snapGeneration so the effect force-resnaps (threshold 0). Demo still does that in onCollapseChange.

jeff-phillips-18

Thanks @jpinsonneau. LGTM!

jpinsonneau · 2026-07-28T16:44:43Z

Thanks @jpinsonneau. LGTM!

Thanks for your quick feedback ! I appreciate 👍

jeff-phillips-18 · 2026-07-29T16:23:07Z

+  action(() => {
+    state[SELECTION_STATE] = selectedIds;
+  })();


Why is this necessary? We shouldn't modify the controller's state directly and the following fireEvent will update the state correctly.

Sure, let me fix that

jeff-phillips-18 · 2026-07-29T16:39:16Z

+        },
+        forceSnap ? 0 : AGGREGATE_HULL_SETTLE_MS
+      );
+


@jpinsonneau Could you explain these two timer functions? Why do we need both? It looks like the timeout function is supposed to happen after the requestAnimationFrame function but on forceSnap it will occur before. Is that purposeful?

While geometry is moving we use two phases: requestAnimationFrame for a cheap approx snap (keeps stubs attached during Cola ticks), then a settle timeout (~100ms) for a precise hull snap. On forceSnap (layout end / collapse) that delay-0 race was real.

I'll skip approx and run one precise snap on requestAnimationFrame only.

jeff-phillips-18 · 2026-07-29T16:45:49Z

+        if (!edge.hasController()) {
+          return;
+        }
+        const plan = computeSnapPlan(edge, role, false);
+        if (plan) {
+          applySnapPlan(edge, plan, moveThreshold, true);
+        }


This is largely repeated below with the only change being the threshold passed to applySnapPlan. We could have one utility function that does this and just takes the edge, role, and threshold. I don't see applySnapPlan ever being called with the clearUnset parameter set to false.

Sure let me create a function and drop cleanUnser

jeff-phillips-18 · 2026-07-29T16:54:12Z

+  start?: XY;
+  end?: XY;


Suggested change

start?: XY;

end?: XY;

start: XY | null;

end: XY | null;

jeff-phillips-18 · 2026-07-29T16:54:45Z

+    const startFixed = plan.start != null;
+    const endFixed = plan.end != null;


Suggested change

const startFixed = plan.start != null;

const endFixed = plan.end != null;

const startFixed = plan.start !== null;

const endFixed = plan.end !== null;

jeff-phillips-18 · 2026-07-29T16:55:29Z

+      return;
+    }
+    const startMoved = startFixed
+      ? significantlyMoved(edge.getStartPoint(), plan.start!.x, plan.start!.y, threshold)


Suggested change

? significantlyMoved(edge.getStartPoint(), plan.start!.x, plan.start!.y, threshold)

? significantlyMoved(edge.getStartPoint(), plan.start.x, plan.start.y, threshold)

Remove all non-null assertions.

jeff-phillips-18 · 2026-07-29T16:56:13Z

+    if (!peer) {
+      return undefined;
+    }
+    const plan: SnapPlan = {};


Suggested change

const plan: SnapPlan = {};

const plan: SnapPlan = {

start: null,

end: null

};

jeff-phillips-18 · 2026-07-29T17:05:14Z

+  if (!edges?.length) {
+    return [];
+  }
+  if (!nodes?.length) {
+    return edges;
+  }


Suggested change

if (!edges?.length) {

return [];

}

if (!nodes?.length) {

return edges;

}

if (!edges?.length || !nodes?.length) {

return [];

}

If there are no nodes, we should have no edges.

jeff-phillips-18 · 2026-07-29T17:10:34Z

+    });
+    vis.fromModel(
+      getModel({
+        groupEdges: true,


Suggested change

groupEdges: true,

groupEdges: false,

Should match the initial demo context value

jeff-phillips-18 · 2026-07-29T17:13:46Z

@jpinsonneau Sorry, went through it again and found some issues I missed before. Also, could you squash all the previous commits?

jpinsonneau · 2026-07-30T08:20:14Z

@jpinsonneau Sorry, went through it again and found some issues I missed before. Also, could you squash all the previous commits?

Addressed the changes and squash into a single commit

jeff-phillips-18 · 2026-07-30T15:10:06Z

+    } else {
+      const aggregate = createSegmentEdge(aggregateEdgeType, segment, edge.id, true, edge.label, true);
+      aggregate.children = [edge.id];
+      pendingAggregates.push(aggregate);


I don't see this being used anywhere

Done alongside of some polishing about unused code / export
https://github.com/patternfly/react-topology/compare/517e755c3eb88303111f29acfbba6874147dd44d..bf8270c7f316216ad32cf13bd9fd8dcd0b530c27

Closes patternfly#17 Co-authored-by: Cursor <cursoragent@cursor.com>

github-actions · 2026-08-03T15:15:26Z

🎉 This PR is included in version 6.6.0-prerelease.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

jeff-phillips-18 reviewed Jul 27, 2026

View reviewed changes

Comment thread packages/demo-app-ts/src/demos/AggregateEdges.tsx Outdated

Comment thread packages/demo-app-ts/src/demos/AggregateEdges.tsx Outdated

Comment thread packages/demo-app-ts/src/demos/AggregateEdges.tsx Outdated

Comment thread packages/demo-app-ts/src/demos/AggregateEdges.tsx Outdated

jpinsonneau mentioned this pull request Jul 27, 2026

NETOBSERV-428 aggregate topology edges between groups netobserv/netobserv-web-console#1618

Open

14 tasks

jpinsonneau force-pushed the feat/aggregate-group-edges branch 2 times, most recently from 4879125 to e4dd81a Compare July 27, 2026 14:20

jeff-phillips-18 reviewed Jul 27, 2026

View reviewed changes

Comment thread packages/demo-app-ts/src/demos/aggregateEdges/AggregateEdges.tsx

jeff-phillips-18 reviewed Jul 28, 2026

View reviewed changes

Comment thread packages/module/src/components/edges/DefaultAggregatedEdge.tsx Outdated

jeff-phillips-18 reviewed Jul 28, 2026

View reviewed changes

Comment thread packages/module/src/components/edges/DefaultAggregatedEdge.tsx Outdated

Comment thread packages/module/src/components/edges/DefaultAggregatedEdge.tsx

jeff-phillips-18 approved these changes Jul 28, 2026

View reviewed changes

jeff-phillips-18 requested changes Jul 29, 2026

View reviewed changes

jpinsonneau force-pushed the feat/aggregate-group-edges branch from 321ab9f to 517e755 Compare July 30, 2026 08:19

jpinsonneau requested a review from jeff-phillips-18 July 30, 2026 08:20

jeff-phillips-18 reviewed Jul 30, 2026

View reviewed changes

feat(edges): aggregate cross-group edges into exit/bridge/entry paths

bf8270c

Closes patternfly#17 Co-authored-by: Cursor <cursoragent@cursor.com>

jpinsonneau force-pushed the feat/aggregate-group-edges branch from 517e755 to bf8270c Compare July 31, 2026 09:29

jpinsonneau requested a review from jeff-phillips-18 August 3, 2026 08:03

jeff-phillips-18 approved these changes Aug 3, 2026

View reviewed changes

jeff-phillips-18 merged commit 74d7dab into patternfly:main Aug 3, 2026
9 checks passed

github-actions Bot added the released on @prerelease label Aug 3, 2026

		const startFixed = plan.start != null;
		const endFixed = plan.end != null;

	? significantlyMoved(edge.getStartPoint(), plan.start!.x, plan.start!.y, threshold)
	? significantlyMoved(edge.getStartPoint(), plan.start.x, plan.start.y, threshold)

Uh oh!

Conversation

jpinsonneau commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

jeff-phillips-18 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jpinsonneau commented Jul 27, 2026

Uh oh!

jeff-phillips-18 commented Jul 27, 2026

Uh oh!

jpinsonneau commented Jul 27, 2026

Uh oh!

Uh oh!

jeff-phillips-18 commented Jul 27, 2026

Uh oh!

jpinsonneau commented Jul 28, 2026

Uh oh!

jeff-phillips-18 commented Jul 28, 2026

Uh oh!

jpinsonneau commented Jul 28, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jeff-phillips-18 left a comment

Choose a reason for hiding this comment

Uh oh!

jpinsonneau commented Jul 28, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeff-phillips-18 commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jpinsonneau commented Jul 30, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Aug 3, 2026

Uh oh!

Reviewers

Assignees

Labels

jpinsonneau commented Jul 23, 2026 •

edited

Loading

jeff-phillips-18 commented Jul 29, 2026 •

edited

Loading