updates per review comments

frontend/packages/topology/src/components/ElementWrapper.tsx

@@ -7,19 +7,13 @@ type ElementWrapperProps = {
   element: GraphElement;
 };
 
-// in a separate component so that changes to interaction handlers do not re-render children
+// in a separate component so that changes to behaviors do not re-render children
 const ElementComponent: React.FC<ElementWrapperProps> = observer(({ element }) => {
   const Component = React.useMemo(
     () => element.getController().getComponent(element.getKind(), element.getType()),
     [element],
   );
-  if (isEdge(element)) {
-    const source = element.getSourceAnchorNode();
-    const target = element.getTargetAnchorNode();
-    if ((source && !source.isVisible()) || (target && !target.isVisible())) {
-      return null;
-    }
-  }
+
   return (
     <ElementContext.Provider value={element}>
       <Component {...element.getState()} element={element} />
@@ -50,6 +44,14 @@ const ElementWrapper: React.FC<ElementWrapperProps> = observer(({ element }) =>
   if (!element.isVisible()) {
     return null;
   }
+
+  if (isEdge(element)) {
+    const source = element.getSourceAnchorNode();
+    const target = element.getTargetAnchorNode();
+    if ((source && !source.isVisible()) || (target && !target.isVisible())) {
+      return null;
+    }
+  }
   const commonProps = {
     [`data-id`]: element.getId(),
     [`data-kind`]: element.getKind(),

frontend/packages/topology/src/elements/BaseEdge.ts

@@ -1,7 +1,7 @@
 import { observable, computed } from 'mobx';
 import Point from '../geom/Point';
 import { Edge, Node, EdgeModel, ModelKind, AnchorEnd, Anchor } from '../types';
-import { getClosestVisibleParent } from '../utils';
+import { getTopCollapsedParent } from '../utils';
 import BaseElement from './BaseElement';
 
 export default class BaseEdge<E extends EdgeModel = EdgeModel, D = any> extends BaseElement<E, D>
@@ -22,15 +22,13 @@ export default class BaseEdge<E extends EdgeModel = EdgeModel, D = any> extends
   private endPoint?: Point;
 
   @computed
-  private get sourceAnchor(): Anchor | null {
-    const source = this.getSourceAnchorNode();
-    return source && source.getAnchor(AnchorEnd.source, this.getType());
+  private get sourceAnchor(): Anchor {
+    return this.getSourceAnchorNode().getAnchor(AnchorEnd.source, this.getType());
   }
 
   @computed
-  private get targetAnchor(): Anchor | null {
-    const target = this.getTargetAnchorNode();
-    return target && target.getAnchor(AnchorEnd.target, this.getType());
+  private get targetAnchor(): Anchor {
+    return this.getTargetAnchorNode().getAnchor(AnchorEnd.target, this.getType());
   }
 
   getKind(): ModelKind {
@@ -59,18 +57,18 @@ export default class BaseEdge<E extends EdgeModel = EdgeModel, D = any> extends
     this.target = target;
   }
 
-  getSourceAnchorNode(): Node | null {
+  getSourceAnchorNode(): Node {
     if (!this.source) {
       throw new Error(`Edge with ID '${this.getId()}' has no source.`);
     }
-    return getClosestVisibleParent(this.source);
+    return getTopCollapsedParent(this.source);
   }
 
-  getTargetAnchorNode(): Node | null {
+  getTargetAnchorNode(): Node {
     if (!this.target) {
       throw new Error(`Edge with ID '${this.getId()}' has no target.`);
     }
-    return getClosestVisibleParent(this.target);
+    return getTopCollapsedParent(this.target);
   }
 
   getBendpoints(): Point[] {
@@ -105,9 +103,9 @@ export default class BaseEdge<E extends EdgeModel = EdgeModel, D = any> extends
     } else if (this.endPoint) {
       referencePoint = this.endPoint;
     } else {
-      referencePoint = this.targetAnchor ? this.targetAnchor.getReferencePoint() : Point.EMPTY;
+      referencePoint = this.targetAnchor.getReferencePoint();
     }
-    return this.sourceAnchor ? this.sourceAnchor.getLocation(referencePoint) : Point.EMPTY;
+    return this.sourceAnchor.getLocation(referencePoint);
   }
 
   setStartPoint(x?: number, y?: number): void {
@@ -129,9 +127,9 @@ export default class BaseEdge<E extends EdgeModel = EdgeModel, D = any> extends
     } else if (this.startPoint) {
       referencePoint = this.startPoint;
     } else {
-      referencePoint = this.sourceAnchor ? this.sourceAnchor.getReferencePoint() : Point.EMPTY;
+      referencePoint = this.sourceAnchor.getReferencePoint();
     }
-    return this.targetAnchor ? this.targetAnchor.getLocation(referencePoint) : Point.EMPTY;
+    return this.targetAnchor.getLocation(referencePoint);
   }
 
   setEndPoint(x?: number, y?: number): void {

frontend/packages/topology/src/elements/BaseElement.ts

@@ -5,7 +5,6 @@ import {
   Graph,
   GraphElement,
   isGraph,
-  Node,
   isNode,
   Controller,
   ModelKind,
@@ -129,18 +128,11 @@ export default abstract class BaseElement<E extends ElementModel = ElementModel,
   }
 
   isVisible(): boolean {
-    let { visible } = this;
-    try {
-      let parent = this.getParent();
-
-      while (visible && parent) {
-        visible = parent.isVisible() && (!isNode(parent) || !(parent as Node).isCollapsed());
-        parent = parent.getParent();
-      }
-      // eslint-disable-next-line no-empty
-    } catch (e) {}
-
-    return visible;
+    return (
+      this.visible &&
+      (!this.parent ||
+        (this.parent.isVisible() && (!isNode(this.parent) || !this.parent.isCollapsed())))
+    );
   }
 
   getData(): D | undefined {

frontend/packages/topology/src/types.ts

@@ -130,8 +130,8 @@ export interface Edge<E extends EdgeModel = EdgeModel, D = any> extends GraphEle
   setSource(source: Node): void;
   getTarget(): Node;
   setTarget(target: Node): void;
-  getSourceAnchorNode(): Node | null;
-  getTargetAnchorNode(): Node | null;
+  getSourceAnchorNode(): Node;
+  getTargetAnchorNode(): Node;
   getStartPoint(): Point;
   setStartPoint(x?: number, y?: number): void;
   getEndPoint(): Point;

frontend/packages/topology/src/utils/createAggregateEdges.ts

@@ -38,7 +38,7 @@ const createAggregateEdges = (
       const target = getDisplayedNodeForNode(edge.target, nodes);
 
       // Make sure visible is defined so that changes override what could already be in the element
-      edge.visible = edge.visible === undefined ? true : edge.visible;
+      edge.visible = 'visible' in edge ? edge.visible : true;
 
       if (source !== edge.source || target !== edge.target) {
         if (source !== target) {
@@ -51,8 +51,11 @@ const createAggregateEdges = (
           if (existing) {
             // At least one other edge, add this edge and add the aggregate edge to the edges
 
-            // Hide edges that are depicted by this aggregate edge
+            // Add this edge to the aggregate and set it not visible
             existing.children && existing.children.push(edge.id);
+            edge.visible = false;
+
+            // Hide edges that are depicted by this aggregate edge
             _.forEach(existing.children, (existingChild) => {
               const updateEdge = newEdges.find((newEdge) => newEdge.id === existingChild);
               if (updateEdge) {
@@ -80,7 +83,7 @@ const createAggregateEdges = (
               children: [edge.id],
               source,
               target,
-              id: `${source}_${target}`,
+              id: `aggregate_${source}_${target}`,
               type: aggregateEdgeType,
             };
             aggregateEdges.push(newEdge);

frontend/packages/topology/src/utils/element-utils.ts

@@ -46,6 +46,21 @@ const leafNodeElements = (nodeElements: Node | Node[] | null): Node[] => {
   return [nodeElements];
 };
 
+const getTopCollapsedParent = (node: Node): Node => {
+  let returnNode: Node = node;
+  try {
+    let parent = !isGraph(node) && node.getParent();
+    while (parent && !isGraph(parent)) {
+      if ((parent as Node).isCollapsed()) {
+        returnNode = parent as Node;
+      }
+      parent = parent.getParent();
+    }
+    // eslint-disable-next-line no-empty
+  } catch (e) {}
+  return returnNode;
+};
+
 const getClosestVisibleParent = (node: Node): Node | null => {
   if (!node) {
     return null;
@@ -102,6 +117,7 @@ const getGroupPadding = (element: GraphElement, padding = 0): number => {
 export {
   groupNodeElements,
   leafNodeElements,
+  getTopCollapsedParent,
   getClosestVisibleParent,
   getElementPadding,
   getGroupPadding,