[3624] Fix issue with header separator not spanning entire node width

Bug: #3624 Signed-off-by: Florian ROUËNÉ <[email protected]>
eclipse-sirius · Jun 13, 2024 · 39979bc · 39979bc
1 parent c382016
commit 39979bc
Show file tree

Hide file tree

Showing 8 changed files with 60 additions and 17 deletions.
diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc
@@ -54,6 +54,7 @@ More existing APIs will be migrated to this new common pattern.
 - https://github.com/eclipse-sirius/sirius-web/issues/3575[#3575] [core] Restore support for studio palette colors
 - https://github.com/eclipse-sirius/sirius-web/issues/3582[#3582] [tree] Restore the initial direct edit tree item label for the explorer
 - https://github.com/eclipse-sirius/sirius-web/issues/3611[#3611] [diagram] Fix missing creation tool image in the contextual palette
+- https://github.com/eclipse-sirius/sirius-web/issues/3624[#3624] [diagram] Fix an issue where the header separator does not fill the entire width of the node
 
 === New Features
 

diff --git a/...ages/diagrams/frontend/sirius-components-diagrams/src/converter/IconLabelNodeConverter.ts b/...ages/diagrams/frontend/sirius-components-diagrams/src/converter/IconLabelNodeConverter.ts
@@ -99,6 +99,8 @@ const toIconLabelNode = (
       isHeader: insideLabel.isHeader,
       displayHeaderSeparator: insideLabel.displayHeaderSeparator,
       overflowStrategy: insideLabel.overflowStrategy,
+      headerSeparatorStyle: {},
+      headerPosition: undefined,
     };
   }
 

diff --git a/packages/diagrams/frontend/sirius-components-diagrams/src/converter/convertLabel.ts b/packages/diagrams/frontend/sirius-components-diagrams/src/converter/convertLabel.ts
@@ -45,19 +45,29 @@ export const convertInsideLabel = (
     },
     iconURL: labelStyle.iconURL,
     overflowStrategy: gqlInsideLabel.overflowStrategy,
