diff --git a/ci/release/changelogs/next.md b/ci/release/changelogs/next.md index f026c7e6d..b09b0c1fc 100644 --- a/ci/release/changelogs/next.md +++ b/ci/release/changelogs/next.md @@ -49,3 +49,6 @@ - `$BROWSER` now works to open a custom browser correctly. For example, to open Firefox on macOS: `BROWSER='open -aFirefox'` [#311](https://github.com/terrastruct/d2/pull/311) +- Fixes numbered IDs being wrongly positioned in `dagre` + [#321](https://github.com/terrastruct/d2/issues/321). Thank you @pleshevskiy for the + report. diff --git a/d2layouts/d2dagrelayout/layout.go b/d2layouts/d2dagrelayout/layout.go index 0162820cb..a380d6f10 100644 --- a/d2layouts/d2dagrelayout/layout.go +++ b/d2layouts/d2dagrelayout/layout.go @@ -30,6 +30,7 @@ var setupJS string var dagreJS string type DagreNode struct { + ID string `json:"id"` X float64 `json:"x"` Y float64 `json:"y"` Width float64 `json:"width"` @@ -49,7 +50,7 @@ type dagreGraphAttrs struct { rankdir string } -func Layout(ctx context.Context, d2graph *d2graph.Graph) (err error) { +func Layout(ctx context.Context, g *d2graph.Graph) (err error) { defer xdefer.Errorf(&err, "failed to dagre layout") debugJS := false @@ -66,7 +67,7 @@ func Layout(ctx context.Context, d2graph *d2graph.Graph) (err error) { edgesep: 40, nodesep: 60, } - switch d2graph.Root.Attributes.Direction.Value { + switch g.Root.Attributes.Direction.Value { case "down": rootAttrs.rankdir = "TB" case "right": @@ -84,14 +85,16 @@ func Layout(ctx context.Context, d2graph *d2graph.Graph) (err error) { } loadScript := "" - for _, obj := range d2graph.Objects { + idToObj := make(map[string]*d2graph.Object) + for _, obj := range g.Objects { id := obj.AbsID() + idToObj[id] = obj loadScript += generateAddNodeLine(id, int(obj.Width), int(obj.Height)) - if obj.Parent != d2graph.Root { + if obj.Parent != g.Root { loadScript += generateAddParentLine(id, obj.Parent.AbsID()) } } - for _, edge := range d2graph.Edges { + for _, edge := range g.Edges { // dagre doesn't work with edges to containers so we connect container edges to their first child instead (going all the way down) // we will chop the edge where it intersects the container border so it only shows the edge from the container src := edge.Src @@ -124,13 +127,7 @@ func Layout(ctx context.Context, d2graph *d2graph.Graph) (err error) { return err } - // val, err := v8ctx.RunScript("JSON.stringify(dagre.graphlib.json.write(g))", "q.js") - // if err != nil { - // return err - // } - // log.Debug(ctx, "graph", slog.F("json", val.String())) - - for i, obj := range d2graph.Objects { + for i := range g.Objects { val, err := v8ctx.RunScript(fmt.Sprintf("JSON.stringify(g.node(g.nodes()[%d]))", i), "value.js") if err != nil { return err @@ -139,6 +136,11 @@ func Layout(ctx context.Context, d2graph *d2graph.Graph) (err error) { if err := json.Unmarshal([]byte(val.String()), &dn); err != nil { return err } + if debugJS { + log.Debug(ctx, "graph", slog.F("json", dn)) + } + + obj := idToObj[dn.ID] // dagre gives center of node obj.TopLeft = geo.NewPoint(math.Round(dn.X-dn.Width/2), math.Round(dn.Y-dn.Height/2)) @@ -159,7 +161,7 @@ func Layout(ctx context.Context, d2graph *d2graph.Graph) (err error) { } } - for i, edge := range d2graph.Edges { + for i, edge := range g.Edges { val, err := v8ctx.RunScript(fmt.Sprintf("JSON.stringify(g.edge(g.edges()[%d]))", i), "value.js") if err != nil { return err @@ -168,6 +170,9 @@ func Layout(ctx context.Context, d2graph *d2graph.Graph) (err error) { if err := json.Unmarshal([]byte(val.String()), &de); err != nil { return err } + if debugJS { + log.Debug(ctx, "graph", slog.F("json", de)) + } points := make([]*geo.Point, len(de.Points)) for i := range de.Points { @@ -250,7 +255,7 @@ func setGraphAttrs(attrs dagreGraphAttrs) string { } func generateAddNodeLine(id string, width, height int) string { - return fmt.Sprintf("g.setNode(`%s`, { width: %d, height: %d });\n", id, width, height) + return fmt.Sprintf("g.setNode(`%s`, { id: `%s`, width: %d, height: %d });\n", id, id, width, height) } func generateAddParentLine(childID, parentID string) string { diff --git a/e2etests/e2e_test.go b/e2etests/e2e_test.go index 567008cd6..2dd5e4326 100644 --- a/e2etests/e2e_test.go +++ b/e2etests/e2e_test.go @@ -125,6 +125,8 @@ func run(t *testing.T, tc testCase) { svgBytes, err := d2svg.Render(diagram) assert.Success(t, err) + err = os.MkdirAll(dataPath, 0755) + assert.Success(t, err) err = ioutil.WriteFile(pathGotSVG, svgBytes, 0600) assert.Success(t, err) defer os.Remove(pathGotSVG) diff --git a/e2etests/stable_test.go b/e2etests/stable_test.go index 0d191b284..6bc25dcf1 100644 --- a/e2etests/stable_test.go +++ b/e2etests/stable_test.go @@ -1210,6 +1210,14 @@ finally.sequence.scorer.abc -> finally.sequence.item.a finally.sequence.itemOutcome.a.b.c.d.e -> finally.sequence.scorer finally.sequence.scorer -> finally.sequence.itemResponse.c`, }, + { + name: "number_connections", + script: `1 -> 2 +foo baz: Foo Baz + +foo baz -> hello +`, + }, } runa(t, tcs) diff --git a/e2etests/testdata/stable/number_connections/dagre/board.exp.json b/e2etests/testdata/stable/number_connections/dagre/board.exp.json new file mode 100644 index 000000000..8ab82e6c4 --- /dev/null +++ b/e2etests/testdata/stable/number_connections/dagre/board.exp.json @@ -0,0 +1,259 @@ +{ + "name": "", + "shapes": [ + { + "id": "foo baz", + "type": "", + "pos": { + "x": 173, + "y": 0 + }, + "width": 159, + "height": 126, + "opacity": 1, + "strokeDash": 0, + "strokeWidth": 2, + "borderRadius": 0, + "fill": "#F7F8FE", + "stroke": "#0D32B2", + "shadow": false, + "3d": false, + "multiple": false, + "tooltip": "", + "link": "", + "icon": null, + "iconPosition": "", + "fields": null, + "methods": null, + "columns": null, + "label": "Foo Baz", + "fontSize": 16, + "fontFamily": "DEFAULT", + "language": "", + "color": "#0A0F25", + "italic": false, + "bold": true, + "underline": false, + "labelWidth": 59, + "labelHeight": 26, + "labelPosition": "INSIDE_MIDDLE_CENTER", + "zIndex": 0, + "level": 1 + }, + { + "id": "1", + "type": "", + "pos": { + "x": 1, + "y": 0 + }, + "width": 112, + "height": 126, + "opacity": 1, + "strokeDash": 0, + "strokeWidth": 2, + "borderRadius": 0, + "fill": "#F7F8FE", + "stroke": "#0D32B2", + "shadow": false, + "3d": false, + "multiple": false, + "tooltip": "", + "link": "", + "icon": null, + "iconPosition": "", + "fields": null, + "methods": null, + "columns": null, + "label": "1", + "fontSize": 16, + "fontFamily": "DEFAULT", + "language": "", + "color": "#0A0F25", + "italic": false, + "bold": true, + "underline": false, + "labelWidth": 12, + "labelHeight": 26, + "labelPosition": "INSIDE_MIDDLE_CENTER", + "zIndex": 0, + "level": 1 + }, + { + "id": "2", + "type": "", + "pos": { + "x": 0, + "y": 226 + }, + "width": 113, + "height": 126, + "opacity": 1, + "strokeDash": 0, + "strokeWidth": 2, + "borderRadius": 0, + "fill": "#F7F8FE", + "stroke": "#0D32B2", + "shadow": false, + "3d": false, + "multiple": false, + "tooltip": "", + "link": "", + "icon": null, + "iconPosition": "", + "fields": null, + "methods": null, + "columns": null, + "label": "2", + "fontSize": 16, + "fontFamily": "DEFAULT", + "language": "", + "color": "#0A0F25", + "italic": false, + "bold": true, + "underline": false, + "labelWidth": 13, + "labelHeight": 26, + "labelPosition": "INSIDE_MIDDLE_CENTER", + "zIndex": 0, + "level": 1 + }, + { + "id": "hello", + "type": "", + "pos": { + "x": 182, + "y": 226 + }, + "width": 140, + "height": 126, + "opacity": 1, + "strokeDash": 0, + "strokeWidth": 2, + "borderRadius": 0, + "fill": "#F7F8FE", + "stroke": "#0D32B2", + "shadow": false, + "3d": false, + "multiple": false, + "tooltip": "", + "link": "", + "icon": null, + "iconPosition": "", + "fields": null, + "methods": null, + "columns": null, + "label": "hello", + "fontSize": 16, + "fontFamily": "DEFAULT", + "language": "", + "color": "#0A0F25", + "italic": false, + "bold": true, + "underline": false, + "labelWidth": 40, + "labelHeight": 26, + "labelPosition": "INSIDE_MIDDLE_CENTER", + "zIndex": 0, + "level": 1 + } + ], + "connections": [ + { + "id": "(1 -> 2)[0]", + "src": "1", + "srcArrow": "none", + "srcLabel": "", + "dst": "2", + "dstArrow": "triangle", + "dstLabel": "", + "opacity": 1, + "strokeDash": 0, + "strokeWidth": 2, + "stroke": "#0D32B2", + "label": "", + "fontSize": 16, + "fontFamily": "DEFAULT", + "language": "", + "color": "#676C7E", + "italic": true, + "bold": false, + "underline": false, + "labelWidth": 0, + "labelHeight": 0, + "labelPosition": "", + "labelPercentage": 0, + "route": [ + { + "x": 56.5, + "y": 126 + }, + { + "x": 56.5, + "y": 166 + }, + { + "x": 56.5, + "y": 186 + }, + { + "x": 56.5, + "y": 226 + } + ], + "isCurve": true, + "animated": false, + "tooltip": "", + "icon": null, + "zIndex": 0 + }, + { + "id": "(foo baz -> hello)[0]", + "src": "foo baz", + "srcArrow": "none", + "srcLabel": "", + "dst": "hello", + "dstArrow": "triangle", + "dstLabel": "", + "opacity": 1, + "strokeDash": 0, + "strokeWidth": 2, + "stroke": "#0D32B2", + "label": "", + "fontSize": 16, + "fontFamily": "DEFAULT", + "language": "", + "color": "#676C7E", + "italic": true, + "bold": false, + "underline": false, + "labelWidth": 0, + "labelHeight": 0, + "labelPosition": "", + "labelPercentage": 0, + "route": [ + { + "x": 252, + "y": 126 + }, + { + "x": 252, + "y": 166 + }, + { + "x": 252, + "y": 186 + }, + { + "x": 252, + "y": 226 + } + ], + "isCurve": true, + "animated": false, + "tooltip": "", + "icon": null, + "zIndex": 0 + } + ] +} diff --git a/e2etests/testdata/stable/number_connections/dagre/sketch.exp.svg b/e2etests/testdata/stable/number_connections/dagre/sketch.exp.svg new file mode 100644 index 000000000..6ef443e1e --- /dev/null +++ b/e2etests/testdata/stable/number_connections/dagre/sketch.exp.svg @@ -0,0 +1,24 @@ + +Foo Baz12hello \ No newline at end of file diff --git a/e2etests/testdata/todo/container_child_edge/board.exp.json b/e2etests/testdata/stable/number_connections/elk/board.exp.json similarity index 58% rename from e2etests/testdata/todo/container_child_edge/board.exp.json rename to e2etests/testdata/stable/number_connections/elk/board.exp.json index 7dc266f6c..057c7da5a 100644 --- a/e2etests/testdata/todo/container_child_edge/board.exp.json +++ b/e2etests/testdata/stable/number_connections/elk/board.exp.json @@ -2,58 +2,19 @@ "name": "", "shapes": [ { - "id": "container", + "id": "foo baz", "type": "", "pos": { - "x": 0, - "y": 0 + "x": 12, + "y": 12 }, - "width": 255, - "height": 452, - "level": 1, - "opacity": 1, - "strokeDash": 0, - "strokeWidth": 2, - "borderRadius": 0, - "fill": "#E3E9FD", - "stroke": "#0D32B2", - "shadow": false, - "3d": false, - "multiple": false, - "tooltip": "", - "link": "", - "icon": null, - "iconPosition": "", - "fields": null, - "methods": null, - "columns": null, - "label": "container", - "fontSize": 28, - "fontFamily": "DEFAULT", - "language": "", - "color": "#0A0F25", - "italic": false, - "bold": true, - "underline": false, - "labelWidth": 117, - "labelHeight": 41, - "labelPosition": "INSIDE_TOP_CENTER" - }, - { - "id": "container.first", - "type": "", - "pos": { - "x": 60, - "y": 50 - }, - "width": 135, + "width": 159, "height": 126, - "level": 2, "opacity": 1, "strokeDash": 0, "strokeWidth": 2, "borderRadius": 0, - "fill": "#EDF0FD", + "fill": "#F7F8FE", "stroke": "#0D32B2", "shadow": false, "3d": false, @@ -65,7 +26,7 @@ "fields": null, "methods": null, "columns": null, - "label": "first", + "label": "Foo Baz", "fontSize": 16, "fontFamily": "DEFAULT", "language": "", @@ -73,25 +34,26 @@ "italic": false, "bold": true, "underline": false, - "labelWidth": 35, + "labelWidth": 59, "labelHeight": 26, - "labelPosition": "INSIDE_MIDDLE_CENTER" + "labelPosition": "INSIDE_MIDDLE_CENTER", + "zIndex": 0, + "level": 1 }, { - "id": "container.second", + "id": "1", "type": "", "pos": { - "x": 50, - "y": 276 + "x": 191, + "y": 12 }, - "width": 155, + "width": 112, "height": 126, - "level": 2, "opacity": 1, "strokeDash": 0, "strokeWidth": 2, "borderRadius": 0, - "fill": "#EDF0FD", + "fill": "#F7F8FE", "stroke": "#0D32B2", "shadow": false, "3d": false, @@ -103,7 +65,7 @@ "fields": null, "methods": null, "columns": null, - "label": "second", + "label": "1", "fontSize": 16, "fontFamily": "DEFAULT", "language": "", @@ -111,25 +73,105 @@ "italic": false, "bold": true, "underline": false, - "labelWidth": 55, + "labelWidth": 12, "labelHeight": 26, - "labelPosition": "INSIDE_MIDDLE_CENTER" + "labelPosition": "INSIDE_MIDDLE_CENTER", + "zIndex": 0, + "level": 1 + }, + { + "id": "2", + "type": "", + "pos": { + "x": 191, + "y": 238 + }, + "width": 113, + "height": 126, + "opacity": 1, + "strokeDash": 0, + "strokeWidth": 2, + "borderRadius": 0, + "fill": "#F7F8FE", + "stroke": "#0D32B2", + "shadow": false, + "3d": false, + "multiple": false, + "tooltip": "", + "link": "", + "icon": null, + "iconPosition": "", + "fields": null, + "methods": null, + "columns": null, + "label": "2", + "fontSize": 16, + "fontFamily": "DEFAULT", + "language": "", + "color": "#0A0F25", + "italic": false, + "bold": true, + "underline": false, + "labelWidth": 13, + "labelHeight": 26, + "labelPosition": "INSIDE_MIDDLE_CENTER", + "zIndex": 0, + "level": 1 + }, + { + "id": "hello", + "type": "", + "pos": { + "x": 22, + "y": 238 + }, + "width": 140, + "height": 126, + "opacity": 1, + "strokeDash": 0, + "strokeWidth": 2, + "borderRadius": 0, + "fill": "#F7F8FE", + "stroke": "#0D32B2", + "shadow": false, + "3d": false, + "multiple": false, + "tooltip": "", + "link": "", + "icon": null, + "iconPosition": "", + "fields": null, + "methods": null, + "columns": null, + "label": "hello", + "fontSize": 16, + "fontFamily": "DEFAULT", + "language": "", + "color": "#0A0F25", + "italic": false, + "bold": true, + "underline": false, + "labelWidth": 40, + "labelHeight": 26, + "labelPosition": "INSIDE_MIDDLE_CENTER", + "zIndex": 0, + "level": 1 } ], "connections": [ { - "id": "container.(first -> second)[0]", - "src": "container.first", + "id": "(1 -> 2)[0]", + "src": "1", "srcArrow": "none", "srcLabel": "", - "dst": "container.second", + "dst": "2", "dstArrow": "triangle", "dstLabel": "", "opacity": 1, "strokeDash": 0, "strokeWidth": 2, "stroke": "#0D32B2", - "label": "1->2", + "label": "", "fontSize": 16, "fontFamily": "DEFAULT", "language": "", @@ -137,46 +179,38 @@ "italic": true, "bold": false, "underline": false, - "labelWidth": 29, - "labelHeight": 21, - "labelPosition": "INSIDE_MIDDLE_CENTER", + "labelWidth": 0, + "labelHeight": 0, + "labelPosition": "", "labelPercentage": 0, "route": [ { - "x": 103.10840707964601, - "y": 176 + "x": 247, + "y": 138 }, { - "x": 87.6216814159292, - "y": 216 - }, - { - "x": 87.55, - "y": 236 - }, - { - "x": 102.75, - "y": 276 + "x": 247, + "y": 238 } ], - "isCurve": true, "animated": false, "tooltip": "", - "icon": null + "icon": null, + "zIndex": 0 }, { - "id": "(container -> container.second)[0]", - "src": "container", + "id": "(foo baz -> hello)[0]", + "src": "foo baz", "srcArrow": "none", "srcLabel": "", - "dst": "container.second", + "dst": "hello", "dstArrow": "triangle", "dstLabel": "", "opacity": 1, "strokeDash": 0, "strokeWidth": 2, "stroke": "#0D32B2", - "label": "c->2", + "label": "", "fontSize": 16, "fontFamily": "DEFAULT", "language": "", @@ -184,32 +218,24 @@ "italic": true, "bold": false, "underline": false, - "labelWidth": 28, - "labelHeight": 21, - "labelPosition": "INSIDE_MIDDLE_CENTER", + "labelWidth": 0, + "labelHeight": 0, + "labelPosition": "", "labelPercentage": 0, "route": [ { - "x": 151.891592920354, - "y": 176 + "x": 91.5, + "y": 138 }, { - "x": 167.3783185840708, - "y": 216 - }, - { - "x": 167.45, - "y": 236 - }, - { - "x": 152.25, - "y": 276 + "x": 91.5, + "y": 238 } ], - "isCurve": true, "animated": false, "tooltip": "", - "icon": null + "icon": null, + "zIndex": 0 } ] } diff --git a/e2etests/testdata/todo/container_child_edge/sketch.exp.svg b/e2etests/testdata/stable/number_connections/elk/sketch.exp.svg similarity index 68% rename from e2etests/testdata/todo/container_child_edge/sketch.exp.svg rename to e2etests/testdata/stable/number_connections/elk/sketch.exp.svg index edbac3aa5..2c622cddb 100644 --- a/e2etests/testdata/todo/container_child_edge/sketch.exp.svg +++ b/e2etests/testdata/stable/number_connections/elk/sketch.exp.svg @@ -2,35 +2,23 @@ \ No newline at end of file