From 81a996a678ca8814ef912f5935e33399a17b8837 Mon Sep 17 00:00:00 2001 From: Endri Bezati Date: Mon, 31 Aug 2026 16:31:37 +0200 Subject: [PATCH 1/6] feat(views): draw boundary ports as schematic symbols, not boxes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A network's inputs and outputs drew as rounded pills, 120x50, with the name centred and the type under it. The wire met the pill at its vertical centre — which is the GAP between the two lines of text, so the wire ran through the middle of the label. They are now schematic port symbols: an arrow glyph sitting on the wire's own axis, with the text running outward from it, away from the wire. An input is a bare arrow; an output is an arrow stopping against a bar, the way a terminal is drawn. The name sits ON the axis, so the wire enters on the name line; the type sits on a second line below, clear of the wire entirely. Names are right-aligned on inputs and left-aligned on outputs. Both text columns therefore run away from the glyph column into the margin, and the glyphs stay in a straight line. Node width stays fixed across every boundary node for exactly that reason: their inner edges line up whatever ELK does with alignment inside a layer, and text overflowing into the margin is harmless. Height drops from 50 to a 22px row pitch, a little under half. The geometry lives in shared, alongside the port anchor and for the same reason. The two halves are computed in different processes — the server sizes the node and places the port the routers anchor to, the client draws the arrow — and nothing connects them at runtime. Disagree by a pixel and the wire ends near the arrow rather than on it, which no error reports. Two things the spec calls for that needed a decision rather than a number. The pill was what showed selection and hover, so that moves to the hit target: a 20x22 rectangle centred on the glyph, far larger than the 8px arrow so it can be grabbed, and only one row tall so that hiding the type line at low zoom cannot move the drag target. And the port element stays in the model as the edge anchor but is no longer drawn, since the glyph would otherwise be doubled. The type line is no longer tinted by direction. It is supporting text on its own line, and colouring it made it compete with the name. --- .../diagram-client/src/diagram-client.css | 51 ++++---- packages/diagram-client/src/views.ts | 113 +++++++++++----- .../src/model/graph-gmodel-source.ts | 26 +++- .../test/boundary-port-geometry.test.ts | 122 ++++++++++++++++++ packages/shared/src/boundary-port-geometry.ts | 78 +++++++++++ packages/shared/src/index.ts | 1 + 6 files changed, 332 insertions(+), 59 deletions(-) create mode 100644 packages/diagram-server/test/boundary-port-geometry.test.ts create mode 100644 packages/shared/src/boundary-port-geometry.ts diff --git a/packages/diagram-client/src/diagram-client.css b/packages/diagram-client/src/diagram-client.css index 811f072..aff3aae 100644 --- a/packages/diagram-client/src/diagram-client.css +++ b/packages/diagram-client/src/diagram-client.css @@ -880,50 +880,57 @@ i.cal-toggle-feedback-edges.cal-feedback-hidden-icon { Boundary node styles (network input/output ports) ============================================ */ -.boundary-node .boundary-body { - stroke-width: 1.5px; -} +/* A boundary port is a symbol on the wire, not a box: an arrow glyph on the + axis with the name beside it and the type on a second line below. Sizes come + from BoundaryPortGeometry, which the server reads too — the glyph and the + port anchor have to land on the same pixel. */ .boundary-node .boundary-label { - fill: var(--vscode-editor-foreground, #ffffff); font-family: var(--vscode-font-family); - font-size: 11px; + font-size: 9.5px; font-weight: 500; } +/* The type is deliberately NOT tinted by direction: it is supporting text on + its own line, and colouring it would make it compete with the name. */ .boundary-node .boundary-type-label { - font-size: 9px; + font-size: 8px; font-weight: 400; - opacity: 0.85; -} - -.boundary-input .boundary-body { - fill: var(--vscode-charts-green, #89d185); - fill-opacity: 0.3; - stroke: var(--vscode-charts-green, #89d185); + fill: #a8a8a2; } +.boundary-input .boundary-glyph, .boundary-input .boundary-label { fill: var(--vscode-charts-green, #89d185); } -.boundary-output .boundary-body { +.boundary-output .boundary-glyph, +.boundary-output .boundary-label { fill: var(--vscode-charts-blue, #007fd4); - fill-opacity: 0.3; - stroke: var(--vscode-charts-blue, #007fd4); } -.boundary-output .boundary-label { - fill: var(--vscode-charts-blue, #007fd4); +/* The grab target. Invisible until the port is selected or hovered, which is + also what replaced the old body as the thing that shows selection. */ +.boundary-node .boundary-hit { + fill: transparent; + stroke: none; +} + +.boundary-node.hover .boundary-hit { + fill: var(--vscode-toolbar-hoverBackground, rgba(128, 128, 128, 0.18)); } -.boundary-node.selected .boundary-body { +.boundary-node.selected .boundary-hit { + fill: none; stroke: var(--vscode-focusBorder, #007fd4); - stroke-width: 2px; + stroke-width: 1px; + rx: 2px; } -.boundary-node.hover .boundary-body { - fill-opacity: 0.5; +/* The port element stays in the model as the edge anchor, but the glyph above + is what is drawn — rendering both would double the arrow. */ +.boundary-node .workflow-port { + display: none; } .boundary-node .direction-arrow { diff --git a/packages/diagram-client/src/views.ts b/packages/diagram-client/src/views.ts index 1a58e00..631d919 100644 --- a/packages/diagram-client/src/views.ts +++ b/packages/diagram-client/src/views.ts @@ -42,7 +42,7 @@ import { import { EDGE_OPEN_ANCHOR_X_ARG, EDGE_OPEN_ANCHOR_Y_ARG } from './edge-open-mouse-listener'; import { clientBehavior } from './profile'; import { isIncidentEdgeInActiveDrag } from './elk-live-drag-router'; -import { WorkflowDiagramCss, WorkflowDiagramMetadata, WorkflowDiagramTypes } from '@dialogram/shared'; +import { BoundaryPortGeometry, WorkflowDiagramCss, WorkflowDiagramMetadata, WorkflowDiagramTypes } from '@dialogram/shared'; import { WorkflowDiagramConstants } from '@dialogram/shared'; /** Walk to the model root; the drag-active window is tracked per root. */ @@ -88,7 +88,7 @@ const NODE_HEIGHT = 80; const HEADER_HEIGHT = WorkflowDiagramConstants.HEADER_HEIGHT_PX; const PORT_RADIUS = 5; const BOUNDARY_NODE_WIDTH = 80; -const BOUNDARY_NODE_HEIGHT = 30; +const BOUNDARY_NODE_HEIGHT = BoundaryPortGeometry.ROW_PITCH_PX; // Icon size for center node icons const NODE_ICON_SIZE = 36; @@ -1017,6 +1017,62 @@ export class ProxyNodeView extends ShapeView { * Boundary input port node (left margin) - network's input exposed to internal entities * Styled as a terminal connector shape with port name centered */ +/** + * The glyph for a boundary port, drawn on the wire's axis. + * + * An input is a bare arrow pointing into the diagram; an output is an arrow + * that stops against a bar, the way a terminal is drawn on a schematic. + * + * The hit target is deliberately far larger than the arrow, and deliberately + * only as tall as one row: an 8px arrow cannot be grabbed reliably, and + * including the type line would move the drag target whenever the type is + * hidden at low zoom. It also carries selection now that there is no body to + * outline. + * + * The x it is built at must agree with where the SERVER placed the port, or the + * wire misses the arrow — both read `BoundaryPortGeometry` for exactly that + * reason. + */ +function boundaryGlyph(isInput: boolean, nodeWidth: number): VNode[] { + const G = BoundaryPortGeometry; + const axis = G.AXIS_Y_PX; + const arrow = isInput ? G.SOURCE_ARROW : G.SINK_ARROW; + // Inputs occupy the node's right edge, outputs its left. + const glyphX = isInput ? nodeWidth - G.glyphWidth(true) : 0; + const top = axis - arrow.height / 2; + const tipX = glyphX + arrow.width; + + const parts: VNode[] = [ + svg('rect', { + class: { 'boundary-hit': true }, + attrs: { + x: glyphX + G.glyphWidth(isInput) / 2 - G.HIT.width / 2, + y: axis - G.HIT.height / 2, + width: G.HIT.width, + height: G.HIT.height + } + }), + // Both arrows point the way the data flows: into the diagram for an + // input, into the bar for an output. + svg('path', { + class: { 'boundary-glyph': true }, + attrs: { d: `M ${glyphX} ${top} L ${tipX} ${axis} L ${glyphX} ${top + arrow.height} Z` } + }) + ]; + if (!isInput) { + parts.push(svg('rect', { + class: { 'boundary-glyph': true, 'boundary-glyph-bar': true }, + attrs: { + x: tipX, + y: axis - G.SINK_BAR.height / 2, + width: G.SINK_BAR.width, + height: G.SINK_BAR.height + } + })); + } + return parts; +} + @injectable() export class BoundaryInputNodeView extends ShapeView { override render(node: Readonly, context: RenderingContext, args?: IViewArgs): VNode | undefined { @@ -1025,8 +1081,7 @@ export class BoundaryInputNodeView extends ShapeView { } // Use server-provided size, not Sprotty-computed bounds - const { width, height } = getNodeSize(node, BOUNDARY_NODE_WIDTH, BOUNDARY_NODE_HEIGHT); - const cornerRadius = height / 2; + const { width } = getNodeSize(node, BOUNDARY_NODE_WIDTH, BOUNDARY_NODE_HEIGHT); return svg('g', { class: { @@ -1039,17 +1094,11 @@ export class BoundaryInputNodeView extends ShapeView { transform: `translate(${node.position.x}, ${node.position.y})` } }, - // Rounded terminal shape (left side fully rounded, right side has small radius) - svg('rect', { - class: { 'boundary-body': true }, - attrs: { - x: 0, y: 0, - width, height, - rx: cornerRadius, - ry: cornerRadius - } - }), - // Children: boundary labels + output port on right side + // No body: the glyph on the axis IS the port, and the two text lines + // run outward from it into the margin. + ...boundaryGlyph(true, width), + // Children: boundary labels + the port itself (anchor only; the + // glyph above is what is seen). ...context.renderChildren(node) ); } @@ -1067,8 +1116,7 @@ export class BoundaryOutputNodeView extends ShapeView { } // Use server-provided size, not Sprotty-computed bounds - const { width, height } = getNodeSize(node, BOUNDARY_NODE_WIDTH, BOUNDARY_NODE_HEIGHT); - const cornerRadius = height / 2; + const { width } = getNodeSize(node, BOUNDARY_NODE_WIDTH, BOUNDARY_NODE_HEIGHT); return svg('g', { class: { @@ -1081,17 +1129,8 @@ export class BoundaryOutputNodeView extends ShapeView { transform: `translate(${node.position.x}, ${node.position.y})` } }, - // Rounded terminal shape (right side fully rounded) - svg('rect', { - class: { 'boundary-body': true }, - attrs: { - x: 0, y: 0, - width, height, - rx: cornerRadius, - ry: cornerRadius - } - }), - // Children: boundary labels + input port on left side + // No body — see the input view above. + ...boundaryGlyph(false, width), ...context.renderChildren(node) ); } @@ -2131,12 +2170,20 @@ export class WorkflowLabelView implements IView { const labelType = (label as any).type as string || ''; const pos = label.position ?? { x: 0, y: 0 }; - // Boundary labels (name + type) are positioned relative to the boundary node. + // Boundary labels (name + type) are positioned relative to the boundary + // node, which is why they must carry no layout feature — see + // `BoundaryLabel`. Text runs OUTWARD from the glyph, away from the wire: + // right-aligned on an input (whose glyph is on its right edge), + // left-aligned on an output (whose glyph is on its left). That keeps the + // glyphs in a column with the text running off into the margin, and + // keeps the type off the wire — the name sits on the axis, the type on + // its own line below. if (labelType === WorkflowDiagramTypes.LABEL_BOUNDARY_NAME || labelType === WorkflowDiagramTypes.LABEL_BOUNDARY_TYPE) { const parent: any = (label as any).parent; const parentW = parent?.size?.width ?? parent?.bounds?.width ?? 100; - const parentH = parent?.size?.height ?? parent?.bounds?.height ?? 40; const isType = labelType === WorkflowDiagramTypes.LABEL_BOUNDARY_TYPE; + const isInput = parent?.type === WorkflowDiagramTypes.NODE_BOUNDARY_INPUT; + const offset = BoundaryPortGeometry.textOffset(isInput); return svg('g', { attrs: { transform: `translate(${pos.x}, ${pos.y})` } @@ -2144,9 +2191,9 @@ export class WorkflowLabelView implements IView { svg('text', { class: { 'boundary-label': true, 'boundary-type-label': isType }, attrs: { - x: parentW / 2, - y: isType ? (parentH / 2 + 10) : (parentH / 2 - 2), - 'text-anchor': 'middle', + x: isInput ? parentW - offset : offset, + y: isType ? BoundaryPortGeometry.TYPE_Y_PX : BoundaryPortGeometry.AXIS_Y_PX, + 'text-anchor': isInput ? 'end' : 'start', 'dominant-baseline': 'middle' } }, label.text || '') diff --git a/packages/diagram-server/src/model/graph-gmodel-source.ts b/packages/diagram-server/src/model/graph-gmodel-source.ts index c1e4ad8..f09d92a 100644 --- a/packages/diagram-server/src/model/graph-gmodel-source.ts +++ b/packages/diagram-server/src/model/graph-gmodel-source.ts @@ -1,4 +1,4 @@ -import { WorkflowDiagramCss, WorkflowDiagramConstants, WorkflowDiagramMetadata, WorkflowDiagramTypes } from '@dialogram/shared'; +import { BoundaryPortGeometry, WorkflowDiagramCss, WorkflowDiagramConstants, WorkflowDiagramMetadata, WorkflowDiagramTypes } from '@dialogram/shared'; import * as path from 'node:path'; import { findFeedbackEdges } from './feedback-edges'; import { URI } from 'vscode-uri'; @@ -1030,14 +1030,32 @@ export class GraphGModelSource { private createBoundaryNode(node: PyGraphNode, stableNodeId: string): GNode { const port = node.ports[0]; const isInput = node.kind === 'wf-input'; - const size = { width: 120, height: 50 }; + // One row tall now, not a 50px box — the port is a symbol on the wire's + // axis rather than a container. Width stays fixed across every boundary + // node so their inner edges, and therefore their glyphs, line up in a + // column whatever ELK does with alignment inside the layer; the text + // runs outward from that edge into the margin, where overflow is + // harmless. See BoundaryPortGeometry. + const size = { width: 120, height: BoundaryPortGeometry.ROW_PITCH_PX }; + const glyph = { + width: BoundaryPortGeometry.glyphWidth(isInput), + height: BoundaryPortGeometry.SOURCE_ARROW.height + }; const portId = `${stableNodeId}_port_${port?.name ?? 'Port'}`; const p: GPort = { id: portId, type: isInput ? WorkflowDiagramTypes.PORT_OUTPUT : WorkflowDiagramTypes.PORT_INPUT, cssClasses: [WorkflowDiagramCss.PORT], - position: { x: isInput ? size.width : -8, y: 22 }, - size: { width: 8, height: 6 }, + // Sits exactly where the client draws the glyph, so the anchor the + // routers compute lands on the arrow's tip: an input's arrow ends at + // the node's right edge, an output's begins at its left edge. The y + // puts the anchor on the axis — the NAME's line, not the node's + // centre, which is what used to run the wire between the two lines. + position: { + x: isInput ? size.width - glyph.width : 0, + y: BoundaryPortGeometry.AXIS_Y_PX - glyph.height / 2 + }, + size: glyph, args: { [WorkflowDiagramMetadata.PORT_NAME]: port?.name ?? '', [WorkflowDiagramMetadata.PORT_DIRECTION]: isInput ? 'output' : 'input' diff --git a/packages/diagram-server/test/boundary-port-geometry.test.ts b/packages/diagram-server/test/boundary-port-geometry.test.ts new file mode 100644 index 0000000..509ddbf --- /dev/null +++ b/packages/diagram-server/test/boundary-port-geometry.test.ts @@ -0,0 +1,122 @@ +/** + * The wire has to touch the arrow. + * + * A boundary port is drawn as a glyph on the wire's axis, and the two halves of + * that are computed in different processes: the SERVER sizes the node and places + * the port element the routers anchor to, the CLIENT draws the arrow. Nothing + * connects them at runtime — if they disagree the wire simply ends somewhere + * near the arrow instead of on it, which no test would notice and no error would + * report. + * + * So both read `BoundaryPortGeometry`, and this pins the server's half of that + * agreement: the anchor the routers compute must land exactly where the client + * puts the arrow's tip. The client's half is held by the compiler — it builds + * the glyph from the same constants. + * + * The old layout is what makes the axis worth asserting: a 50px box with the + * wire at its vertical centre put the wire in the GAP between the name and the + * type, running through the middle of the label. The axis is now the name's own + * line, and the type sits below it, clear of the wire. + */ +import { describe, expect, it } from 'vitest'; +import { BoundaryPortGeometry, portAnchor, WorkflowDiagramMetadata } from '@dialogram/shared'; +import { GraphGModelSource, type PyGraphDocument } from '../src/model/graph-gmodel-source'; + +function docWith(kind: 'wf-input' | 'wf-output'): PyGraphDocument { + return { + version: '1', + graph: { + id: 'root', + nodes: [{ + id: `wf:${kind}:Com`, + kind, + label: 'Com', + scope: 'root', + ports: [{ + id: 'p', + name: 'Com', + direction: kind === 'wf-input' ? 'out' : 'in', + type: 'Commit' + }] + }], + edges: [] + } + }; +} + +/** The boundary node and its port, as the server hands them to the client. */ +function boundaryOf(kind: 'wf-input' | 'wf-output') { + const result = new GraphGModelSource().transform(docWith(kind)); + const node: any = result.graph.children?.find((c: any) => c.args?.['wf:boundaryKind'] !== undefined); + const port: any = node.children?.find((c: any) => c.type?.startsWith('port')); + expect(port, 'boundary node has no port').toBeDefined(); + // The node sits at the origin until ELK places it, so port positions are + // already absolute for the purposes of the anchor arithmetic. + return { + node, + anchor: portAnchor({ absolute: port.position, size: port.size, type: port.type }) + }; +} + +describe('boundary port geometry', () => { + it('is one row tall, not a box', () => { + expect(boundaryOf('wf-input').node.size.height).toBe(BoundaryPortGeometry.ROW_PITCH_PX); + }); + + it('anchors the wire on the name line, not the node centre', () => { + const input = boundaryOf('wf-input'); + const output = boundaryOf('wf-output'); + + expect(input.anchor.y).toBe(BoundaryPortGeometry.AXIS_Y_PX); + expect(output.anchor.y).toBe(BoundaryPortGeometry.AXIS_Y_PX); + // The distinction the redesign turns on: the old anchor was here. + expect(input.anchor.y).not.toBe(input.node.size.height / 2); + }); + + it("anchors an input at its arrow's tip, on the node's right edge", () => { + const { node, anchor } = boundaryOf('wf-input'); + + expect(anchor.side).toBe('EAST'); + expect(anchor.x).toBe(node.size.width); + }); + + it("anchors an output where its arrow begins, on the node's left edge", () => { + const { anchor } = boundaryOf('wf-output'); + + expect(anchor.side).toBe('WEST'); + expect(anchor.x).toBe(0); + }); + + it('keeps the type off the wire', () => { + const G = BoundaryPortGeometry; + // The name owns the axis; the type's line starts below the name's. + expect(G.AXIS_Y_PX).toBeLessThan(G.NAME_LINE_HEIGHT_PX); + expect(G.TYPE_Y_PX).toBeGreaterThan(G.NAME_LINE_HEIGHT_PX); + expect(G.NAME_LINE_HEIGHT_PX + G.TYPE_LINE_HEIGHT_PX).toBe(G.ROW_PITCH_PX); + }); + + it('leaves a gap between glyph and text on both sides', () => { + expect(BoundaryPortGeometry.textOffset(true)) + .toBe(BoundaryPortGeometry.SOURCE_ARROW.width + BoundaryPortGeometry.GLYPH_TEXT_GAP_PX); + // An output's glyph is arrow plus bar, so its text starts further out. + expect(BoundaryPortGeometry.textOffset(false)) + .toBe(BoundaryPortGeometry.SINK_ARROW.width + BoundaryPortGeometry.SINK_BAR.width + + BoundaryPortGeometry.GLYPH_TEXT_GAP_PX); + }); + + it('keeps the grab target bigger than the arrow but only one row tall', () => { + const G = BoundaryPortGeometry; + + expect(G.HIT.width).toBeGreaterThan(G.glyphWidth(false)); + // Not taller than a row: the type line can be hidden at low zoom, and a + // hit area covering it would move the drag target when it goes. + expect(G.HIT.height).toBe(G.ROW_PITCH_PX); + }); + + it('still carries the port identity', () => { + const { node } = boundaryOf('wf-input'); + + expect(node.args[WorkflowDiagramMetadata.PORT_NAME]).toBe('Com'); + expect(node.args[WorkflowDiagramMetadata.PORT_TYPE]).toBe('Commit'); + }); +}); diff --git a/packages/shared/src/boundary-port-geometry.ts b/packages/shared/src/boundary-port-geometry.ts new file mode 100644 index 0000000..6a8a6bd --- /dev/null +++ b/packages/shared/src/boundary-port-geometry.ts @@ -0,0 +1,78 @@ +/** + * The geometry of a boundary port: a schematic port symbol, not a box. + * + * A network's inputs and outputs used to draw as rounded pills with the name + * centred and the type under it. Two lines of text inside a 50px box, the wire + * meeting it at the box's vertical centre — which is the gap BETWEEN the two + * lines, so the wire ran through the middle of the label. + * + * This is the schematic treatment instead: an arrow glyph sitting on the wire's + * own axis, with the text running outward from it, away from the wire. The name + * sits ON the axis so the wire enters on the name line; the type sits on a + * second line below, clear of the wire entirely. Names are right-aligned on + * inputs and left-aligned on outputs, so both text columns run away from the + * glyph column and the glyphs stay in a straight line. + * + * It lives in shared because the two halves of it are computed in different + * processes and must agree exactly: the SERVER sizes the node and places the + * port (which is what the edge routers anchor to), the CLIENT draws the glyph + * and the two text lines. If the client's glyph and the server's port anchor + * disagree, the wire visibly misses the arrow it is supposed to touch. + */ + +/** Arrow, bar, or hit-target extents. */ +export interface BoundaryGlyphBox { + width: number; + height: number; +} + +export namespace BoundaryPortGeometry { + /** Total height of one port: the name line plus the type line. */ + export const ROW_PITCH_PX = 22; + /** The name's line box. Its centre is the axis. */ + export const NAME_LINE_HEIGHT_PX = 12; + /** The type's line box, directly below the name's. */ + export const TYPE_LINE_HEIGHT_PX = 10; + + export const NAME_FONT_PX = 9.5; + export const TYPE_FONT_PX = 8; + + /** Between the glyph and the first character of text. */ + export const GLYPH_TEXT_GAP_PX = 4; + + /** An input's glyph: a plain arrow, pointing into the diagram. */ + export const SOURCE_ARROW: BoundaryGlyphBox = { width: 8, height: 9 }; + /** An output's glyph: an arrow that stops against a bar, like a terminal. */ + export const SINK_ARROW: BoundaryGlyphBox = { width: 7, height: 9 }; + export const SINK_BAR: BoundaryGlyphBox = { width: 2, height: 11 }; + + /** + * The grab target, centred on the glyph. + * + * An 8px arrow is too small to hit reliably, and the type line must NOT be + * part of it — the type can be hidden at low zoom, and a hit area that + * changed with it would move the drag target as you zoom. + */ + export const HIT: BoundaryGlyphBox = { width: 20, height: ROW_PITCH_PX }; + + /** Where the wire meets the port, measured down from the node's top. */ + export const AXIS_Y_PX = NAME_LINE_HEIGHT_PX / 2; + + /** Centre of the type line, measured down from the node's top. */ + export const TYPE_Y_PX = NAME_LINE_HEIGHT_PX + TYPE_LINE_HEIGHT_PX / 2; + + /** Full width of a glyph, including an output's bar. */ + export function glyphWidth(isInput: boolean): number { + return isInput ? SOURCE_ARROW.width : SINK_ARROW.width + SINK_BAR.width; + } + + /** + * Where text starts, as an offset from the node's own inner edge. + * + * Inputs read right-to-left from the node's right edge; outputs left-to-right + * from its left edge. Either way the text begins one gap past the glyph. + */ + export function textOffset(isInput: boolean): number { + return glyphWidth(isInput) + GLYPH_TEXT_GAP_PX; + } +} diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 5d4202e..13390c9 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -9,6 +9,7 @@ export * from './diagram-types'; export * from './diagram-constants'; export * from './port-anchor'; export * from './port-stub'; +export * from './boundary-port-geometry'; export * from './binding-keys'; export * from './diagram-seams'; export * from './chat-seams'; From 28b2adcef0840ceaf26a7295ba53ac133301ec67 Mon Sep 17 00:00:00 2001 From: Endri Bezati Date: Mon, 31 Aug 2026 16:40:01 +0200 Subject: [PATCH 2/6] fix(views): align boundary text, free the type colour, and scale the symbol up MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three faults from the first cut, one of them mine to have caught before it shipped. The name rendered ON TOP of its own glyph. The text was drawn inside a group translated by `label.position` — inherited from the old code, where it was equally meaningless. These labels carry no layout feature precisely so that nothing but the view positions them, which means their position is simply whatever it was last left at; translating by it moved the text off the coordinates the view had just computed. The text is now drawn directly, with no wrapping transform. The type took the name's colour. It carries both `boundary-label` and `boundary-type-label`, so `.boundary-input .boundary-label` matched it too — at the SAME specificity, two classes each, leaving document order to decide. The direction rules came later and won, and the type rendered green on inputs and blue on outputs. Qualifying with both classes outranks them outright, and the rule now also sits after them so a reorder cannot recreate the tie. That one is worth a test rather than a fix alone, because a colour assertion would not have caught it: the declaration was always present and always correct, and what was wrong was whether it applied. So the test asserts the two things that decide that — specificity, and that it is not leaning on order. Writing it turned up a second-order version of the same trap: the first draft scanned the raw file and matched the selector inside the comment EXPLAINING the rule. It strips comments now, so only what the browser sees is scanned. Finally the whole symbol was too small. The drawing it came from is set at a finer weight than the diagram renders at, so a 9.5px name and an 8px arrow read as fine print beside a 3px wire. Everything is scaled by about a quarter, in proportion: the arrow still spans most of the name's line, the type still sits a little under two thirds of the name, and the row grows 22 to 28. --- .../diagram-client/src/diagram-client.css | 24 +++--- packages/diagram-client/src/views.ts | 27 ++++--- .../test/boundary-type-label-cascade.test.ts | 79 +++++++++++++++++++ packages/shared/src/boundary-port-geometry.ts | 27 ++++--- 4 files changed, 125 insertions(+), 32 deletions(-) create mode 100644 packages/diagram-client/test/boundary-type-label-cascade.test.ts diff --git a/packages/diagram-client/src/diagram-client.css b/packages/diagram-client/src/diagram-client.css index aff3aae..de0e02d 100644 --- a/packages/diagram-client/src/diagram-client.css +++ b/packages/diagram-client/src/diagram-client.css @@ -887,18 +887,10 @@ i.cal-toggle-feedback-edges.cal-feedback-hidden-icon { .boundary-node .boundary-label { font-family: var(--vscode-font-family); - font-size: 9.5px; + font-size: 12px; font-weight: 500; } -/* The type is deliberately NOT tinted by direction: it is supporting text on - its own line, and colouring it would make it compete with the name. */ -.boundary-node .boundary-type-label { - font-size: 8px; - font-weight: 400; - fill: #a8a8a2; -} - .boundary-input .boundary-glyph, .boundary-input .boundary-label { fill: var(--vscode-charts-green, #89d185); @@ -909,6 +901,20 @@ i.cal-toggle-feedback-edges.cal-feedback-hidden-icon { fill: var(--vscode-charts-blue, #007fd4); } +/* The type is deliberately NOT tinted by direction: it is supporting text on + its own line, and colouring it makes it compete with the name. + + Both the class and the position matter. The type label also carries + `boundary-label`, so `.boundary-input .boundary-label` matches it too — at + equal specificity, which the direction rules would then win on document + order. Qualifying with both classes outranks them outright, and it sits after + them so neither can be reintroduced by a reorder. */ +.boundary-node .boundary-label.boundary-type-label { + font-size: 10px; + font-weight: 400; + fill: #a8a8a2; +} + /* The grab target. Invisible until the port is selected or hovered, which is also what replaced the old body as the thing that shows selection. */ .boundary-node .boundary-hit { diff --git a/packages/diagram-client/src/views.ts b/packages/diagram-client/src/views.ts index 631d919..b17344b 100644 --- a/packages/diagram-client/src/views.ts +++ b/packages/diagram-client/src/views.ts @@ -2185,19 +2185,20 @@ export class WorkflowLabelView implements IView { const isInput = parent?.type === WorkflowDiagramTypes.NODE_BOUNDARY_INPUT; const offset = BoundaryPortGeometry.textOffset(isInput); - return svg('g', { - attrs: { transform: `translate(${pos.x}, ${pos.y})` } - }, - svg('text', { - class: { 'boundary-label': true, 'boundary-type-label': isType }, - attrs: { - x: isInput ? parentW - offset : offset, - y: isType ? BoundaryPortGeometry.TYPE_Y_PX : BoundaryPortGeometry.AXIS_Y_PX, - 'text-anchor': isInput ? 'end' : 'start', - 'dominant-baseline': 'middle' - } - }, label.text || '') - ); + // NOT translated by `label.position`. These labels carry no layout + // feature precisely so that nothing positions them but this view — + // their position is whatever it was left at, and applying it shifted + // the text off the coordinates computed here. It is the reason the + // name rendered on top of its own glyph. + return svg('text', { + class: { 'boundary-label': true, 'boundary-type-label': isType }, + attrs: { + x: isInput ? parentW - offset : offset, + y: isType ? BoundaryPortGeometry.TYPE_Y_PX : BoundaryPortGeometry.AXIS_Y_PX, + 'text-anchor': isInput ? 'end' : 'start', + 'dominant-baseline': 'middle' + } + }, label.text || ''); } // Port labels - ELK positions the label box, we handle text anchoring diff --git a/packages/diagram-client/test/boundary-type-label-cascade.test.ts b/packages/diagram-client/test/boundary-type-label-cascade.test.ts new file mode 100644 index 0000000..cb7ac8a --- /dev/null +++ b/packages/diagram-client/test/boundary-type-label-cascade.test.ts @@ -0,0 +1,79 @@ +/** + * The type line must not take the name's colour. + * + * This shipped wrong once, and the reason is worth pinning rather than the + * pixel. The type label carries BOTH `boundary-label` and + * `boundary-type-label`, so `.boundary-input .boundary-label` — the rule that + * tints a name green — matches it too. Its own rule was written as + * `.boundary-node .boundary-type-label`, which is the SAME specificity (two + * classes), so the winner came down to document order, and the direction rules + * came later. The type rendered green on inputs and blue on outputs, exactly + * like the name beside it. + * + * A colour assertion would not have caught it — the declaration was always + * there and always correct. What was wrong was whether it applied. So these + * assert the two things that decide that: the selector outranks the direction + * rules on specificity, and it is not relying on order to do it. + */ +import { readFileSync } from 'node:fs'; +import * as path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { describe, expect, it } from 'vitest'; + +const SOURCE = readFileSync( + path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../src/diagram-client.css'), + 'utf8' +); + +/** + * Comments stripped, because the comments here DISCUSS the selectors they + * explain — an order check against the raw file matched the prose above the + * rule and failed on it. Only what the browser sees is scanned. + */ +const CSS = SOURCE.replace(/\/\*[\s\S]*?\*\//g, ''); + +/** The selector list of the rule that sets the type colour. */ +function typeColourRule(): { selector: string; index: number } { + const match = /([^}]*?)\{[^}]*?fill:\s*#a8a8a2[^}]*?\}/s.exec(CSS); + expect(match, 'no rule sets the type label colour').not.toBeNull(); + return { selector: match![1].trim(), index: match!.index }; +} + +describe('boundary type label colour', () => { + it('is set at all', () => { + expect(typeColourRule().selector).toContain('boundary-type-label'); + }); + + /** + * Two classes on the same element is a tie, and a tie is decided by order — + * which is how this broke. Qualifying with both classes makes it three, so + * it wins outright. + */ + it('outranks the direction rules on specificity, not on order', () => { + const { selector } = typeColourRule(); + + expect( + selector, + 'must qualify both classes so it beats `.boundary-input .boundary-label`' + ).toMatch(/\.boundary-label\.boundary-type-label|\.boundary-type-label\.boundary-label/); + }); + + /** + * Belt and braces: even with the specificity fixed, sitting after the rules + * it competes with means a future reorder cannot quietly recreate the tie. + */ + it('also sits after the rules that tint the name', () => { + const { index } = typeColourRule(); + const directionRules = [...CSS.matchAll(/\.boundary-(input|output) \.boundary-label/g)]; + + expect(directionRules.length, 'the direction rules moved or vanished').toBeGreaterThan(0); + for (const rule of directionRules) { + expect(rule.index).toBeLessThan(index); + } + }); + + it('leaves the name tinted by direction', () => { + expect(CSS).toMatch(/\.boundary-input \.boundary-glyph,\s*\.boundary-input \.boundary-label/); + expect(CSS).toMatch(/\.boundary-output \.boundary-glyph,\s*\.boundary-output \.boundary-label/); + }); +}); diff --git a/packages/shared/src/boundary-port-geometry.ts b/packages/shared/src/boundary-port-geometry.ts index 6a8a6bd..2cab6bf 100644 --- a/packages/shared/src/boundary-port-geometry.ts +++ b/packages/shared/src/boundary-port-geometry.ts @@ -27,24 +27,31 @@ export interface BoundaryGlyphBox { } export namespace BoundaryPortGeometry { + // The drawing these came from is set at a finer weight than the diagram + // actually renders at: against a 3px wire, a 9.5px name and an 8px arrow + // read as fine print beside a cable. Everything below is that drawing scaled + // by about a quarter, keeping its proportions — the arrow still spans most + // of the name's line, the type still sits a little under two thirds of the + // name's size, and the gap still reads as one space. + /** Total height of one port: the name line plus the type line. */ - export const ROW_PITCH_PX = 22; + export const ROW_PITCH_PX = 28; /** The name's line box. Its centre is the axis. */ - export const NAME_LINE_HEIGHT_PX = 12; + export const NAME_LINE_HEIGHT_PX = 15; /** The type's line box, directly below the name's. */ - export const TYPE_LINE_HEIGHT_PX = 10; + export const TYPE_LINE_HEIGHT_PX = 13; - export const NAME_FONT_PX = 9.5; - export const TYPE_FONT_PX = 8; + export const NAME_FONT_PX = 12; + export const TYPE_FONT_PX = 10; /** Between the glyph and the first character of text. */ - export const GLYPH_TEXT_GAP_PX = 4; + export const GLYPH_TEXT_GAP_PX = 5; /** An input's glyph: a plain arrow, pointing into the diagram. */ - export const SOURCE_ARROW: BoundaryGlyphBox = { width: 8, height: 9 }; + export const SOURCE_ARROW: BoundaryGlyphBox = { width: 10, height: 12 }; /** An output's glyph: an arrow that stops against a bar, like a terminal. */ - export const SINK_ARROW: BoundaryGlyphBox = { width: 7, height: 9 }; - export const SINK_BAR: BoundaryGlyphBox = { width: 2, height: 11 }; + export const SINK_ARROW: BoundaryGlyphBox = { width: 9, height: 12 }; + export const SINK_BAR: BoundaryGlyphBox = { width: 2.5, height: 14 }; /** * The grab target, centred on the glyph. @@ -53,7 +60,7 @@ export namespace BoundaryPortGeometry { * part of it — the type can be hidden at low zoom, and a hit area that * changed with it would move the drag target as you zoom. */ - export const HIT: BoundaryGlyphBox = { width: 20, height: ROW_PITCH_PX }; + export const HIT: BoundaryGlyphBox = { width: 24, height: ROW_PITCH_PX }; /** Where the wire meets the port, measured down from the node's top. */ export const AXIS_Y_PX = NAME_LINE_HEIGHT_PX / 2; From 7139621640148f07065cbeeb7af8d10e62034049 Mon Sep 17 00:00:00 2001 From: Endri Bezati Date: Mon, 31 Aug 2026 16:49:30 +0200 Subject: [PATCH 3/6] refactor(views): make the name part of the port symbol, and the type look like a node's MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two changes with one shape: a boundary port is now drawn as ONE object by the node's own view, the way entity nodes already draw themselves. The name was a separate label element rendered by the label view. That is what made it float: an element positioned by nothing still carries whatever position it was last left with, and two pieces of code — the label view placing text and the node view placing the arrow — had no way to agree on where the symbol was. Removing the transform fixed the symptom; drawing the name in the node view, beside the glyph it belongs to, removes the possibility. Glyph, name and type are now built together from the node's own args. The type reuses the NODES' footer class rather than styling of its own, so a port's type reads exactly like an entity's — one definition of what a type looks like instead of two that can drift. That also retires a specificity fight rather than winning it. The type label carried both `boundary-label` and `boundary-type-label`, so the rule tinting a NAME green matched it too at identical specificity; order decided, and the type came out in the name's colour. Nothing tints it now because no direction selector can reach it, which the test states as the rule: what tints by direction may reach the name and the glyph, and nothing else. The label elements stay in the model — a rename addresses `_label_name`, and the server owns them — but render nothing. One consequence worth naming: double-clicking the name ON THE CANVAS to rename went with the rendered label. Renaming from the property panel is unaffected. If canvas rename is wanted back it should be a double-click on the port symbol itself, which is a listener rather than a label. --- .../diagram-client/src/diagram-client.css | 37 +++----- packages/diagram-client/src/views.ts | 86 +++++++++++-------- .../test/boundary-type-label-cascade.test.ts | 79 ----------------- .../test/boundary-type-styling.test.ts | 70 +++++++++++++++ 4 files changed, 134 insertions(+), 138 deletions(-) delete mode 100644 packages/diagram-client/test/boundary-type-label-cascade.test.ts create mode 100644 packages/diagram-client/test/boundary-type-styling.test.ts diff --git a/packages/diagram-client/src/diagram-client.css b/packages/diagram-client/src/diagram-client.css index de0e02d..617c5c8 100644 --- a/packages/diagram-client/src/diagram-client.css +++ b/packages/diagram-client/src/diagram-client.css @@ -881,38 +881,34 @@ i.cal-toggle-feedback-edges.cal-feedback-hidden-icon { ============================================ */ /* A boundary port is a symbol on the wire, not a box: an arrow glyph on the - axis with the name beside it and the type on a second line below. Sizes come - from BoundaryPortGeometry, which the server reads too — the glyph and the - port anchor have to land on the same pixel. */ + axis, with the name on that same line and the type below it. All three are + drawn by the node's own view — there is no separate label element to style, + which is why nothing here reaches for `.boundary-label` any more. Sizes come + from BoundaryPortGeometry, which the server reads too, so the glyph and the + port anchor land on the same pixel. */ -.boundary-node .boundary-label { +.boundary-node .boundary-name { font-family: var(--vscode-font-family); font-size: 12px; font-weight: 500; } .boundary-input .boundary-glyph, -.boundary-input .boundary-label { +.boundary-input .boundary-name { fill: var(--vscode-charts-green, #89d185); } .boundary-output .boundary-glyph, -.boundary-output .boundary-label { +.boundary-output .boundary-name { fill: var(--vscode-charts-blue, #007fd4); } -/* The type is deliberately NOT tinted by direction: it is supporting text on - its own line, and colouring it makes it compete with the name. - - Both the class and the position matter. The type label also carries - `boundary-label`, so `.boundary-input .boundary-label` matches it too — at - equal specificity, which the direction rules would then win on document - order. Qualifying with both classes outranks them outright, and it sits after - them so neither can be reintroduced by a reorder. */ -.boundary-node .boundary-label.boundary-type-label { +/* The type deliberately carries the NODES' footer class and nothing else, so a + port's type reads exactly like an entity's rather than inventing a second + convention. Tinting it by direction would also make it compete with the name + sitting right above it. */ +.boundary-node .boundary-type { font-size: 10px; - font-weight: 400; - fill: #a8a8a2; } /* The grab target. Invisible until the port is selected or hovered, which is @@ -933,17 +929,12 @@ i.cal-toggle-feedback-edges.cal-feedback-hidden-icon { rx: 2px; } -/* The port element stays in the model as the edge anchor, but the glyph above +/* The port element stays in the model as the edge anchor, but the symbol above is what is drawn — rendering both would double the arrow. */ .boundary-node .workflow-port { display: none; } -.boundary-node .direction-arrow { - fill: var(--vscode-foreground, #cccccc); - opacity: 0.6; -} - /* ============================================ Port styles (rounded squares) ============================================ */ diff --git a/packages/diagram-client/src/views.ts b/packages/diagram-client/src/views.ts index b17344b..89ca415 100644 --- a/packages/diagram-client/src/views.ts +++ b/packages/diagram-client/src/views.ts @@ -1017,8 +1017,14 @@ export class ProxyNodeView extends ShapeView { * Boundary input port node (left margin) - network's input exposed to internal entities * Styled as a terminal connector shape with port name centered */ +/** One of a boundary node's own text values, from the args the server set. */ +function boundaryText(node: unknown, key: string): string { + const value = (node as { args?: Record })?.args?.[key]; + return typeof value === 'string' ? value : ''; +} + /** - * The glyph for a boundary port, drawn on the wire's axis. + * A boundary port, drawn on the wire's axis: glyph, name and type as one object. * * An input is a bare arrow pointing into the diagram; an output is an arrow * that stops against a bar, the way a terminal is drawn on a schematic. @@ -1033,7 +1039,7 @@ export class ProxyNodeView extends ShapeView { * wire misses the arrow — both read `BoundaryPortGeometry` for exactly that * reason. */ -function boundaryGlyph(isInput: boolean, nodeWidth: number): VNode[] { +function boundaryPort(isInput: boolean, nodeWidth: number, name: string, type: string): VNode[] { const G = BoundaryPortGeometry; const axis = G.AXIS_Y_PX; const arrow = isInput ? G.SOURCE_ARROW : G.SINK_ARROW; @@ -1041,6 +1047,8 @@ function boundaryGlyph(isInput: boolean, nodeWidth: number): VNode[] { const glyphX = isInput ? nodeWidth - G.glyphWidth(true) : 0; const top = axis - arrow.height / 2; const tipX = glyphX + arrow.width; + const textX = isInput ? nodeWidth - G.textOffset(true) : G.textOffset(false); + const anchor = isInput ? 'end' : 'start'; const parts: VNode[] = [ svg('rect', { @@ -1070,6 +1078,25 @@ function boundaryGlyph(isInput: boolean, nodeWidth: number): VNode[] { } })); } + + // The name belongs to the symbol, not beside it. It used to be a child + // label element positioned by its own view, which is what made it drift off + // the glyph — an element with no layout feature still carries a stale + // position, and anything that reads it moves the text. Drawn here it cannot + // disagree with the arrow it labels. + parts.push(svg('text', { + class: { 'boundary-name': true }, + attrs: { x: textX, y: axis, 'text-anchor': anchor, 'dominant-baseline': 'middle' } + }, name)); + + // The type reuses the nodes' own footer treatment, so a port's type reads + // exactly like an entity's does rather than inventing a second convention. + if (type) { + parts.push(svg('text', { + class: { 'type-footer-label': true, 'boundary-type': true }, + attrs: { x: textX, y: G.TYPE_Y_PX, 'text-anchor': anchor, 'dominant-baseline': 'middle' } + }, type)); + } return parts; } @@ -1094,11 +1121,12 @@ export class BoundaryInputNodeView extends ShapeView { transform: `translate(${node.position.x}, ${node.position.y})` } }, - // No body: the glyph on the axis IS the port, and the two text lines - // run outward from it into the margin. - ...boundaryGlyph(true, width), - // Children: boundary labels + the port itself (anchor only; the - // glyph above is what is seen). + // No body: the symbol on the axis IS the port — glyph, name and + // type together, drawn as one thing. + ...boundaryPort(true, width, boundaryText(node, WorkflowDiagramMetadata.PORT_NAME), + boundaryText(node, WorkflowDiagramMetadata.PORT_TYPE)), + // The port element (an anchor only) and the label elements, which + // render nothing — see WorkflowLabelView. ...context.renderChildren(node) ); } @@ -1130,7 +1158,8 @@ export class BoundaryOutputNodeView extends ShapeView { } }, // No body — see the input view above. - ...boundaryGlyph(false, width), + ...boundaryPort(false, width, boundaryText(node, WorkflowDiagramMetadata.PORT_NAME), + boundaryText(node, WorkflowDiagramMetadata.PORT_TYPE)), ...context.renderChildren(node) ); } @@ -2170,35 +2199,20 @@ export class WorkflowLabelView implements IView { const labelType = (label as any).type as string || ''; const pos = label.position ?? { x: 0, y: 0 }; - // Boundary labels (name + type) are positioned relative to the boundary - // node, which is why they must carry no layout feature — see - // `BoundaryLabel`. Text runs OUTWARD from the glyph, away from the wire: - // right-aligned on an input (whose glyph is on its right edge), - // left-aligned on an output (whose glyph is on its left). That keeps the - // glyphs in a column with the text running off into the margin, and - // keeps the type off the wire — the name sits on the axis, the type on - // its own line below. + // Boundary labels render NOTHING here. + // + // The boundary node view draws its own name and type as part of the port + // symbol, so drawing them again from the label elements would double the + // text. The elements stay in the model because they are what a rename + // addresses (`_label_name`), and because the server owns them — + // they simply have no appearance of their own. + // + // This is also what put the name on top of its glyph. A label positioned + // by its own view still carries whatever position it was left with, and + // the view applied it; the text and the arrow it belonged to were being + // placed by two different pieces of code that had no way to agree. if (labelType === WorkflowDiagramTypes.LABEL_BOUNDARY_NAME || labelType === WorkflowDiagramTypes.LABEL_BOUNDARY_TYPE) { - const parent: any = (label as any).parent; - const parentW = parent?.size?.width ?? parent?.bounds?.width ?? 100; - const isType = labelType === WorkflowDiagramTypes.LABEL_BOUNDARY_TYPE; - const isInput = parent?.type === WorkflowDiagramTypes.NODE_BOUNDARY_INPUT; - const offset = BoundaryPortGeometry.textOffset(isInput); - - // NOT translated by `label.position`. These labels carry no layout - // feature precisely so that nothing positions them but this view — - // their position is whatever it was left at, and applying it shifted - // the text off the coordinates computed here. It is the reason the - // name rendered on top of its own glyph. - return svg('text', { - class: { 'boundary-label': true, 'boundary-type-label': isType }, - attrs: { - x: isInput ? parentW - offset : offset, - y: isType ? BoundaryPortGeometry.TYPE_Y_PX : BoundaryPortGeometry.AXIS_Y_PX, - 'text-anchor': isInput ? 'end' : 'start', - 'dominant-baseline': 'middle' - } - }, label.text || ''); + return undefined; } // Port labels - ELK positions the label box, we handle text anchoring diff --git a/packages/diagram-client/test/boundary-type-label-cascade.test.ts b/packages/diagram-client/test/boundary-type-label-cascade.test.ts deleted file mode 100644 index cb7ac8a..0000000 --- a/packages/diagram-client/test/boundary-type-label-cascade.test.ts +++ /dev/null @@ -1,79 +0,0 @@ -/** - * The type line must not take the name's colour. - * - * This shipped wrong once, and the reason is worth pinning rather than the - * pixel. The type label carries BOTH `boundary-label` and - * `boundary-type-label`, so `.boundary-input .boundary-label` — the rule that - * tints a name green — matches it too. Its own rule was written as - * `.boundary-node .boundary-type-label`, which is the SAME specificity (two - * classes), so the winner came down to document order, and the direction rules - * came later. The type rendered green on inputs and blue on outputs, exactly - * like the name beside it. - * - * A colour assertion would not have caught it — the declaration was always - * there and always correct. What was wrong was whether it applied. So these - * assert the two things that decide that: the selector outranks the direction - * rules on specificity, and it is not relying on order to do it. - */ -import { readFileSync } from 'node:fs'; -import * as path from 'node:path'; -import { fileURLToPath } from 'node:url'; -import { describe, expect, it } from 'vitest'; - -const SOURCE = readFileSync( - path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../src/diagram-client.css'), - 'utf8' -); - -/** - * Comments stripped, because the comments here DISCUSS the selectors they - * explain — an order check against the raw file matched the prose above the - * rule and failed on it. Only what the browser sees is scanned. - */ -const CSS = SOURCE.replace(/\/\*[\s\S]*?\*\//g, ''); - -/** The selector list of the rule that sets the type colour. */ -function typeColourRule(): { selector: string; index: number } { - const match = /([^}]*?)\{[^}]*?fill:\s*#a8a8a2[^}]*?\}/s.exec(CSS); - expect(match, 'no rule sets the type label colour').not.toBeNull(); - return { selector: match![1].trim(), index: match!.index }; -} - -describe('boundary type label colour', () => { - it('is set at all', () => { - expect(typeColourRule().selector).toContain('boundary-type-label'); - }); - - /** - * Two classes on the same element is a tie, and a tie is decided by order — - * which is how this broke. Qualifying with both classes makes it three, so - * it wins outright. - */ - it('outranks the direction rules on specificity, not on order', () => { - const { selector } = typeColourRule(); - - expect( - selector, - 'must qualify both classes so it beats `.boundary-input .boundary-label`' - ).toMatch(/\.boundary-label\.boundary-type-label|\.boundary-type-label\.boundary-label/); - }); - - /** - * Belt and braces: even with the specificity fixed, sitting after the rules - * it competes with means a future reorder cannot quietly recreate the tie. - */ - it('also sits after the rules that tint the name', () => { - const { index } = typeColourRule(); - const directionRules = [...CSS.matchAll(/\.boundary-(input|output) \.boundary-label/g)]; - - expect(directionRules.length, 'the direction rules moved or vanished').toBeGreaterThan(0); - for (const rule of directionRules) { - expect(rule.index).toBeLessThan(index); - } - }); - - it('leaves the name tinted by direction', () => { - expect(CSS).toMatch(/\.boundary-input \.boundary-glyph,\s*\.boundary-input \.boundary-label/); - expect(CSS).toMatch(/\.boundary-output \.boundary-glyph,\s*\.boundary-output \.boundary-label/); - }); -}); diff --git a/packages/diagram-client/test/boundary-type-styling.test.ts b/packages/diagram-client/test/boundary-type-styling.test.ts new file mode 100644 index 0000000..b94b13f --- /dev/null +++ b/packages/diagram-client/test/boundary-type-styling.test.ts @@ -0,0 +1,70 @@ +/** + * A boundary port's type must look like a node's type, and must not be tinted. + * + * This shipped wrong once. The type label carried both `boundary-label` and + * `boundary-type-label`, so `.boundary-input .boundary-label` — the rule that + * tints a NAME green — matched it too, at identical specificity. Document order + * decided, the direction rules came later, and the type rendered in the name's + * colour on both sides. Note that a colour assertion would not have caught it: + * the grey declaration was present and correct the whole time, and what was + * wrong was whether it applied. + * + * Both halves are now structural rather than a specificity fight. The type is + * drawn by the node's own view carrying the NODES' footer class, so it inherits + * one definition of what a type looks like instead of maintaining a second, and + * no selector that tints a name can reach it. + */ +import { readFileSync } from 'node:fs'; +import * as path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { describe, expect, it } from 'vitest'; + +const here = path.dirname(fileURLToPath(import.meta.url)); +const read = (file: string): string => readFileSync(path.resolve(here, file), 'utf8'); + +/** Comments stripped: they discuss the very selectors they explain, and an + * earlier version of this test matched a selector inside its own comment. */ +const CSS = read('../src/diagram-client.css').replace(/\/\*[\s\S]*?\*\//g, ''); +const VIEWS = read('../src/views.ts'); + +describe('boundary port type styling', () => { + it('reuses the nodes’ own footer class rather than a second convention', () => { + // The class the entity node views put on their type footer. + expect(VIEWS).toMatch(/'type-footer-label':\s*true,\s*'boundary-type':\s*true/); + }); + + it('has that class actually define the shared look', () => { + const footer = /\.type-footer-label\s*\{([^}]*)\}/.exec(CSS); + + expect(footer, '.type-footer-label no longer exists').not.toBeNull(); + expect(footer![1]).toContain('descriptionForeground'); + expect(footer![1]).toContain('italic'); + }); + + /** + * The bug, stated as a rule: whatever tints by direction may reach the name + * and the glyph, and nothing else. The type is not in that set. + */ + it('is out of reach of every direction tint', () => { + const tinted = [...CSS.matchAll(/\.boundary-(?:input|output)\s+\.([\w-]+)/g)].map(m => m[1]); + + expect(tinted.length, 'the direction rules moved or vanished').toBeGreaterThan(0); + expect([...new Set(tinted)].sort()).toEqual(['boundary-glyph', 'boundary-name']); + }); + + /** And the name still IS tinted — otherwise the rule above passes trivially. */ + it('leaves the name tinted by direction', () => { + expect(CSS).toMatch(/\.boundary-input\s+\.boundary-name\s*\{[^}]*charts-green/s); + expect(CSS).toMatch(/\.boundary-output\s+\.boundary-name\s*\{[^}]*charts-blue/s); + }); + + /** + * The name and type are drawn by the node view now, so the label elements + * must draw nothing — rendering both would double every string on screen. + */ + it('renders nothing from the boundary label elements', () => { + expect(VIEWS).toMatch( + /LABEL_BOUNDARY_NAME\s*\|\|[^)]*LABEL_BOUNDARY_TYPE\)\s*\{\s*return undefined;/ + ); + }); +}); From 49eab57300aba9b7fb58b3df008e084a5be1c3fb Mon Sep 17 00:00:00 2001 From: Endri Bezati Date: Mon, 31 Aug 2026 17:00:00 +0200 Subject: [PATCH 4/6] fix(views): force the boundary text anchor as a style, not an attribute MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The name kept rendering centred on its own glyph, through two attempted fixes. Neither was the cause. A presentation attribute loses to any stylesheet rule, and upstream GLSP/Sprotty styles set `text-anchor: middle`. So `attrs: {'text-anchor': 'end'}` was simply ignored: the text centred on the x it was given, which put the arrow about halfway along the name every time. Removing the label's stale transform and then moving the text into the node view were both real improvements, and both left this untouched, which is why the picture barely changed. Setting it as an inline style wins. This was already known here. The port labels forty lines further down carry the fix and a comment saying exactly why — "Presentation attributes can be overridden by CSS... Force via inline style." I wrote new text-drawing code in the same file without reading it, then explained the resulting overlap twice with theories that fitted the symptom. So the test asserts the STYLE specifically rather than that anchoring exists at all: an assertion that merely found `text-anchor` in the source would have passed for the whole life of the bug. --- packages/diagram-client/src/views.ts | 7 ++++++ .../test/boundary-type-styling.test.ts | 22 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/packages/diagram-client/src/views.ts b/packages/diagram-client/src/views.ts index 89ca415..ceb04e3 100644 --- a/packages/diagram-client/src/views.ts +++ b/packages/diagram-client/src/views.ts @@ -1086,6 +1086,12 @@ function boundaryPort(isInput: boolean, nodeWidth: number, name: string, type: s // disagree with the arrow it labels. parts.push(svg('text', { class: { 'boundary-name': true }, + // As an inline style, NOT only as an attribute. A presentation attribute + // loses to any stylesheet rule, and upstream GLSP/Sprotty styles set + // `text-anchor: middle` — which is why the name kept rendering centred + // on its own glyph however the attribute was set. The port labels below + // hit this years ago and say so; this is the same fix. + style: { 'text-anchor': anchor }, attrs: { x: textX, y: axis, 'text-anchor': anchor, 'dominant-baseline': 'middle' } }, name)); @@ -1094,6 +1100,7 @@ function boundaryPort(isInput: boolean, nodeWidth: number, name: string, type: s if (type) { parts.push(svg('text', { class: { 'type-footer-label': true, 'boundary-type': true }, + style: { 'text-anchor': anchor }, attrs: { x: textX, y: G.TYPE_Y_PX, 'text-anchor': anchor, 'dominant-baseline': 'middle' } }, type)); } diff --git a/packages/diagram-client/test/boundary-type-styling.test.ts b/packages/diagram-client/test/boundary-type-styling.test.ts index b94b13f..3f7146f 100644 --- a/packages/diagram-client/test/boundary-type-styling.test.ts +++ b/packages/diagram-client/test/boundary-type-styling.test.ts @@ -58,6 +58,28 @@ describe('boundary port type styling', () => { expect(CSS).toMatch(/\.boundary-output\s+\.boundary-name\s*\{[^}]*charts-blue/s); }); + /** + * Anchoring has to be an inline STYLE, not only an attribute. + * + * A presentation attribute loses to any stylesheet rule, and upstream + * GLSP/Sprotty styles set `text-anchor: middle`. So the name rendered + * centred on its own glyph no matter what the attribute said — the text and + * the arrow appeared on top of each other, twice, before this was found. + * The port labels in the same file already carried the fix and a comment + * explaining it. + * + * Asserting the style specifically is the point: an assertion that merely + * found `text-anchor` somewhere would have passed throughout the bug. + */ + it('forces the text anchor as a style, which CSS cannot override', () => { + const symbol = /function boundaryPort[\s\S]*?\n\}/.exec(VIEWS); + expect(symbol, 'boundaryPort has been renamed').not.toBeNull(); + + // One per text element it draws: the name and the type. + const styled = [...symbol![0].matchAll(/style:\s*\{\s*'text-anchor':/g)]; + expect(styled.length).toBe(2); + }); + /** * The name and type are drawn by the node view now, so the label elements * must draw nothing — rendering both would double every string on screen. From e41840e86b987b5cb0b8d1bcb3e2774eafe1cb71 Mon Sep 17 00:00:00 2001 From: Endri Bezati Date: Mon, 31 Aug 2026 17:11:30 +0200 Subject: [PATCH 5/6] fix(views): make a boundary port's whole row the object, name included MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The name was drawn by the port's own view, but the port was not grabbable by it: the hit target was a 24px box centred on the glyph, so clicking or dragging a port by its name did nothing, and selecting one outlined the arrow while the name sat outside the outline. Drawn as part of the symbol, behaving as a separate thing. The target is now the node's whole row. Its width comes from the node rather than a constant, which is the part that matters — any fixed number is a box around the arrow again, just a wider one. The small box had a real reason behind it, which the row keeps: a 10px arrow is hard to hit, and the target must not depend on the type line, since that can be hidden at low zoom. The height is still the fixed row pitch and still measured from nothing, so hiding the type moves nothing. Text that overruns the node's width is not covered — the box is 120 wide and the text runs outward into the margin past that. It affects only unusually long names, and widening the node to fit each one would break the alignment the column depends on. --- packages/diagram-client/src/views.ts | 10 +++---- ...g.test.ts => boundary-port-symbol.test.ts} | 29 +++++++++++++++++-- .../test/boundary-port-geometry.test.ts | 15 +++++----- packages/shared/src/boundary-port-geometry.ts | 18 ++++++++---- 4 files changed, 52 insertions(+), 20 deletions(-) rename packages/diagram-client/test/{boundary-type-styling.test.ts => boundary-port-symbol.test.ts} (75%) diff --git a/packages/diagram-client/src/views.ts b/packages/diagram-client/src/views.ts index ceb04e3..5e5a2c1 100644 --- a/packages/diagram-client/src/views.ts +++ b/packages/diagram-client/src/views.ts @@ -1051,14 +1051,12 @@ function boundaryPort(isInput: boolean, nodeWidth: number, name: string, type: s const anchor = isInput ? 'end' : 'start'; const parts: VNode[] = [ + // The whole row is the grab target, so the name belongs to the port + // rather than sitting beside it — see HIT_HEIGHT_PX. First in the list + // so it paints behind the glyph and the text. svg('rect', { class: { 'boundary-hit': true }, - attrs: { - x: glyphX + G.glyphWidth(isInput) / 2 - G.HIT.width / 2, - y: axis - G.HIT.height / 2, - width: G.HIT.width, - height: G.HIT.height - } + attrs: { x: 0, y: 0, width: nodeWidth, height: G.HIT_HEIGHT_PX } }), // Both arrows point the way the data flows: into the diagram for an // input, into the bar for an output. diff --git a/packages/diagram-client/test/boundary-type-styling.test.ts b/packages/diagram-client/test/boundary-port-symbol.test.ts similarity index 75% rename from packages/diagram-client/test/boundary-type-styling.test.ts rename to packages/diagram-client/test/boundary-port-symbol.test.ts index 3f7146f..1874b6c 100644 --- a/packages/diagram-client/test/boundary-type-styling.test.ts +++ b/packages/diagram-client/test/boundary-port-symbol.test.ts @@ -1,5 +1,11 @@ /** - * A boundary port's type must look like a node's type, and must not be tinted. + * A boundary port is ONE object, drawn by the node's own view. + * + * These assert the properties that kept failing while it was assembled from + * separate pieces: that the whole row is the object, that the text anchors the + * way it is told to, and that the type looks like a node's type. + * + * On the type in particular: it must look like a node's, and must not be tinted. * * This shipped wrong once. The type label carried both `boundary-label` and * `boundary-type-label`, so `.boundary-input .boundary-label` — the rule that @@ -27,7 +33,7 @@ const read = (file: string): string => readFileSync(path.resolve(here, file), 'u const CSS = read('../src/diagram-client.css').replace(/\/\*[\s\S]*?\*\//g, ''); const VIEWS = read('../src/views.ts'); -describe('boundary port type styling', () => { +describe('boundary port symbol', () => { it('reuses the nodes’ own footer class rather than a second convention', () => { // The class the entity node views put on their type footer. expect(VIEWS).toMatch(/'type-footer-label':\s*true,\s*'boundary-type':\s*true/); @@ -80,6 +86,25 @@ describe('boundary port type styling', () => { expect(styled.length).toBe(2); }); + /** + * The grab target is the node's whole row, which is what makes the name part + * of the port instead of something sitting beside it. + * + * It began as a small box centred on the glyph — reasonable, since a 10px + * arrow is hard to hit, but it meant a port could not be clicked or dragged + * by its own name, and selecting one outlined the arrow while leaving the + * name outside the outline. Asserting the WIDTH comes from the node is the + * point: any fixed number would be a box around the arrow again. + */ + it('makes the whole row the grab target, not a box around the arrow', () => { + const symbol = /function boundaryPort[\s\S]*?\n\}/.exec(VIEWS); + const hit = /'boundary-hit':\s*true[\s\S]*?attrs:\s*\{([^}]*)\}/.exec(symbol![0]); + + expect(hit, 'the hit rectangle has gone').not.toBeNull(); + expect(hit![1]).toMatch(/x:\s*0/); + expect(hit![1]).toMatch(/width:\s*nodeWidth/); + }); + /** * The name and type are drawn by the node view now, so the label elements * must draw nothing — rendering both would double every string on screen. diff --git a/packages/diagram-server/test/boundary-port-geometry.test.ts b/packages/diagram-server/test/boundary-port-geometry.test.ts index 509ddbf..074cf44 100644 --- a/packages/diagram-server/test/boundary-port-geometry.test.ts +++ b/packages/diagram-server/test/boundary-port-geometry.test.ts @@ -104,13 +104,14 @@ describe('boundary port geometry', () => { + BoundaryPortGeometry.GLYPH_TEXT_GAP_PX); }); - it('keeps the grab target bigger than the arrow but only one row tall', () => { - const G = BoundaryPortGeometry; - - expect(G.HIT.width).toBeGreaterThan(G.glyphWidth(false)); - // Not taller than a row: the type line can be hidden at low zoom, and a - // hit area covering it would move the drag target when it goes. - expect(G.HIT.height).toBe(G.ROW_PITCH_PX); + /** + * The grab target is the row, which is what makes the name part of the port + * rather than something sitting next to it. Its height is the fixed pitch + * and not anything measured from the text, so hiding the type line at low + * zoom cannot move the target. + */ + it('keeps the grab target a fixed row tall', () => { + expect(BoundaryPortGeometry.HIT_HEIGHT_PX).toBe(BoundaryPortGeometry.ROW_PITCH_PX); }); it('still carries the port identity', () => { diff --git a/packages/shared/src/boundary-port-geometry.ts b/packages/shared/src/boundary-port-geometry.ts index 2cab6bf..00370df 100644 --- a/packages/shared/src/boundary-port-geometry.ts +++ b/packages/shared/src/boundary-port-geometry.ts @@ -54,13 +54,21 @@ export namespace BoundaryPortGeometry { export const SINK_BAR: BoundaryGlyphBox = { width: 2.5, height: 14 }; /** - * The grab target, centred on the glyph. + * The grab target: the node's whole row, not a box around the arrow. * - * An 8px arrow is too small to hit reliably, and the type line must NOT be - * part of it — the type can be hidden at low zoom, and a hit area that - * changed with it would move the drag target as you zoom. + * It started as a small rectangle centred on the glyph, on the reasoning + * that a 10px arrow is too small to hit reliably. True, but it made the name + * something that merely sat NEXT to the port rather than part of it — you + * could not click or drag a port by its own name, and selecting one outlined + * the arrow while leaving the name outside. A port is one object; its whole + * row is that object. + * + * The width is the node's, so it is the caller that supplies it. The height + * stays the fixed row pitch rather than anything measured from the text, + * which is what keeps the original property: hiding the type line at low + * zoom moves nothing, because the target never depended on it. */ - export const HIT: BoundaryGlyphBox = { width: 24, height: ROW_PITCH_PX }; + export const HIT_HEIGHT_PX = ROW_PITCH_PX; /** Where the wire meets the port, measured down from the node's top. */ export const AXIS_Y_PX = NAME_LINE_HEIGHT_PX / 2; From 4a84cadeb6f9f46297335150b0c6ccb818ee58e0 Mon Sep 17 00:00:00 2001 From: Endri Bezati Date: Mon, 31 Aug 2026 17:23:30 +0200 Subject: [PATCH 6/6] fix(views): let a wire arrive at a boundary port as a plain line, and size the target to the text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two faults, both from treating the port symbol as decoration laid beside the diagram rather than as part of it. An edge ending at a boundary port drew its own arrowhead on top of the port's. They do not merely duplicate each other: the edge's tip is deliberately nudged four pixels PAST its endpoint so it meets the stroke cap, and that endpoint is the glyph's own edge — so the arrowhead came to rest inside the glyph. The drawing shows a plain line running into the symbol, which is what a schematic does, because the symbol is already the arrow. Suppressed in `renderArrow` itself so every edge view that inherits it is covered. The grab target was the node's box, a fixed 120 wide. That width exists so the glyph columns line up and says nothing about how wide any one port reads, so a short name left dead clickable space beside it while a long one overflowed the box and stopped being clickable at the very point it became visible. The target now follows the text, out from the glyph. That needs a text width, which the model cannot measure, so it is estimated. Acceptable only because nothing about the drawing depends on it — glyph, text and anchor are all placed from fixed geometry, and being a few pixels out makes the target slightly generous or slightly tight while moving nothing. Marked as never to be used for layout. The arrowhead test is worth noting. Written first as a search of the source for the guard, it passed with the guard disabled by `false &&` — it was checking that the words were there, not that they did anything. It now calls `renderArrow` and asserts what comes back, with an ordinary node as the control, and fails under that same mutation. --- packages/diagram-client/src/views.ts | 46 ++++++++++++++-- .../test/boundary-port-symbol.test.ts | 53 ++++++++++++++++++- packages/shared/src/boundary-port-geometry.ts | 19 +++++++ 3 files changed, 112 insertions(+), 6 deletions(-) diff --git a/packages/diagram-client/src/views.ts b/packages/diagram-client/src/views.ts index 5e5a2c1..acabeff 100644 --- a/packages/diagram-client/src/views.ts +++ b/packages/diagram-client/src/views.ts @@ -1017,6 +1017,19 @@ export class ProxyNodeView extends ShapeView { * Boundary input port node (left margin) - network's input exposed to internal entities * Styled as a terminal connector shape with port name centered */ +/** Whether an edge terminates on a boundary port, whose glyph is already an arrow. */ +function endsAtBoundaryPort(edge: Readonly): boolean { + let element: any = (edge as any).target; + while (element) { + if (element.type === WorkflowDiagramTypes.NODE_BOUNDARY_INPUT + || element.type === WorkflowDiagramTypes.NODE_BOUNDARY_OUTPUT) { + return true; + } + element = element.parent; + } + return false; +} + /** One of a boundary node's own text values, from the args the server set. */ function boundaryText(node: unknown, key: string): string { const value = (node as { args?: Record })?.args?.[key]; @@ -1050,13 +1063,29 @@ function boundaryPort(isInput: boolean, nodeWidth: number, name: string, type: s const textX = isInput ? nodeWidth - G.textOffset(true) : G.textOffset(false); const anchor = isInput ? 'end' : 'start'; + // How far the text reaches, from the glyph outward. Approximate by design — + // see approximateTextWidth; it sizes the target and nothing else. + const textReach = Math.max( + G.approximateTextWidth(name, G.NAME_FONT_PX), + G.approximateTextWidth(type, G.TYPE_FONT_PX) + ); + const hit = isInput + ? { x: textX - textReach, width: textReach + G.textOffset(true) } + : { x: 0, width: G.textOffset(false) + textReach }; + const parts: VNode[] = [ // The whole row is the grab target, so the name belongs to the port // rather than sitting beside it — see HIT_HEIGHT_PX. First in the list // so it paints behind the glyph and the text. + // + // Sized to the text rather than to the node box. The box is a fixed 120 + // so that the glyph columns line up, which has nothing to do with how + // wide any particular port reads: using it made a short name leave dead + // clickable space beside it, while a long one overflowed the box and + // stopped being clickable at exactly the point it became visible. svg('rect', { class: { 'boundary-hit': true }, - attrs: { x: 0, y: 0, width: nodeWidth, height: G.HIT_HEIGHT_PX } + attrs: { x: hit.x, y: 0, width: hit.width, height: G.HIT_HEIGHT_PX } }), // Both arrows point the way the data flows: into the diagram for an // input, into the bar for an output. @@ -1733,7 +1762,7 @@ export class WorkflowEdgeView extends PolylineEdgeView { }, svg('tspan', {}, toIndexLabel))] : []), // Arrow marker at target (use trimmed segments so arrow is at end of visible line) - this.renderArrow(normalizedSegments) + this.renderArrow(normalizedSegments, edge) ); } @@ -2112,7 +2141,16 @@ export class WorkflowEdgeView extends PolylineEdgeView { return path; } - protected renderArrow(segments: { x: number; y: number }[]): VNode { + protected renderArrow(segments: { x: number; y: number }[], edge?: Readonly): VNode { + // A boundary port is drawn AS an arrow, so an edge ending at one must + // not bring its own. Two arrowheads meeting is not the only problem: + // the tip is deliberately nudged past the endpoint to meet the stroke + // cap, and the endpoint is the glyph's own edge — so it landed inside + // the glyph. The wire is a plain line into the symbol, which is how the + // drawing shows it. + if (edge && endsAtBoundaryPort(edge)) { + return svg('g', {}); + } if (segments.length < 2) { return svg('g', {}); } @@ -2160,7 +2198,7 @@ export class WorkflowEdgeView extends PolylineEdgeView { */ @injectable() export class WorkflowNoArrowEdgeView extends WorkflowEdgeView { - protected override renderArrow(_segments: { x: number; y: number }[]): VNode { + protected override renderArrow(_segments: { x: number; y: number }[], _edge?: Readonly): VNode { return svg('g', {}); } } diff --git a/packages/diagram-client/test/boundary-port-symbol.test.ts b/packages/diagram-client/test/boundary-port-symbol.test.ts index 1874b6c..0525a12 100644 --- a/packages/diagram-client/test/boundary-port-symbol.test.ts +++ b/packages/diagram-client/test/boundary-port-symbol.test.ts @@ -24,6 +24,8 @@ import { readFileSync } from 'node:fs'; import * as path from 'node:path'; import { fileURLToPath } from 'node:url'; import { describe, expect, it } from 'vitest'; +import { WorkflowDiagramTypes } from '@dialogram/shared'; +import { WorkflowEdgeView } from '../src/views'; const here = path.dirname(fileURLToPath(import.meta.url)); const read = (file: string): string => readFileSync(path.resolve(here, file), 'utf8'); @@ -101,8 +103,55 @@ describe('boundary port symbol', () => { const hit = /'boundary-hit':\s*true[\s\S]*?attrs:\s*\{([^}]*)\}/.exec(symbol![0]); expect(hit, 'the hit rectangle has gone').not.toBeNull(); - expect(hit![1]).toMatch(/x:\s*0/); - expect(hit![1]).toMatch(/width:\s*nodeWidth/); + // Derived from the text's reach, not from a constant and not from the + // node box — the box is a fixed width so the glyph columns line up, + // which says nothing about how wide any one port reads. + expect(hit![1]).toMatch(/x:\s*hit\.x/); + expect(hit![1]).toMatch(/width:\s*hit\.width/); + expect(symbol![0]).toMatch(/approximateTextWidth/); + }); + + /** + * A boundary port IS an arrow, so an edge ending at one must not draw + * another. They do not merely duplicate: the edge's tip is nudged past its + * endpoint to meet the stroke cap, and that endpoint is the glyph's own + * edge, so the arrowhead came to rest inside the glyph. + * + * Called for real rather than grepped. The first version of this test + * searched the source for the guard, and passed happily when the guard was + * disabled with `false &&` — it was checking that the words were present, + * not that they did anything. + */ + describe('an edge ending at a boundary port', () => { + const view = new WorkflowEdgeView(); + const segments = [{ x: 0, y: 0 }, { x: 10, y: 0 }]; + const arrowFor = (target: unknown): { tag: string } => + (view as unknown as { + renderArrow(s: typeof segments, e: unknown): { tag: string }; + }).renderArrow(segments, { target } as never); + + it('arrives as a plain line', () => { + const arrow = arrowFor({ type: WorkflowDiagramTypes.NODE_BOUNDARY_OUTPUT }); + + expect(arrow.tag).toBe('g'); + }); + + it('arrives as a plain line when it ends on the port inside the node', () => { + // Edges attach to the GPort, not the node, so the check has to walk up. + const arrow = arrowFor({ + type: 'port:output', + parent: { type: WorkflowDiagramTypes.NODE_BOUNDARY_INPUT } + }); + + expect(arrow.tag).toBe('g'); + }); + + /** The control: an ordinary edge must still get its arrowhead. */ + it('still draws one into an ordinary node', () => { + const arrow = arrowFor({ type: 'node:task', parent: undefined }); + + expect(arrow.tag).toBe('polygon'); + }); }); /** diff --git a/packages/shared/src/boundary-port-geometry.ts b/packages/shared/src/boundary-port-geometry.ts index 00370df..a592949 100644 --- a/packages/shared/src/boundary-port-geometry.ts +++ b/packages/shared/src/boundary-port-geometry.ts @@ -76,6 +76,25 @@ export namespace BoundaryPortGeometry { /** Centre of the type line, measured down from the node's top. */ export const TYPE_Y_PX = NAME_LINE_HEIGHT_PX + TYPE_LINE_HEIGHT_PX / 2; + /** + * A rough width for a run of text, used ONLY to size the grab target. + * + * The real width is a browser measurement the model has no access to, and + * asking for one would mean a render pass. An estimate is fine here because + * nothing about the drawing depends on it: the glyph, the text and the port + * anchor are all positioned from fixed geometry, and this decides only how + * far the clickable area reaches. Being a few pixels out makes the target + * slightly generous or slightly tight, and nothing moves either way. + * + * It must never be used to lay anything out. + */ + export function approximateTextWidth(text: string, fontPx: number): number { + // Averaged across the mixed-case identifiers these labels actually hold; + // the UI font is proportional, so no single ratio is right for all of + // them, and erring wide costs nothing but a slightly larger target. + return text.length * fontPx * 0.6; + } + /** Full width of a glyph, including an output's bar. */ export function glyphWidth(isInput: boolean): number { return isInput ? SOURCE_ARROW.width : SINK_ARROW.width + SINK_BAR.width;