[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 · 752518b · 752518b
1 parent c382016
commit 752518b
Show file tree

Hide file tree

Showing 10 changed files with 217 additions and 21 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/integration-tests/cypress/e2e/project/diagrams/collapse.cy.ts b/integration-tests/cypress/e2e/project/diagrams/collapse.cy.ts
@@ -69,8 +69,6 @@ describe('Diagram - collapsible node', () => {
         const studio = new Studio();
         studio.createProjectFromDomain('Cypress - Studio Instance', domainName, 'Root').then((res) => {
           instanceProjectId = res.projectId;
-
-          new Explorer().createRepresentation('Root', `${domainName} Diagram Description`, 'diagram');
         });
       });
 
@@ -84,17 +82,18 @@ describe('Diagram - collapsible node', () => {
         explorer.createObject('Entity1', 'Relation Entity2');
         explorer.select('Entity1');
         details.getTextField('Name').type('Entity1{enter}');
+        explorer.createRepresentation('Root', `${domainName} Diagram Description`, 'diagram');
         diagram.fitToScreen();
         diagram
           .getNodes('diagram', 'Entity1')
-          .findByTestId('Label - Entity1')
+          .findByTestId('Label bottom separator - Entity1')
           .should('have.css', 'border-bottom', '1px solid rgb(51, 176, 195)');
         diagram.getNodes('diagram', 'Entity1').click('top');
         diagram.getPalette().should('exist');
         diagram.getPalette().findByTestId('Collapse - Tool').click();
         diagram
           .getNodes('diagram', 'Entity1')
-          .findByTestId('Label - Entity1')
+          .findByTestId('Label bottom separator - Entity1')
           .should('have.css', 'border-bottom-width', '0px');
       });
     });

diff --git a/integration-tests/cypress/e2e/project/diagrams/diagram-label.cy.ts b/integration-tests/cypress/e2e/project/diagrams/diagram-label.cy.ts
@@ -402,4 +402,154 @@ describe('Diagram - inside outside labels', () => {
       });
     });
   });
