diff --git a/d2layouts/d2sequence/layout_test.go b/d2layouts/d2sequence/layout_test.go index ac79e510b4..415d7827f0 100644 --- a/d2layouts/d2sequence/layout_test.go +++ b/d2layouts/d2sequence/layout_test.go @@ -471,3 +471,126 @@ func TestSequenceToDescendant(t *testing.T) { t.Fatal("expected route to end at `a` lifeline") } } + +func TestGroupStartsWithNoteOverlap(t *testing.T) { + input := ` +shape: sequence_diagram + +job + +group1: { + job.t1 -> job.t2: one + job.t1 -> job.t3: two +} +group2: { + job.t1."overlap with group1" +} +` + ctx := log.With(context.Background(), testlog.New(t)) + g, _, err := d2compiler.Compile("", strings.NewReader(input), nil) + assert.Nil(t, err) + + for _, obj := range g.Objects { + if obj.Box == nil { + obj.Box = geo.NewBox(nil, 100, 100) + } + } + + err = d2sequence.Layout(ctx, g, func(ctx context.Context, g *d2graph.Graph) error { + return nil + }) + assert.Nil(t, err) + + group1, has := g.Root.HasChild([]string{"group1"}) + assert.True(t, has) + group2, has := g.Root.HasChild([]string{"group2"}) + assert.True(t, has) + + assert.GreaterOrEqual(t, group2.TopLeft.Y, group1.TopLeft.Y+group1.Height, + "group2 (top %v) must not overlap group1 (bottom %v)", group2.TopLeft.Y, group1.TopLeft.Y+group1.Height) +} + +func TestGroupStartsWithNoteFollowedByMessages(t *testing.T) { + input := ` +shape: sequence_diagram + +a +b + +group1: { + a."step one note" + a -> b: step one +} +group2: { + b."step two note" + b -> a: step two +} +` + ctx := log.With(context.Background(), testlog.New(t)) + g, _, err := d2compiler.Compile("", strings.NewReader(input), nil) + assert.Nil(t, err) + + for _, obj := range g.Objects { + if obj.Box == nil { + obj.Box = geo.NewBox(nil, 100, 100) + } + } + + err = d2sequence.Layout(ctx, g, func(ctx context.Context, g *d2graph.Graph) error { + return nil + }) + assert.Nil(t, err) + + group1, has := g.Root.HasChild([]string{"group1"}) + assert.True(t, has) + group2, has := g.Root.HasChild([]string{"group2"}) + assert.True(t, has) + + assert.GreaterOrEqual(t, group2.TopLeft.Y, group1.TopLeft.Y+group1.Height, + "group2 (top %v) must not overlap group1 (bottom %v)", group2.TopLeft.Y, group1.TopLeft.Y+group1.Height) +} + +func TestMultipleGroupsStartingWithNotes(t *testing.T) { + input := ` +shape: sequence_diagram + +srv + +g1: { + srv."init config" +} +g2: { + srv."start worker" +} +g3: { + srv."listen on port" +} +` + ctx := log.With(context.Background(), testlog.New(t)) + g, _, err := d2compiler.Compile("", strings.NewReader(input), nil) + assert.Nil(t, err) + + for _, obj := range g.Objects { + if obj.Box == nil { + obj.Box = geo.NewBox(nil, 100, 100) + } + } + + err = d2sequence.Layout(ctx, g, func(ctx context.Context, g *d2graph.Graph) error { + return nil + }) + assert.Nil(t, err) + + g1, has := g.Root.HasChild([]string{"g1"}) + assert.True(t, has) + g2, has := g.Root.HasChild([]string{"g2"}) + assert.True(t, has) + g3, has := g.Root.HasChild([]string{"g3"}) + assert.True(t, has) + + assert.GreaterOrEqual(t, g2.TopLeft.Y, g1.TopLeft.Y+g1.Height, + "g2 (top %v) must not overlap g1 (bottom %v)", g2.TopLeft.Y, g1.TopLeft.Y+g1.Height) + assert.GreaterOrEqual(t, g3.TopLeft.Y, g2.TopLeft.Y+g2.Height, + "g3 (top %v) must not overlap g2 (bottom %v)", g3.TopLeft.Y, g2.TopLeft.Y+g2.Height) +} + diff --git a/d2layouts/d2sequence/sequence_diagram.go b/d2layouts/d2sequence/sequence_diagram.go index 58711f9478..d217acc3c5 100644 --- a/d2layouts/d2sequence/sequence_diagram.go +++ b/d2layouts/d2sequence/sequence_diagram.go @@ -478,6 +478,27 @@ func IsLifelineEnd(obj *d2graph.Object) bool { return obj.ID == LifelineEndID(obj.ID[:markerIndex]) } +func isSelfOrSibling(message *d2graph.Edge) bool { + if message.Src == message.Dst { + return true + } + if strings.HasPrefix(message.Dst.AbsID(), message.Src.AbsID()+".") { + return true + } + if strings.HasPrefix(message.Src.AbsID(), message.Dst.AbsID()+".") { + return true + } + currSrc := message.Src + for !currSrc.Parent.IsSequenceDiagram() { + currSrc = currSrc.Parent + } + currDst := message.Dst + for !currDst.Parent.IsSequenceDiagram() { + currDst = currDst.Parent + } + return currSrc == currDst +} + func (sd *sequenceDiagram) placeNotes() { rankToX := make(map[int]float64) for _, actor := range sd.actors { @@ -490,8 +511,8 @@ func (sd *sequenceDiagram) placeNotes() { for _, msg := range sd.messages { if sd.verticalIndices[msg.AbsID()] < verticalIndex { - if msg.Src == msg.Dst { - // For self-messages, account for the full vertical space they occupy + if isSelfOrSibling(msg) { + // For self/sibling messages, account for the full vertical space they occupy y += sd.yStep + math.Max(float64(msg.LabelDimensions.Height), MIN_MESSAGE_DISTANCE)*1.5 } else { y += sd.yStep + float64(msg.LabelDimensions.Height) @@ -609,21 +630,7 @@ func (sd *sequenceDiagram) routeMessages() error { } else { return fmt.Errorf("could not find center of %s. Is it declared as an actor?", message.Dst.ID) } - isToDescendant := strings.HasPrefix(message.Dst.AbsID(), message.Src.AbsID()+".") - isFromDescendant := strings.HasPrefix(message.Src.AbsID(), message.Dst.AbsID()+".") - isSelfMessage := message.Src == message.Dst - - currSrc := message.Src - for !currSrc.Parent.IsSequenceDiagram() { - currSrc = currSrc.Parent - } - currDst := message.Dst - for !currDst.Parent.IsSequenceDiagram() { - currDst = currDst.Parent - } - isToSibling := currSrc == currDst - - if isSelfMessage || isToDescendant || isFromDescendant || isToSibling { + if isSelfOrSibling(message) { midX := startX + math.Max(SELF_MESSAGE_HORIZONTAL_TRAVEL, float64(message.LabelDimensions.Width)/2.+label.PADDING*2) startY := messageOffset + noteOffset endY := startY + math.Max(float64(message.LabelDimensions.Height), MIN_MESSAGE_DISTANCE)*1.5