Merge pull request #1586 from gavin-ts/simple-edges-in-grid

Simple edges in grid

ci/release/changelogs/next.md

@@ -1,6 +1,7 @@
 #### Features 🚀
 
 - UTF-16 files are automatically detected and supported [#1525](https://github.com/terrastruct/d2/pull/1525)
+- Grid diagrams can now have simple edges between cells [#1586](https://github.com/terrastruct/d2/pull/1586)
 
 #### Improvements 🧹
 

d2compiler/compile.go

@@ -1072,14 +1072,34 @@ func (c *compiler) validateNear(g *d2graph.Graph) {
 
 func (c *compiler) validateEdges(g *d2graph.Graph) {
 	for _, edge := range g.Edges {
-		if gd := edge.Src.Parent.ClosestGridDiagram(); gd != nil {
+		srcGrid := edge.Src.Parent.ClosestGridDiagram()
-			c.errorf(edge.GetAstEdge(), "edges in grid diagrams are not supported yet")
+		dstGrid := edge.Dst.Parent.ClosestGridDiagram()
+		if srcGrid != nil || dstGrid != nil {
+			if top := srcGrid.TopGridDiagram(); srcGrid != top {
+				// valid: grid.child1 -> grid.child2
+				// invalid: grid.childGrid.child1 -> grid.childGrid.child2
+				c.errorf(edge.GetAstEdge(), "edge must be on direct child of grid diagram %#v", top.AbsID())
 				continue
 			}
-		if gd := edge.Dst.Parent.ClosestGridDiagram(); gd != nil {
+			if top := dstGrid.TopGridDiagram(); dstGrid != top {
-			c.errorf(edge.GetAstEdge(), "edges in grid diagrams are not supported yet")
+				// valid: grid.child1 -> grid.child2
+				// invalid: grid.childGrid.child1 -> grid.childGrid.child2
+				c.errorf(edge.GetAstEdge(), "edge must be on direct child of grid diagram %#v", top.AbsID())
 				continue
 			}
+			if srcGrid != dstGrid {
+				// valid: a -> grid
+				// invalid: a -> grid.child
+				c.errorf(edge.GetAstEdge(), "edges into grid diagrams are not supported yet")
+				continue
+			}
+			if srcGrid != edge.Src.Parent || dstGrid != edge.Dst.Parent {
+				// valid: grid.child1 -> grid.child2
+				// invalid: grid.child1 -> grid.child2.child1
+				c.errorf(edge.GetAstEdge(), "grid diagrams can only have edges between children right now")
+				continue
+			}
+		}
 	}
 }
 

d2compiler/compile_test.go

@@ -2476,16 +2476,34 @@ d2/testdata/d2compiler/TestCompile/grid_gap_negative.d2:3:16: vertical-gap must
 			name: "grid_edge",
 			text: `hey: {
 	grid-rows: 1
-	a -> b
+	a -> b: ok
 }
 	c -> hey.b
 	hey.a -> c
+	hey -> hey.a
 
 	hey -> c: ok
 `,
-			expErr: `d2/testdata/d2compiler/TestCompile/grid_edge.d2:3:2: edges in grid diagrams are not supported yet
+			expErr: `d2/testdata/d2compiler/TestCompile/grid_edge.d2:5:2: edges into grid diagrams are not supported yet
-d2/testdata/d2compiler/TestCompile/grid_edge.d2:5:2: edges in grid diagrams are not supported yet
+d2/testdata/d2compiler/TestCompile/grid_edge.d2:6:2: edges into grid diagrams are not supported yet
-d2/testdata/d2compiler/TestCompile/grid_edge.d2:6:2: edges in grid diagrams are not supported yet`,
+d2/testdata/d2compiler/TestCompile/grid_edge.d2:7:2: edges into grid diagrams are not supported yet`,
+		},
+		{
+			name: "grid_deeper_edge",
+			text: `hey: {
+	grid-rows: 1
+	a -> b: ok
+	b: {
+		c -> d: not yet
+	}
+	a: {
+		grid-columns: 1
+		e -> f: also not yet
+	}
+}
+`,
+			expErr: `d2/testdata/d2compiler/TestCompile/grid_deeper_edge.d2:9:3: edge must be on direct child of grid diagram "hey"
+d2/testdata/d2compiler/TestCompile/grid_deeper_edge.d2:5:3: grid diagrams can only have edges between children right now`,
 		},
 		{
 			name: "grid_nested",

d2graph/d2graph.go

@@ -1169,6 +1169,13 @@ func (e *Edge) Text() *d2target.MText {
 	}
 }
 
+func (e *Edge) Move(dx, dy float64) {
+	for _, p := range e.Route {
+		p.X += dx
+		p.Y += dy
+	}
+}
+
 func (e *Edge) AbsID() string {
 	srcIDA := e.Src.AbsIDArray()
 	dstIDA := e.Dst.AbsIDArray()

d2graph/grid_diagram.go

@@ -14,3 +14,22 @@ func (obj *Object) ClosestGridDiagram() *Object {
 	}
 	return obj.Parent.ClosestGridDiagram()
 }
+
+// TopGridDiagram returns the least nested (outermost) grid diagram
+func (obj *Object) TopGridDiagram() *Object {
+	if obj == nil {
+		return nil
+	}
+	var gd *Object
+	if obj.IsGridDiagram() {
+		gd = obj
+	}
+	curr := obj.Parent
+	for curr != nil {
+		if curr.IsGridDiagram() {
+			gd = curr
+		}
+		curr = curr.Parent
+	}
+	return gd
+}

d2graph/layout.go

@@ -60,7 +60,7 @@ func (g *Graph) ExtractAsNestedGraph(obj *Object) *Graph {
 func pluckObjAndEdges(g *Graph, obj *Object) (descendantsObjects []*Object, edges []*Edge) {
 	for i := 0; i < len(g.Edges); i++ {
 		edge := g.Edges[i]
-		if edge.Src == obj || edge.Dst == obj {
+		if edge.Src.IsDescendantOf(obj) && edge.Dst.IsDescendantOf(obj) {
 			edges = append(edges, edge)
 			g.Edges = append(g.Edges[:i], g.Edges[i+1:]...)
 			i--
@@ -69,15 +69,10 @@ func pluckObjAndEdges(g *Graph, obj *Object) (descendantsObjects []*Object, edge
 
 	for i := 0; i < len(g.Objects); i++ {
 		temp := g.Objects[i]
-		if temp.AbsID() == obj.AbsID() {
+		if temp.IsDescendantOf(obj) {
-			descendantsObjects = append(descendantsObjects, obj)
+			descendantsObjects = append(descendantsObjects, temp)
 			g.Objects = append(g.Objects[:i], g.Objects[i+1:]...)
-			for _, child := range obj.ChildrenArray {
+			i--
-				subObjects, subEdges := pluckObjAndEdges(g, child)
-				descendantsObjects = append(descendantsObjects, subObjects...)
-				edges = append(edges, subEdges...)
-			}
-			break
 		}
 	}
 
@@ -86,7 +81,12 @@ func pluckObjAndEdges(g *Graph, obj *Object) (descendantsObjects []*Object, edge
 
 func (g *Graph) InjectNestedGraph(tempGraph *Graph, parent *Object) {
 	obj := tempGraph.Root.ChildrenArray[0]
-	obj.MoveWithDescendantsTo(0, 0)
+	dx := 0 - obj.TopLeft.X
+	dy := 0 - obj.TopLeft.Y
+	obj.MoveWithDescendants(dx, dy)
+	for _, e := range tempGraph.Edges {
+		e.Move(dx, dy)
+	}
 	obj.Parent = parent
 	for _, obj := range tempGraph.Objects {
 		obj.Graph = g

d2layouts/d2grid/grid_diagram.go

@@ -11,6 +11,7 @@ import (
 type gridDiagram struct {
 	root    *d2graph.Object
 	objects []*d2graph.Object
+	edges   []*d2graph.Edge
 	rows    int
 	columns int
 
@@ -107,6 +108,9 @@ func (gd *gridDiagram) shift(dx, dy float64) {
 	for _, obj := range gd.objects {
 		obj.MoveWithDescendants(dx, dy)
 	}
+	for _, e := range gd.edges {
+		e.Move(dx, dy)
+	}
 }
 
 func (gd *gridDiagram) cleanup(obj *d2graph.Object, graph *d2graph.Graph) {
@@ -122,4 +126,5 @@ func (gd *gridDiagram) cleanup(obj *d2graph.Object, graph *d2graph.Graph) {
 		restore(obj, child)
 		child.IterDescendants(restore)
 	}
+	graph.Edges = append(graph.Edges, gd.edges...)
 }

d2layouts/d2grid/layout.go

@@ -31,7 +31,7 @@ const (
 // 7. Put grid children back in correct location
 func Layout(ctx context.Context, g *d2graph.Graph, layout d2graph.LayoutGraph) d2graph.LayoutGraph {
 	return func(ctx context.Context, g *d2graph.Graph) error {
-		gridDiagrams, objectOrder, err := withoutGridDiagrams(ctx, g, layout)
+		gridDiagrams, objectOrder, edgeOrder, err := withoutGridDiagrams(ctx, g, layout)
 		if err != nil {
 			return err
 		}
@@ -42,19 +42,24 @@ func Layout(ctx context.Context, g *d2graph.Graph, layout d2graph.LayoutGraph) d
 			return err
 		}
 
-		cleanup(g, gridDiagrams, objectOrder)
+		cleanup(g, gridDiagrams, objectOrder, edgeOrder)
 		return nil
 	}
 }
 
-func withoutGridDiagrams(ctx context.Context, g *d2graph.Graph, layout d2graph.LayoutGraph) (gridDiagrams map[string]*gridDiagram, objectOrder map[string]int, err error) {
+func withoutGridDiagrams(ctx context.Context, g *d2graph.Graph, layout d2graph.LayoutGraph) (gridDiagrams map[string]*gridDiagram, objectOrder, edgeOrder map[string]int, err error) {
 	toRemove := make(map[*d2graph.Object]struct{})
+	edgeToRemove := make(map[*d2graph.Edge]struct{})
 	gridDiagrams = make(map[string]*gridDiagram)
 
 	objectOrder = make(map[string]int)
 	for i, obj := range g.Objects {
 		objectOrder[obj.AbsID()] = i
 	}
+	edgeOrder = make(map[string]int)
+	for i, edge := range g.Edges {
+		edgeOrder[edge.AbsID()] = i
+	}
 
 	var processGrid func(obj *d2graph.Object) error
 	processGrid = func(obj *d2graph.Object) error {
@@ -79,6 +84,9 @@ func withoutGridDiagrams(ctx context.Context, g *d2graph.Graph, layout d2graph.L
 				sort.SliceStable(obj.ChildrenArray, func(i, j int) bool {
 					return objectOrder[obj.ChildrenArray[i].AbsID()] < objectOrder[obj.ChildrenArray[j].AbsID()]
 				})
+				sort.SliceStable(g.Edges, func(i, j int) bool {
+					return edgeOrder[g.Edges[i].AbsID()] < edgeOrder[g.Edges[j].AbsID()]
+				})
 
 				for _, o := range tempGraph.Objects {
 					toRemove[o] = struct{}{}
@@ -200,6 +208,28 @@ func withoutGridDiagrams(ctx context.Context, g *d2graph.Graph, layout d2graph.L
 		for _, o := range gd.objects {
 			toRemove[o] = struct{}{}
 		}
+
+		// simple straight line edge routing between grid objects
+		for i, e := range g.Edges {
+			edgeOrder[e.AbsID()] = i
+			if !e.Src.Parent.IsDescendantOf(obj) && !e.Dst.Parent.IsDescendantOf(obj) {
+				continue
+			}
+			// if edge is within grid, remove it from outer layout
+			gd.edges = append(gd.edges, e)
+			edgeToRemove[e] = struct{}{}
+
+			if e.Src.Parent != obj || e.Dst.Parent != obj {
+				continue
+			}
+			// if edge is grid child, use simple routing
+			e.Route = []*geo.Point{e.Src.Center(), e.Dst.Center()}
+			e.TraceToShape(e.Route, 0, 1)
+			if e.Label.Value != "" {
+				e.LabelPosition = go2.Pointer(string(label.InsideMiddleCenter))
+			}
+		}
+
 		return nil
 	}
 
@@ -218,7 +248,7 @@ func withoutGridDiagrams(ctx context.Context, g *d2graph.Graph, layout d2graph.L
 			}
 
 			if err := processGrid(obj); err != nil {
-				return nil, nil, err
+				return nil, nil, nil, err
 			}
 		}
 	}
@@ -230,8 +260,15 @@ func withoutGridDiagrams(ctx context.Context, g *d2graph.Graph, layout d2graph.L
 		}
 	}
 	g.Objects = layoutObjects
+	layoutEdges := make([]*d2graph.Edge, 0, len(edgeToRemove))
+	for _, e := range g.Edges {
+		if _, exists := edgeToRemove[e]; !exists {
+			layoutEdges = append(layoutEdges, e)
+		}
+	}
+	g.Edges = layoutEdges
 
-	return gridDiagrams, objectOrder, nil
+	return gridDiagrams, objectOrder, edgeOrder, nil
 }
 
 func layoutGrid(g *d2graph.Graph, obj *d2graph.Object) (*gridDiagram, error) {
@@ -940,11 +977,14 @@ func getDistToTarget(layout [][]*d2graph.Object, targetSize float64, horizontalG
 // - translating the grid to its position placed by the core layout engine
 // - restore the children of the grid
 // - sorts objects to their original graph order
-func cleanup(graph *d2graph.Graph, gridDiagrams map[string]*gridDiagram, objectsOrder map[string]int) {
+func cleanup(graph *d2graph.Graph, gridDiagrams map[string]*gridDiagram, objectsOrder, edgeOrder map[string]int) {
 	defer func() {
 		sort.SliceStable(graph.Objects, func(i, j int) bool {
 			return objectsOrder[graph.Objects[i].AbsID()] < objectsOrder[graph.Objects[j].AbsID()]
 		})
+		sort.SliceStable(graph.Edges, func(i, j int) bool {
+			return edgeOrder[graph.Edges[i].AbsID()] < edgeOrder[graph.Edges[j].AbsID()]
+		})
 	}()
 
 	var restore func(obj *d2graph.Object)

e2etests/stable_test.go

@@ -2833,6 +2833,8 @@ y: profits {
 		loadFromFile(t, "overlapping_child_label"),
 		loadFromFile(t, "dagre_spacing"),
 		loadFromFile(t, "dagre_spacing_right"),
+		loadFromFile(t, "simple_grid_edges"),
+		loadFromFile(t, "grid_nested_simple_edges"),
 	}
 
 	runa(t, tcs)

e2etests/testdata/files/grid_nested_simple_edges.d2

@@ -0,0 +1,43 @@
+direction: right
+outer-grid -> outer-container
+
+outer-grid: {
+  grid-columns: 1
+
+  inner-grid -> container -> etc
+
+  container: {
+    label.near: top-left
+    # edges not yet supported here since they must be direct grid children
+    a
+    b
+    c
+  }
+
+  inner-grid: {
+    grid-rows: 1
+    1
+    2
+    3
+    # edges here are not supported yet since this is inside another grid
+  }
+}
+
+outer-container: {
+  grid -> container
+
+  grid: {
+    grid-rows: 1
+    # direct child edges ok in least nested grid
+    1 -> 2 -> 3
+  }
+
+  container: {
+    # non grid edges ok
+    4 -> 5 -> 6
+    nested container: {
+      # nested non grid edges ok
+      7 -> 8
+    }
+  }
+}

e2etests/testdata/files/simple_grid_edges.d2

@@ -0,0 +1,160 @@
+grid-rows: 4
+grid-columns: 5
+horizontal-gap: 20
+vertical-gap: 5
+
+*.class: [text; blue]
+
+0,0: {
+  label: "npm i -g\n@forge/cli"
+  style: {
+    fill: "#30304c"
+    stroke: transparent
+    font-color: white
+    font: mono
+    font-size: 10
+    bold: false
+  }
+}
+0,1: {
+  label: "Set up an\nAtlassian site"
+  class: [text; gray]
+}
+0,2.class: empty
+0,3: {
+  label: "View the hello\nworld app"
+  class: [text; gray]
+}
+0,4: forge\ntunnel
+
+1*.class: note
+1*.label: ""
+1,0
+1,1
+1,2
+1,3
+1,4
+
+2,0: forge\nlogin
+2,1: forge\ncreate
+2,2: forge\ndeploy
+2,3: forge\ninstall
+2,4: {
+  shape: diamond
+  label: "Hot reload\nchanges?"
+  class: [text; gray]
+}
+
+3*.class: note
+3,0: Step 1
+3,1: Step 2
+3,2: Step 3
+3,3: Step 4
+3,4: ""
+
+4,0: "" {
+  grid-rows: 3
+  grid-columns: 1
+  grid-gap: 0
+
+  class: []
+
+  style: {
+    fill: transparent
+    stroke: transparent
+  }
+
+  *.style: {
+    fill: transparent
+    stroke: transparent
+    font-color: "#30304c"
+    font-size: 10
+    bold: false
+  }
+  *.label.near: center-left
+  *.height: 20
+  a: ⬤ Forge CLI {
+    style.font-color: "#0033cc"
+  }
+
+  b: ⬤ Required {
+    style.font-color: "#30304c"
+  }
+  c: ⬤ Optional {
+    style.font-color: "#cecece"
+  }
+}
+4,1.class: empty
+4,2.class: empty
+4,3.class: empty
+4,4: forge\ndeploy
+
+0,0 -> 2,0 -> 2,1 -> 2,2 -> 2,3 -> 2,4: {
+  class: arrow
+}
+2,1 -> 0,1: {
+  class: arrow
+  style.stroke: "#cecece"
+}
+2,3 -> 0,3: {
+  class: arrow
+  style.stroke: "#cecece"
+}
+2,4 -> 0,4: Yes {
+  class: arrow
+  style.font-size: 10
+}
+2,4 -> 4,4: No {
+  class: arrow
+  style.font-size: 10
+}
+
+classes: {
+  text.style: {
+    stroke: transparent
+    font-color: white
+    font: mono
+    font-size: 10
+    bold: false
+  }
+  text: {
+    width: 100
+    height: 60
+  }
+  blue.style: {
+    fill: "#0033cc"
+    stroke: "#0033cc"
+    border-radius: 10
+  }
+  gray.style: {
+    fill: "#cecece"
+    stroke: "#cecece"
+    border-radius: 10
+  }
+  note: {
+    height: 30
+    label.near: top-center
+    style: {
+      font-size: 10
+      bold: false
+      fill: transparent
+      stroke: transparent
+    }
+  }
+  empty: {
+    label: ""
+    width: 50
+    height: 50
+    style: {
+      fill: transparent
+      stroke: transparent
+    }
+  }
+  arrow: {
+    target-arrowhead.shape: arrow
+    style: {
+      stroke: black
+      stroke-width: 2
+    }
+  }
+}

testdata/d2compiler/TestCompile/grid_deeper_edge.exp.json

@@ -0,0 +1,15 @@
+{
+  "graph": null,
+  "err": {
+    "errs": [
+      {
+        "range": "d2/testdata/d2compiler/TestCompile/grid_deeper_edge.d2,8:2:86-8:8:92",
+        "errmsg": "d2/testdata/d2compiler/TestCompile/grid_deeper_edge.d2:9:3: edge must be on direct child of grid diagram \"hey\""
+      },
+      {
+        "range": "d2/testdata/d2compiler/TestCompile/grid_deeper_edge.d2,4:2:41-4:8:47",
+        "errmsg": "d2/testdata/d2compiler/TestCompile/grid_deeper_edge.d2:5:3: grid diagrams can only have edges between children right now"
+      }
+    ]
+  }
+}

testdata/d2compiler/TestCompile/grid_edge.exp.json

@@ -3,16 +3,16 @@
   "err": {
     "errs": [
       {
-        "range": "d2/testdata/d2compiler/TestCompile/grid_edge.d2,2:1:22-2:7:28",
+        "range": "d2/testdata/d2compiler/TestCompile/grid_edge.d2,4:1:36-4:11:46",
-        "errmsg": "d2/testdata/d2compiler/TestCompile/grid_edge.d2:3:2: edges in grid diagrams are not supported yet"
+        "errmsg": "d2/testdata/d2compiler/TestCompile/grid_edge.d2:5:2: edges into grid diagrams are not supported yet"
       },
       {
-        "range": "d2/testdata/d2compiler/TestCompile/grid_edge.d2,4:1:32-4:11:42",
+        "range": "d2/testdata/d2compiler/TestCompile/grid_edge.d2,5:1:48-5:11:58",
-        "errmsg": "d2/testdata/d2compiler/TestCompile/grid_edge.d2:5:2: edges in grid diagrams are not supported yet"
+        "errmsg": "d2/testdata/d2compiler/TestCompile/grid_edge.d2:6:2: edges into grid diagrams are not supported yet"
       },
       {
-        "range": "d2/testdata/d2compiler/TestCompile/grid_edge.d2,5:1:44-5:11:54",
+        "range": "d2/testdata/d2compiler/TestCompile/grid_edge.d2,6:1:60-6:13:72",
-        "errmsg": "d2/testdata/d2compiler/TestCompile/grid_edge.d2:6:2: edges in grid diagrams are not supported yet"
+        "errmsg": "d2/testdata/d2compiler/TestCompile/grid_edge.d2:7:2: edges into grid diagrams are not supported yet"
       }
     ]
   }