+  context('Given a studio with a top header label node', () => {
+    let studioProjectId: string = '';
+    let domainName: string = '';
+
+    before(() => {
+      cy.createProjectFromTemplate('studio-template').then((res) => {
+        const payload = res.body.data.createProjectFromTemplate;
+        if (isCreateProjectFromTemplateSuccessPayload(payload)) {
+          const projectId = payload.project.id;
+          studioProjectId = projectId;
+
+          const project = new Project();
+          project.visit(projectId);
+          project.disableDeletionConfirmationDialog();
+
+          const explorer = new Explorer();
+          const details = new Details();
+          explorer.expand('DomainNewModel');
+          cy.get('[title="domain::Domain"]').then(($div) => {
+            domainName = $div.data().testid;
+            explorer.expand(domainName);
+            explorer.createObject('Entity1', 'Relation');
+            details.getCheckBox('Containment').check();
+            details.openReferenceWidgetOptions('Target Type');
+            details.selectReferenceWidgetOption('Entity2');
+            explorer.expand('ViewNewModel');
+            explorer.expand('View');
+            explorer.expand(`${domainName} Diagram Description`);
+            explorer.select('Entity1 Node');
+            details.getCheckBox('Collapsible').check();
+            details.openReferenceWidgetOptions('Reused Child Node Descriptions');
+            details.selectReferenceWidgetOption('Entity2 Node');
+            explorer.expand('Entity1 Node');
+            explorer.select('aql:self.name');
+            details.getRadioOption('Position', 'TOP_LEFT').click();
+            explorer.expand('aql:self.name');
+            explorer.select('InsideLabelStyle');
+            details.getCheckBox('With Header').check();
+            details.getCheckBox('Display Header Separator').check();
+          });
+        }
+      });
+    });
+
+    after(() => cy.deleteProject(studioProjectId));
+    context('When we create a new instance project', () => {
+      let instanceProjectId: string = '';
+
+      beforeEach(() => {
+        const studio = new Studio();
+        studio.createProjectFromDomain('Cypress - Studio Instance', domainName, 'Root').then((res) => {
+          instanceProjectId = res.projectId;
+        });
+      });
+
+      afterEach(() => cy.deleteProject(instanceProjectId));
+
+      it('Then the separator is display under the label', () => {
+        const explorer = new Explorer();
+        const diagram = new Diagram();
+        const details = new Details();
+        explorer.createObject('Root', 'Entity1s Entity1');
+        explorer.createObject('Entity1', 'Relation Entity2');
+        explorer.select('Entity1');
+        details.getTextField('Name').type('Entity1{enter}');
+        explorer.createRepresentation('Root', `${domainName} Diagram Description`, 'diagram');
+        diagram.fitToScreen();
+        diagram
+          .getNodes('diagram', 'Entity1')
+          .findByTestId('Label bottom separator - Entity1')
+          .should('have.css', 'border-bottom', '1px solid rgb(51, 176, 195)');
+        diagram.getNodes('diagram', 'Entity1').findByTestId('Label top separator - Entity1').should('not.exist');
+      });
+    });
+  });
+  context('Given a studio with a bottom header label node', () => {
+    let studioProjectId: string = '';
+    let domainName: string = '';
+
+    before(() => {
+      cy.createProjectFromTemplate('studio-template').then((res) => {
+        const payload = res.body.data.createProjectFromTemplate;
+        if (isCreateProjectFromTemplateSuccessPayload(payload)) {
+          const projectId = payload.project.id;
+          studioProjectId = projectId;
+
+          const project = new Project();
+          project.visit(projectId);
+          project.disableDeletionConfirmationDialog();
+
+          const explorer = new Explorer();
+          const details = new Details();
+          explorer.expand('DomainNewModel');
+          cy.get('[title="domain::Domain"]').then(($div) => {
+            domainName = $div.data().testid;
+            explorer.expand(domainName);
+            explorer.createObject('Entity1', 'Relation');
+            details.getCheckBox('Containment').check();
+            details.openReferenceWidgetOptions('Target Type');
+            details.selectReferenceWidgetOption('Entity2');
+            explorer.expand('ViewNewModel');
+            explorer.expand('View');
+            explorer.expand(`${domainName} Diagram Description`);
+            explorer.select('Entity1 Node');
+            details.getCheckBox('Collapsible').check();
+            details.openReferenceWidgetOptions('Reused Child Node Descriptions');
+            details.selectReferenceWidgetOption('Entity2 Node');
+            explorer.expand('Entity1 Node');
+            explorer.select('aql:self.name');
+            details.getRadioOption('Position', 'BOTTOM_RIGHT').click();
+            explorer.expand('aql:self.name');
+            explorer.select('InsideLabelStyle');
+            details.getCheckBox('With Header').check();
+            details.getCheckBox('Display Header Separator').check();
+          });
+        }
+      });
+    });
+
+    after(() => cy.deleteProject(studioProjectId));
+    context('When we create a new instance project', () => {
+      let instanceProjectId: string = '';
+
+      beforeEach(() => {
+        const studio = new Studio();
+        studio.createProjectFromDomain('Cypress - Studio Instance', domainName, 'Root').then((res) => {
+          instanceProjectId = res.projectId;
+        });
+      });
+
+      afterEach(() => cy.deleteProject(instanceProjectId));
+
+      it('Then the separator is display under the label', () => {
+        const explorer = new Explorer();
+        const diagram = new Diagram();
+        const details = new Details();
+        explorer.createObject('Root', 'Entity1s Entity1');
+        explorer.createObject('Entity1', 'Relation Entity2');
+        explorer.select('Entity1');
+        details.getTextField('Name').type('Entity1{enter}');
+        explorer.createRepresentation('Root', `${domainName} Diagram Description`, 'diagram');
+        diagram.fitToScreen();
+        diagram
+          .getNodes('diagram', 'Entity1')
+          .findByTestId('Label top separator - Entity1')
+          .should('have.css', 'border-top', '1px solid rgb(51, 176, 195)');
+        diagram.getNodes('diagram', 'Entity1').findByTestId('Label bottom separator - Entity1').should('not.exist');
+      });
+    });
+  });
 });
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,20 @@ 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 data-testid={`Label top separator - ${label.text}`} 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 data-testid={`Label bottom separator - ${label.text}`} 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;