Technical debt (#59)

* Re-enable integration tests

* Add test with multiline nodes and edges

* Make a start improving handling Dimensions

* Library builds and its tests succeed

* Fix the playground

* Simplify data structure for label texts

* Typo

* Make relations between dimensions interfaces explicit

* Remove obsolete interface definition

Author: mhdirkse

projects/frank-config-layout/src/lib/dimensions.ts

@@ -0,0 +1,32 @@
+import { Dimensions } from '../public_api';
+import { EdgeLabelDimensions } from './graphics/edge-label-layouter';
+import { NodeAndEdgeDimensions } from './graphics/layout';
+import { SvgGenerationDimensions } from './graphics/svg-generator';
+import { calculateAverageFontCharacterWidth, NodeTextDimensions } from './model/text';
+
+export interface DerivedDimensions
+  extends EdgeLabelDimensions, NodeAndEdgeDimensions, SvgGenerationDimensions, NodeTextDimensions {}
+
+export function getDerivedDimensions(d: Dimensions): DerivedDimensions {
+  return {
+    nodeTextFontSize: d.nodeTextFontSize,
+    nodeTextBorder: d.nodeTextBorder,
+    horizontalNodeBorder: d.horizontalNodeBorder,
+    intermediateWidth: d.intermediateWidth,
+    layerHeight: d.layerHeight,
+    layerDistance: d.layerDistance,
+    nodeBoxHeight: d.nodeBoxHeight,
+    boxConnectorAreaPerc: d.boxConnectorAreaPerc,
+    intermediateLayerPassedByVerticalLine: d.intermediateLayerPassedByVerticalLine,
+    boxCrossProtectionMargin: d.boxCrossProtectionMargin,
+    lineTransgressionPerc: d.lineTransgressionPerc,
+    estEdgeLabelCharacterWidth: calculateAverageFontCharacterWidth(d.edgeLabelFontSize),
+    edgeLabelFontSize: d.edgeLabelFontSize,
+    // In theory, we need a margin between multiple lines of an edge label.
+    // In practice, we get an acceptable result by adjusting the line heigt
+    // to produce it.
+    estEdgeLabelLineHeight: d.edgeLabelFontSize + 3,
+    preferredVertDistanceFromOrigin: d.preferredVertDistanceFromOrigin,
+    strictlyKeepLabelOutOfBox: d.strictlyKeepLabelOutOfBox,
+  };
+}

projects/frank-config-layout/src/lib/graphics/edge-label-layouter.spec.ts

@@ -1,12 +1,12 @@
 import { Line, Point } from '../graphics/graphics';
 import { Box } from './box';
-import { DerivedEdgeLabelDimensions, EdgeLabelLayouter } from './edge-label-layouter';
+import { EdgeLabelDimensions, EdgeLabelLayouter } from './edge-label-layouter';
 
 describe('EdgeLabelLayouter', () => {
   it('When two labels fit next to each other they appear next to each other, downwards', () => {
-    const dimensions: DerivedEdgeLabelDimensions = {
-      estCharacterWidth: 5,
-      estLabelLineHeight: 10,
+    const dimensions: EdgeLabelDimensions = {
+      estEdgeLabelCharacterWidth: 5,
+      estEdgeLabelLineHeight: 10,
       preferredVertDistanceFromOrigin: 30,
       strictlyKeepLabelOutOfBox: false,
     };
@@ -26,9 +26,9 @@ describe('EdgeLabelLayouter', () => {
   });
 
   it('When two labels do not fit next to each other they appear on different heights, downwards', () => {
-    const dimensions: DerivedEdgeLabelDimensions = {
-      estCharacterWidth: 5,
-      estLabelLineHeight: 10,
+    const dimensions: EdgeLabelDimensions = {
+      estEdgeLabelCharacterWidth: 5,
+      estEdgeLabelLineHeight: 10,
       preferredVertDistanceFromOrigin: 30,
       strictlyKeepLabelOutOfBox: false,
     };
@@ -51,9 +51,9 @@ describe('EdgeLabelLayouter', () => {
   // to test with upward lines.
 
   it('When two labels fit next to each other they appear next to each other, upwards', () => {
-    const dimensions: DerivedEdgeLabelDimensions = {
-      estCharacterWidth: 5,
-      estLabelLineHeight: 10,
+    const dimensions: EdgeLabelDimensions = {
+      estEdgeLabelCharacterWidth: 5,
+      estEdgeLabelLineHeight: 10,
       preferredVertDistanceFromOrigin: 30,
       strictlyKeepLabelOutOfBox: false,
     };
@@ -73,9 +73,9 @@ describe('EdgeLabelLayouter', () => {
   });
 
   it('When two labels do not fit next to each other they appear on different heights, upwards', () => {
-    const dimensions: DerivedEdgeLabelDimensions = {
-      estCharacterWidth: 5,
-      estLabelLineHeight: 10,
+    const dimensions: EdgeLabelDimensions = {
+      estEdgeLabelCharacterWidth: 5,
+      estEdgeLabelLineHeight: 10,
       preferredVertDistanceFromOrigin: 30,
       strictlyKeepLabelOutOfBox: false,
     };
@@ -96,8 +96,8 @@ describe('EdgeLabelLayouter', () => {
 
   it('When not strictlyKeepLabelOutOfBox, label can intersect box', () => {
     const dimensions = {
-      estCharacterWidth: 5,
-      estLabelLineHeight: 10,
+      estEdgeLabelCharacterWidth: 5,
+      estEdgeLabelLineHeight: 10,
       preferredVertDistanceFromOrigin: 12,
       strictlyKeepLabelOutOfBox: false,
     };
@@ -114,8 +114,8 @@ describe('EdgeLabelLayouter', () => {
 
   it('When strictlyKeepLabelOutOfBox, label can not intersect box', () => {
     const dimensions = {
-      estCharacterWidth: 5,
-      estLabelLineHeight: 10,
+      estEdgeLabelCharacterWidth: 5,
+      estEdgeLabelLineHeight: 10,
       preferredVertDistanceFromOrigin: 12,
       strictlyKeepLabelOutOfBox: true,
     };

projects/frank-config-layout/src/lib/graphics/edge-label-layouter.ts

@@ -18,45 +18,25 @@ import { NumbersAroundZero } from '../util/util';
 import { Box } from './box';
 import { Interval } from '../util/interval';
 import { Line, Point } from './graphics';
-import { calculateAverageFontCharacterWidth } from '../model/text';
 
 export interface EdgeLabelDimensions {
-  edgeLabelFontSize: number;
+  estEdgeLabelCharacterWidth: number;
+  estEdgeLabelLineHeight: number;
   preferredVertDistanceFromOrigin: number;
   strictlyKeepLabelOutOfBox: boolean;
 }
 
-export interface DerivedEdgeLabelDimensions {
-  estCharacterWidth: number;
-  estLabelLineHeight: number;
-  preferredVertDistanceFromOrigin: number;
-  strictlyKeepLabelOutOfBox: boolean;
-}
-
-export function getDerivedEdgeLabelDimensions(d: EdgeLabelDimensions): DerivedEdgeLabelDimensions {
-  return {
-    estCharacterWidth: calculateAverageFontCharacterWidth(d.edgeLabelFontSize),
-    // TODO issue https://github.com/frankframework/frank-config-layout/issues/51.
-    // When a label has multiple lines then we need a marging between the lines,
-    // not an increased line height. The text is rendered with a martin in svg-generator.ts.
-    estLabelLineHeight: d.edgeLabelFontSize + 3,
-    preferredVertDistanceFromOrigin: d.preferredVertDistanceFromOrigin,
-    strictlyKeepLabelOutOfBox: d.strictlyKeepLabelOutOfBox,
-  };
-}
-
 export class EdgeLabelLayouter {
   private boxes: Box[] = [];
 
-  constructor(readonly derivedDimensions: DerivedEdgeLabelDimensions) {}
+  constructor(readonly dimensions: EdgeLabelDimensions) {}
 
   add(line: Line, numCharactersOnLine: number, numTextLines: number): Box {
     const vdistSources = new NumbersAroundZero();
     while (true) {
       const vdistSource: number = vdistSources.next();
       const vdist: number =
-        this.derivedDimensions.preferredVertDistanceFromOrigin +
-        vdistSource * this.derivedDimensions.estLabelLineHeight;
+        this.dimensions.preferredVertDistanceFromOrigin + vdistSource * this.dimensions.estEdgeLabelLineHeight;
       if (vdist <= 0) {
         // The vertical center of the label would be in the box from which the line originates.
         // Next vdistSource.
@@ -65,14 +45,14 @@ export class EdgeLabelLayouter {
       const candidateCenter: Point = this.pointAt(vdist, line);
       const horizontalBox = Interval.createFromCenterSize(
         candidateCenter.x,
-        numCharactersOnLine * this.derivedDimensions.estCharacterWidth,
+        numCharactersOnLine * this.dimensions.estEdgeLabelCharacterWidth,
       );
       const verticalBox = Interval.createFromCenterSize(
         candidateCenter.y,
-        numTextLines * this.derivedDimensions.estLabelLineHeight,
+        numTextLines * this.dimensions.estEdgeLabelLineHeight,
       );
       const candidateBox = new Box(horizontalBox, verticalBox);
-      if (this.derivedDimensions.strictlyKeepLabelOutOfBox && candidateBox.verticalBox.contains(line.startPoint.y)) {
+      if (this.dimensions.strictlyKeepLabelOutOfBox && candidateBox.verticalBox.contains(line.startPoint.y)) {
         // The label would intersect with the box from which the line originates.
         // Next vdistSource.
         continue;

projects/frank-config-layout/src/lib/graphics/layout.ts

@@ -28,7 +28,7 @@ import { HorizontalConflictResolver } from './horizontal-conflict';
 import { getRange } from '../util/util';
 import { getConnectedIdsOfKey, getKey, keyFor } from '../model/graph';
 import { Box, LineChecker } from './box';
-import { DerivedEdgeLabelDimensions, EdgeLabelLayouter } from './edge-label-layouter';
+import { EdgeLabelDimensions, EdgeLabelLayouter } from './edge-label-layouter';
 import { straighten, StraightenedLine, StraightenedLineSegmentsBuilder } from './straightened-line';
 import { EdgeText, NodeText } from '../model/text';
 
@@ -84,11 +84,12 @@ export class LayoutBuilder {
   private layoutLineSegmentsByOriginalEdge = new Map<string, LayoutLineSegment[]>();
   private originalEdgesByConnector = new Map<string, string>();
   private lineThrougIntermediateNodeAllowance: number;
+
   constructor(
     private model: LayoutModel,
     private og: OriginalGraphReferencingIntermediates,
     private d: NodeAndEdgeDimensions,
-    private derivedEdgeLabelDimensions: DerivedEdgeLabelDimensions,
+    private derivedEdgeLabelDimensions: EdgeLabelDimensions,
   ) {
     this.lineThrougIntermediateNodeAllowance = Math.round((d.intermediateWidth * d.lineTransgressionPerc) / 100);
   }

projects/frank-config-layout/src/lib/graphics/straightened-line.ts

@@ -66,7 +66,6 @@ export class StraightenedLine {
     return [first, second];
   }
 
-  // TODO: Test
   split(pointFunction: (id: string, line: Line) => Point): StraightenedLine[] {
     if (this.replacedNodes.length === 0) {
       return [this];

projects/frank-config-layout/src/lib/graphics/svg-generator.ts

@@ -21,25 +21,24 @@ import { Box } from './box';
 import { Point } from './graphics';
 import { EdgeLabel, Layout, LayoutLineSegment, PlacedNode } from './layout';
 
-// TODO: Issue https://github.com/frankframework/frank-config-layout/issues/51.
-// The way dimensions are divided over different interfaces and passed along
-// is not clear now. Should be reconsidered.
-export function generateSvg(
-  layout: Layout,
-  nodeTextFontSize: number,
-  edgeLabelFontSize: number,
-  border: number,
-): string {
+export interface SvgGenerationDimensions {
+  nodeTextFontSize: number;
+  nodeTextBorder: number;
+  edgeLabelFontSize: number;
+  estEdgeLabelCharacterWidth: number;
+}
+
+export function generateSvg(layout: Layout, d: SvgGenerationDimensions): string {
   return (
     openSvg(layout.width, layout.height) +
     renderDefs() +
     renderNodes(
       layout.nodes.map((n) => n as PlacedNode),
-      border,
-      nodeTextFontSize,
+      d.nodeTextBorder,
+      d.nodeTextFontSize,
     ) +
     renderEdges(layout.layoutLineSegments) +
-    renderLabels(layout.edgeLabels, edgeLabelFontSize) +
+    renderLabels(layout.edgeLabels, d.edgeLabelFontSize, d.estEdgeLabelCharacterWidth) +
     closeSvg()
   );
 }
@@ -165,23 +164,21 @@ function classOfLine(edge: LayoutLineSegment): string {
   }
 }
 
-function renderLabels(labels: EdgeLabel[], edgeLabelFontSize: number): string {
-  return `  <g text-anchor="middle" dominant-baseline="middle">${labels.map((label) => renderLabel(label, edgeLabelFontSize)).join('')}</g>`;
+function renderLabels(labels: EdgeLabel[], edgeLabelFontSize: number, estEdgeLabelCharacterWidth: number): string {
+  return `  <g text-anchor="middle" dominant-baseline="middle">${labels.map((label) => renderLabel(label, edgeLabelFontSize, estEdgeLabelCharacterWidth)).join('')}</g>`;
 }
 
-function renderLabel(label: EdgeLabel, edgeLabelFontSize: number): string {
+function renderLabel(label: EdgeLabel, edgeLabelFontSize: number, estEdgeLabelCharacterWidth: number): string {
   const coordinates: Point[] = arrangeInBox({
     container: new Box(label.horizontalBox, label.verticalBox),
     border: 0,
-    itemWidths: label.text.lines.map((l) => l.width),
-    // TODO: Issue https://github.com/frankframework/frank-config-layout/issues/51.
-    // This is not right - either make height variable or do not store with each line.
-    commonItemHeight: label.text.lines[0].height,
+    itemWidths: label.text.lines.map((l) => l.length * estEdgeLabelCharacterWidth),
+    commonItemHeight: edgeLabelFontSize,
   });
   let result: string = '';
   for (let i = 0; i < label.text.lines.length; ++i) {
     const p: Point = coordinates[i];
-    result += renderSingleLayerText(p.x, p.y, edgeLabelFontSize, label.text.lines[i].text);
+    result += renderSingleLayerText(p.x, p.y, edgeLabelFontSize, label.text.lines[i]);
   }
   return result;
 }