Fix group labels, revert breaking change (#8299)

Author: jeff-phillips-18

packages/react-integration/demo-app-ts/src/components/demos/TopologyDemo/PipelineLayout.tsx

@@ -15,7 +15,9 @@ import {
   GRAPH_LAYOUT_END_EVENT,
   getSpacerNodes,
   getEdgesFromNodes,
-  DEFAULT_EDGE_TYPE
+  DEFAULT_EDGE_TYPE,
+  DEFAULT_SPACER_NODE_TYPE,
+  DEFAULT_FINALLY_NODE_TYPE
 } from '@patternfly/react-topology';
 import '@patternfly/react-styles/css/components/Topology/topology-components.css';
 import pipelineComponentFactory, { GROUPED_EDGE_TYPE } from './components/pipelineComponentFactory';
@@ -49,9 +51,14 @@ const TopologyPipelineLayout: React.FC = () => {
   React.useEffect(() => {
     const spacerNodes = getSpacerNodes(pipelineNodes);
     const nodes = [...pipelineNodes, ...spacerNodes];
+    const edgeType = showGroups ? GROUPED_EDGE_TYPE : DEFAULT_EDGE_TYPE;
     const edges = getEdgesFromNodes(
       nodes.filter(n => !n.group),
-      showGroups ? GROUPED_EDGE_TYPE : DEFAULT_EDGE_TYPE
+      DEFAULT_SPACER_NODE_TYPE,
+      edgeType,
+      edgeType,
+      [DEFAULT_FINALLY_NODE_TYPE],
+      edgeType
     );
 
     controller.fromModel(

packages/react-integration/demo-app-ts/src/components/demos/TopologyDemo/useDemoPipelineNodes.tsx

@@ -156,7 +156,7 @@ export const useDemoPipelineNodes = (
           type: 'task-group',
           children: parallelTasks.map(t => t.id),
           group: true,
-          style: { padding: [15, 15] }
+          label: 'Parallel tasks'
         });
       }
     }
@@ -184,8 +184,7 @@ export const useDemoPipelineNodes = (
       id: 'finally-group',
       type: 'finally-group',
       children: finallyNodes.map(n => n.id),
-      group: true,
-      style: { padding: [15, 15] }
+      group: true
     };
 
     if (showGroups) {
@@ -197,7 +196,8 @@ export const useDemoPipelineNodes = (
               id: `group-${task.data.columnGroup}`,
               type: 'task-group',
               children: [],
-              group: true
+              group: true,
+              label: `Group ${task.data.columnGroup}`
             };
             acc.push(taskGroup);
           }

packages/react-topology/src/pipelines/components/groups/DefaultTaskGroup.tsx

@@ -6,8 +6,8 @@ import CollapseIcon from '@patternfly/react-icons/dist/esm/icons/compress-alt-ic
 import NodeLabel from '../../../components/nodes/labels/NodeLabel';
 import { Layer } from '../../../components/layers';
 import { GROUPS_LAYER } from '../../../const';
-import { useCombineRefs, useHover } from '../../../utils';
-import { BadgeLocation, isGraph, Node } from '../../../types';
+import { maxPadding, useCombineRefs, useHover } from '../../../utils';
+import { BadgeLocation, isGraph, LabelPosition, Node, NodeStyle } from '../../../types';
 import {
   useDragNode,
   WithContextMenuProps,
@@ -35,6 +35,7 @@ type DefaultTaskGroupProps = {
   badgeBorderColor?: string;
   badgeClassName?: string;
   badgeLocation?: BadgeLocation;
+  labelOffset?: number; // Space between the label and the group
   labelIconClass?: string; // Icon to show in label
   labelIcon?: string;
   labelIconPadding?: number;
@@ -63,6 +64,7 @@ const DefaultTaskGroup: React.FunctionComponent<DefaultTaskGroupProps> = ({
   badgeBorderColor,
   badgeClassName,
   badgeLocation,
+  labelOffset = 17,
   labelIconClass,
   labelIcon,
   labelIconPadding,
@@ -73,6 +75,7 @@ const DefaultTaskGroup: React.FunctionComponent<DefaultTaskGroupProps> = ({
   const dragLabelRef = useDragNode()[1];
   const refs = useCombineRefs<SVGPathElement>(hoverRef, dragNodeRef);
   const isHover = hover !== undefined ? hover : hovered;
+  const labelPosition = element.getLabelPosition();
 
   let parent = element.getParent();
   let altGroup = false;
@@ -82,16 +85,40 @@ const DefaultTaskGroup: React.FunctionComponent<DefaultTaskGroupProps> = ({
   }
 
   const children = element.getNodes().filter(c => c.isVisible());
+
+  // cast to number and coerce
+  const padding = maxPadding(element.getStyle<NodeStyle>().padding ?? 17);
+
+  const { minX, minY, maxX, maxY } = children.reduce(
+    (acc, child) => {
+      const bounds = child.getBounds();
+      return {
+        minX: Math.min(acc.minX, bounds.x - padding),
+        minY: Math.min(acc.minY, bounds.y - padding),
+        maxX: Math.max(acc.maxX, bounds.x + bounds.width + padding),
+        maxY: Math.max(acc.maxY, bounds.y + bounds.height + padding)
+      };
+    },
+    { minX: Infinity, minY: Infinity, maxX: 0, maxY: 0 }
+  );
+
+  const [labelX, labelY] = React.useMemo(() => {
+    if (!showLabel || !(label || element.getLabel())) {
+      return [0, 0];
+    }
+    switch (labelPosition) {
+      case LabelPosition.right:
+        return [maxX + labelOffset, minY + (maxY - minY) / 2];
+      case LabelPosition.bottom:
+      default:
+        return [minX + (maxX - minX) / 2, maxY + labelOffset];
+    }
+  }, [element, label, labelOffset, labelPosition, maxX, maxY, minX, minY, showLabel]);
+
   if (children.length === 0) {
     return null;
   }
 
-  const bounds = element.getBounds();
-  const minX = bounds.x;
-  const minY = bounds.y;
-  const maxX = bounds.x + bounds.width;
-  const maxY = bounds.y + bounds.height;
-
   const groupClassName = css(
     styles.topologyGroup,
     className,
@@ -121,8 +148,9 @@ const DefaultTaskGroup: React.FunctionComponent<DefaultTaskGroupProps> = ({
       {showLabel && (label || element.getLabel()) && (
         <NodeLabel
           className={styles.topologyGroupLabel}
-          x={(maxX - minX) / 2}
-          y={maxY + 17}
+          x={labelX}
+          y={labelY}
+          position={labelPosition}
           paddingX={8}
           paddingY={5}
           dragRef={dragNodeRef ? dragLabelRef : undefined}

packages/react-topology/src/pipelines/utils/utils.ts

@@ -114,9 +114,11 @@ export const getSpacerNodes = (
 
 export const getEdgesFromNodes = (
   nodes: PipelineNodeModel[],
-  edgeType = DEFAULT_EDGE_TYPE,
   spacerNodeType = DEFAULT_SPACER_NODE_TYPE,
-  finallyNodeTypes: string[] = [DEFAULT_FINALLY_NODE_TYPE]
+  edgeType = DEFAULT_EDGE_TYPE,
+  spacerEdgeType = DEFAULT_EDGE_TYPE,
+  finallyNodeTypes: string[] = [DEFAULT_FINALLY_NODE_TYPE],
+  finallyEdgeType = DEFAULT_EDGE_TYPE
 ): EdgeModel[] => {
   const edges: EdgeModel[] = [];
 
@@ -135,7 +137,7 @@ export const getEdgesFromNodes = (
       if (node && !finallyNodes.includes(node)) {
         edges.push({
           id: `${sourceId}-${spacer.id}`,
-          type: edgeType,
+          type: spacerEdgeType,
           source: sourceId,
           target: spacer.id
         });
@@ -150,7 +152,7 @@ export const getEdgesFromNodes = (
       if (spacer) {
         edges.push({
           id: `${spacer.id}-${node.id}`,
-          type: edgeType,
+          type: spacerEdgeType,
           source: spacer.id,
           target: node.id
         });
@@ -172,15 +174,15 @@ export const getEdgesFromNodes = (
     finallyNodes.forEach(finallyNode => {
       edges.push({
         id: `${finallyId}-${finallyNode.id}`,
-        type: edgeType,
+        type: spacerEdgeType,
         source: finallyId,
         target: finallyNode.id
       });
     });
     lastTasks.forEach(lastTaskNode => {
       edges.push({
         id: `${lastTaskNode.id}-${finallyId}`,
-        type: edgeType,
+        type: spacerEdgeType,
         source: lastTaskNode.id,
         target: finallyId
       });
@@ -190,7 +192,7 @@ export const getEdgesFromNodes = (
     lastTasks.forEach(lastTaskNode => {
       edges.push({
         id: `finallyId-${lastTaskNode.id}-${finallyNodes[0].id}`,
-        type: edgeType,
+        type: finallyEdgeType,
         source: lastTaskNode.id,
         target: finallyNodes[0].id
       });