+    headerSeparatorStyle: {
+      width: '100%',
+    },
+    headerPosition: undefined,
   };
 
   const alignement = AlignmentMap[gqlInsideLabel.insideLabelLocation];
   if (alignement && alignement.isPrimaryVerticalAlignment) {
     if (alignement.primaryAlignment === 'TOP') {
+      if (insideLabel.isHeader) {
+        insideLabel.headerPosition = 'TOP';
+      }
       if (insideLabel.displayHeaderSeparator && hasVisibleChild) {
-        insideLabel.style.borderBottom = borderStyle;
+        insideLabel.headerSeparatorStyle.borderBottom = borderStyle;
       }
       data.style = { ...data.style, display: 'flex', flexDirection: 'column', justifyContent: 'flex-start' };
     }
     if (alignement.primaryAlignment === 'BOTTOM') {
+      if (insideLabel.isHeader) {
+        insideLabel.headerPosition = 'BOTTOM';
+      }
       if (insideLabel.displayHeaderSeparator && hasVisibleChild) {
-        insideLabel.style.borderTop = borderStyle;
+        insideLabel.headerSeparatorStyle.borderTop = borderStyle;
       }
       data.style = { ...data.style, display: 'flex', flexDirection: 'column', justifyContent: 'flex-end' };
     }

diff --git a/packages/diagrams/frontend/sirius-components-diagrams/src/renderer/DiagramRenderer.types.ts b/packages/diagrams/frontend/sirius-components-diagrams/src/renderer/DiagramRenderer.types.ts
@@ -88,8 +88,12 @@ export interface InsideLabel {
   isHeader: boolean;
   displayHeaderSeparator: boolean;
   overflowStrategy: LabelOverflowStrategy;
+  headerSeparatorStyle: React.CSSProperties;
+  headerPosition: HeaderPosition | undefined;
 }
 
+export type HeaderPosition = 'TOP' | 'BOTTOM';
+
 export type LabelOverflowStrategy = 'NONE' | 'WRAP' | 'ELLIPSIS';
 
 export interface EdgeLabel {

diff --git a/packages/diagrams/frontend/sirius-components-diagrams/src/renderer/Label.tsx b/packages/diagrams/frontend/sirius-components-diagrams/src/renderer/Label.tsx
@@ -28,6 +28,27 @@ const getOverflowStrategy = (label: EdgeLabel | InsideLabel | OutsideLabel): Lab
   return undefined;
 };
 
+const isDisplayTopHeaderSeparator = (label: EdgeLabel | InsideLabel | OutsideLabel): boolean => {
+  if ('displayHeaderSeparator' in label) {
+    return label.displayHeaderSeparator && label.headerPosition === 'BOTTOM';
+  }
+  return false;
+};
+
+const isDisplayBottomHeaderSeparator = (label: EdgeLabel | InsideLabel | OutsideLabel): boolean => {
+  if ('displayHeaderSeparator' in label) {
+    return label.displayHeaderSeparator && label.headerPosition === 'TOP';
+  }
+  return false;
+};
+
+const getHeaderSeparatorStyle = (label: EdgeLabel | InsideLabel | OutsideLabel): React.CSSProperties | undefined => {
+  if ('headerSeparatorStyle' in label) {
+    return label.headerSeparatorStyle;
+  }
+  return undefined;
+};
+
 const labelStyle = (
   theme: Theme,
   style: React.CSSProperties,
@@ -109,12 +130,16 @@ export const Label = memo(({ diagramElementId, label, faded, transform }: LabelP
       </div>
     );
   return (
-    <div
-      data-id={label.id}
-      data-testid={`Label - ${label.text}`}
-      style={labelStyle(theme, label.style, faded, transform)}
-      className="nopan">
-      {content}
-    </div>
+    <>
+      {isDisplayTopHeaderSeparator(label) && <div style={getHeaderSeparatorStyle(label)} />}
+      <div
+        data-id={label.id}
+        data-testid={`Label - ${label.text}`}
+        style={labelStyle(theme, label.style, faded, transform)}
+        className="nopan">
+        {content}
+      </div>
+      {isDisplayBottomHeaderSeparator(label) && <div style={getHeaderSeparatorStyle(label)} />}
+    </>
   );
 });
diff --git a/...rams/frontend/sirius-components-diagrams/src/renderer/layout/FreeFormNodeLayoutHandler.ts b/...rams/frontend/sirius-components-diagrams/src/renderer/layout/FreeFormNodeLayoutHandler.ts
@@ -84,8 +84,6 @@ export class FreeFormNodeLayoutHandler implements INodeLayoutHandler<FreeFormNod
 
     const nodeIndex = findNodeIndex(visibleNodes, node.id);
     const labelElement = document.getElementById(`${node.id}-label-${nodeIndex}`);
-    const withHeader: boolean = node.data.insideLabel?.isHeader ?? false;
-    const displayHeaderSeparator: boolean = node.data.insideLabel?.displayHeaderSeparator ?? false;
 
     const borderNodes = directChildren.filter((node) => node.data.isBorderNode);
     const directNodesChildren = directChildren.filter((child) => !child.data.isBorderNode);
@@ -95,7 +93,7 @@ export class FreeFormNodeLayoutHandler implements INodeLayoutHandler<FreeFormNod
       const previousNode = (previousDiagram?.nodes ?? []).find((previouseNode) => previouseNode.id === child.id);
       const previousPosition = computePreviousPosition(previousNode, child);
       const createdNode = newlyAddedNode?.id === child.id ? newlyAddedNode : undefined;
-      const headerHeightFootprint = getHeaderHeightFootprint(labelElement, withHeader, displayHeaderSeparator);
+      const headerHeightFootprint = getHeaderHeightFootprint(labelElement, node.data.insideLabel, 'TOP');
 
       if (!!createdNode) {
         child.position = createdNode.position;
@@ -128,6 +126,7 @@ export class FreeFormNodeLayoutHandler implements INodeLayoutHandler<FreeFormNod
     // Update node to layout size
     // WARN: We suppose label are always on top of children (that wrong)
     const childrenContentBox = computeNodesBox(visibleNodes, directNodesChildren); // WARN: The current content box algorithm does not take the margin of direct children (it should)
+    const footerHeightFootprint = getHeaderHeightFootprint(labelElement, node.data.insideLabel, 'BOTTOM');
     const directChildrenAwareNodeWidth = childrenContentBox.x + childrenContentBox.width + rectangularNodePadding;
     const northBorderNodeFootprintWidth = getNorthBorderNodeFootprintWidth(visibleNodes, borderNodes, previousDiagram);
     const southBorderNodeFootprintWidth = getSouthBorderNodeFootprintWidth(visibleNodes, borderNodes, previousDiagram);
@@ -146,7 +145,7 @@ export class FreeFormNodeLayoutHandler implements INodeLayoutHandler<FreeFormNod
       borderWidth * 2;
 
     // WARN: the label is not used for the height because children are already position under the label
-    const directChildrenAwareNodeHeight = childrenContentBox.y + childrenContentBox.height + rectangularNodePadding;
+    const directChildrenAwareNodeHeight = childrenContentBox.y + childrenContentBox.height + footerHeightFootprint;
     const eastBorderNodeFootprintHeight = getEastBorderNodeFootprintHeight(visibleNodes, borderNodes, previousDiagram);
     const westBorderNodeFootprintHeight = getWestBorderNodeFootprintHeight(visibleNodes, borderNodes, previousDiagram);
 

diff --git a/packages/diagrams/frontend/sirius-components-diagrams/src/renderer/layout/layoutNode.ts b/packages/diagrams/frontend/sirius-components-diagrams/src/renderer/layout/layoutNode.ts
@@ -118,15 +118,17 @@ export const getChildNodePosition = (
 
 export const getHeaderHeightFootprint = (
   labelElement: HTMLElement | null,
-  withHeader: boolean,
-  displayHeaderSeparator: boolean
+  insideLabel: InsideLabel | null,
+  headerPosition: string | null
 ): number => {
   let headerHeightFootprint = 0;
+  const withHeader: boolean = insideLabel?.isHeader ?? false;
+  const displayHeaderSeparator: boolean = insideLabel?.displayHeaderSeparator ?? false;
 
   if (!labelElement) {
     return headerHeightFootprint;
   }
-  if (withHeader) {
+  if (withHeader && insideLabel?.headerPosition === headerPosition) {
     headerHeightFootprint = labelElement.getBoundingClientRect().height;
     if (displayHeaderSeparator) {
       headerHeightFootprint += rectangularNodePadding;

diff --git a/packages/sirius-web/frontend/sirius-web/src/nodes/EllipseNodeLayoutHandler.ts b/packages/sirius-web/frontend/sirius-web/src/nodes/EllipseNodeLayoutHandler.ts
@@ -78,7 +78,7 @@ export class EllipseNodeLayoutHandler implements INodeLayoutHandler<NodeData> {
       const previousNode = (previousDiagram?.nodes ?? []).find((previouseNode) => previouseNode.id === child.id);
       const previousPosition = computePreviousPosition(previousNode, child);
       const createdNode = newlyAddedNode?.id === child.id ? newlyAddedNode : undefined;
-      const headerHeightFootprint = labelElement ? getHeaderHeightFootprint(labelElement, false, false) : 0;
+      const headerHeightFootprint = labelElement ? getHeaderHeightFootprint(labelElement, null, null) : 0;
 
       if (!!createdNode) {
         child.position = createdNode.position;