From 548af76b5e651f59466e9012b22396a74770a686 Mon Sep 17 00:00:00 2001 From: Gavin Nishizawa Date: Wed, 20 Sep 2023 11:40:48 -0700 Subject: [PATCH 1/5] add empty_nested_grid test --- e2etests/regression_test.go | 1 + e2etests/testdata/files/empty_nested_grid.d2 | 7 +++++++ 2 files changed, 8 insertions(+) create mode 100644 e2etests/testdata/files/empty_nested_grid.d2 diff --git a/e2etests/regression_test.go b/e2etests/regression_test.go index 9a1c29a9f..e098212ba 100644 --- a/e2etests/regression_test.go +++ b/e2etests/regression_test.go @@ -1040,6 +1040,7 @@ cf many required: { loadFromFile(t, "outside_grid_label_position"), loadFromFile(t, "arrowhead_font_color"), loadFromFile(t, "multiple_constant_nears"), + loadFromFile(t, "empty_nested_grid"), } runa(t, tcs) diff --git a/e2etests/testdata/files/empty_nested_grid.d2 b/e2etests/testdata/files/empty_nested_grid.d2 new file mode 100644 index 000000000..7df0a9ed3 --- /dev/null +++ b/e2etests/testdata/files/empty_nested_grid.d2 @@ -0,0 +1,7 @@ +outer: { + grid-rows: 1 + inner: { + grid-rows: 1 + # empty + } +} From 4ce32e7d05b70f1ea230e25d09fd32fa401e099d Mon Sep 17 00:00:00 2001 From: Gavin Nishizawa Date: Wed, 20 Sep 2023 12:08:02 -0700 Subject: [PATCH 2/5] fix empty grid --- d2layouts/d2grid/layout.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/d2layouts/d2grid/layout.go b/d2layouts/d2grid/layout.go index 5e8d0125f..ae917d28e 100644 --- a/d2layouts/d2grid/layout.go +++ b/d2layouts/d2grid/layout.go @@ -64,11 +64,14 @@ func withoutGridDiagrams(ctx context.Context, g *d2graph.Graph, layout d2graph.L var processGrid func(obj *d2graph.Object) error processGrid = func(obj *d2graph.Object) error { for _, child := range obj.ChildrenArray { + if len(child.ChildrenArray) == 0 { + continue + } if child.IsGridDiagram() { if err := processGrid(child); err != nil { return err } - } else if len(child.ChildrenArray) > 0 { + } else { tempGraph := g.ExtractAsNestedGraph(child) if err := layout(ctx, tempGraph); err != nil { return err From d9f9fe5415f0bc4bcf3a1afdc8f9365c5013d678 Mon Sep 17 00:00:00 2001 From: Gavin Nishizawa Date: Wed, 20 Sep 2023 11:49:50 -0700 Subject: [PATCH 3/5] test success --- .../empty_nested_grid/dagre/board.exp.json | 130 ++++++++++++++++++ .../empty_nested_grid/dagre/sketch.exp.svg | 103 ++++++++++++++ .../empty_nested_grid/elk/board.exp.json | 130 ++++++++++++++++++ .../empty_nested_grid/elk/sketch.exp.svg | 103 ++++++++++++++ 4 files changed, 466 insertions(+) create mode 100644 e2etests/testdata/regression/empty_nested_grid/dagre/board.exp.json create mode 100644 e2etests/testdata/regression/empty_nested_grid/dagre/sketch.exp.svg create mode 100644 e2etests/testdata/regression/empty_nested_grid/elk/board.exp.json create mode 100644 e2etests/testdata/regression/empty_nested_grid/elk/sketch.exp.svg diff --git a/e2etests/testdata/regression/empty_nested_grid/dagre/board.exp.json b/e2etests/testdata/regression/empty_nested_grid/dagre/board.exp.json new file mode 100644 index 000000000..3a0c7b711 --- /dev/null +++ b/e2etests/testdata/regression/empty_nested_grid/dagre/board.exp.json @@ -0,0 +1,130 @@ +{ + "name": "", + "isFolderOnly": false, + "fontFamily": "SourceSansPro", + "shapes": [ + { + "id": "outer", + "type": "rectangle", + "pos": { + "x": 0, + "y": 0 + }, + "width": 221, + "height": 196, + "opacity": 1, + "strokeDash": 0, + "strokeWidth": 2, + "borderRadius": 0, + "fill": "B4", + "stroke": "B1", + "shadow": false, + "3d": false, + "multiple": false, + "double-border": false, + "tooltip": "", + "link": "", + "icon": null, + "iconPosition": "", + "blend": false, + "fields": null, + "methods": null, + "columns": null, + "label": "outer", + "fontSize": 28, + "fontFamily": "DEFAULT", + "language": "", + "color": "N1", + "italic": false, + "bold": false, + "underline": false, + "labelWidth": 63, + "labelHeight": 36, + "labelPosition": "INSIDE_TOP_CENTER", + "zIndex": 0, + "level": 1 + }, + { + "id": "outer.inner", + "type": "rectangle", + "pos": { + "x": 60, + "y": 60 + }, + "width": 101, + "height": 76, + "opacity": 1, + "strokeDash": 0, + "strokeWidth": 2, + "borderRadius": 0, + "fill": "B5", + "stroke": "B1", + "shadow": false, + "3d": false, + "multiple": false, + "double-border": false, + "tooltip": "", + "link": "", + "icon": null, + "iconPosition": "", + "blend": false, + "fields": null, + "methods": null, + "columns": null, + "label": "inner", + "fontSize": 24, + "fontFamily": "DEFAULT", + "language": "", + "color": "N1", + "italic": false, + "bold": true, + "underline": false, + "labelWidth": 56, + "labelHeight": 31, + "labelPosition": "INSIDE_MIDDLE_CENTER", + "zIndex": 0, + "level": 2 + } + ], + "connections": [], + "root": { + "id": "", + "type": "", + "pos": { + "x": 0, + "y": 0 + }, + "width": 0, + "height": 0, + "opacity": 0, + "strokeDash": 0, + "strokeWidth": 0, + "borderRadius": 0, + "fill": "N7", + "stroke": "", + "shadow": false, + "3d": false, + "multiple": false, + "double-border": false, + "tooltip": "", + "link": "", + "icon": null, + "iconPosition": "", + "blend": false, + "fields": null, + "methods": null, + "columns": null, + "label": "", + "fontSize": 0, + "fontFamily": "", + "language": "", + "color": "", + "italic": false, + "bold": false, + "underline": false, + "labelWidth": 0, + "labelHeight": 0, + "zIndex": 0, + "level": 0 + } +} diff --git a/e2etests/testdata/regression/empty_nested_grid/dagre/sketch.exp.svg b/e2etests/testdata/regression/empty_nested_grid/dagre/sketch.exp.svg new file mode 100644 index 000000000..e83f62b6e --- /dev/null +++ b/e2etests/testdata/regression/empty_nested_grid/dagre/sketch.exp.svg @@ -0,0 +1,103 @@ +outerinner + + + + \ No newline at end of file diff --git a/e2etests/testdata/regression/empty_nested_grid/elk/board.exp.json b/e2etests/testdata/regression/empty_nested_grid/elk/board.exp.json new file mode 100644 index 000000000..0489c2b35 --- /dev/null +++ b/e2etests/testdata/regression/empty_nested_grid/elk/board.exp.json @@ -0,0 +1,130 @@ +{ + "name": "", + "isFolderOnly": false, + "fontFamily": "SourceSansPro", + "shapes": [ + { + "id": "outer", + "type": "rectangle", + "pos": { + "x": 12, + "y": 12 + }, + "width": 221, + "height": 196, + "opacity": 1, + "strokeDash": 0, + "strokeWidth": 2, + "borderRadius": 0, + "fill": "B4", + "stroke": "B1", + "shadow": false, + "3d": false, + "multiple": false, + "double-border": false, + "tooltip": "", + "link": "", + "icon": null, + "iconPosition": "", + "blend": false, + "fields": null, + "methods": null, + "columns": null, + "label": "outer", + "fontSize": 28, + "fontFamily": "DEFAULT", + "language": "", + "color": "N1", + "italic": false, + "bold": false, + "underline": false, + "labelWidth": 63, + "labelHeight": 36, + "labelPosition": "INSIDE_TOP_CENTER", + "zIndex": 0, + "level": 1 + }, + { + "id": "outer.inner", + "type": "rectangle", + "pos": { + "x": 72, + "y": 72 + }, + "width": 101, + "height": 76, + "opacity": 1, + "strokeDash": 0, + "strokeWidth": 2, + "borderRadius": 0, + "fill": "B5", + "stroke": "B1", + "shadow": false, + "3d": false, + "multiple": false, + "double-border": false, + "tooltip": "", + "link": "", + "icon": null, + "iconPosition": "", + "blend": false, + "fields": null, + "methods": null, + "columns": null, + "label": "inner", + "fontSize": 24, + "fontFamily": "DEFAULT", + "language": "", + "color": "N1", + "italic": false, + "bold": true, + "underline": false, + "labelWidth": 56, + "labelHeight": 31, + "labelPosition": "INSIDE_MIDDLE_CENTER", + "zIndex": 0, + "level": 2 + } + ], + "connections": [], + "root": { + "id": "", + "type": "", + "pos": { + "x": 0, + "y": 0 + }, + "width": 0, + "height": 0, + "opacity": 0, + "strokeDash": 0, + "strokeWidth": 0, + "borderRadius": 0, + "fill": "N7", + "stroke": "", + "shadow": false, + "3d": false, + "multiple": false, + "double-border": false, + "tooltip": "", + "link": "", + "icon": null, + "iconPosition": "", + "blend": false, + "fields": null, + "methods": null, + "columns": null, + "label": "", + "fontSize": 0, + "fontFamily": "", + "language": "", + "color": "", + "italic": false, + "bold": false, + "underline": false, + "labelWidth": 0, + "labelHeight": 0, + "zIndex": 0, + "level": 0 + } +} diff --git a/e2etests/testdata/regression/empty_nested_grid/elk/sketch.exp.svg b/e2etests/testdata/regression/empty_nested_grid/elk/sketch.exp.svg new file mode 100644 index 000000000..78802de34 --- /dev/null +++ b/e2etests/testdata/regression/empty_nested_grid/elk/sketch.exp.svg @@ -0,0 +1,103 @@ +outerinner + + + + \ No newline at end of file From e44ac23c84dd8e486a0483ecfef02f6b303a7364 Mon Sep 17 00:00:00 2001 From: Gavin Nishizawa Date: Wed, 20 Sep 2023 11:51:43 -0700 Subject: [PATCH 4/5] cleanup warning --- d2layouts/d2grid/layout.go | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/d2layouts/d2grid/layout.go b/d2layouts/d2grid/layout.go index ae917d28e..9e9109453 100644 --- a/d2layouts/d2grid/layout.go +++ b/d2layouts/d2grid/layout.go @@ -704,20 +704,14 @@ func (gd *gridDiagram) getBestLayout(targetSize float64, columns bool) [][]*d2gr // if multiple nodes are too big, it isn't ok. but a single node can't shrink so only check here if rowSize > okThreshold*targetSize { skipCount++ - if skipCount >= SKIP_LIMIT { - // there may even be too many to skip - return true - } - return false + // there may even be too many to skip + return skipCount >= SKIP_LIMIT } } // row is too small to be good overall if rowSize < targetSize/okThreshold { skipCount++ - if skipCount >= SKIP_LIMIT { - return true - } - return false + return skipCount >= SKIP_LIMIT } return true } From 185b9d4f7d399471386fd43a44c17e67c7f3afde Mon Sep 17 00:00:00 2001 From: Gavin Nishizawa Date: Wed, 20 Sep 2023 12:21:58 -0700 Subject: [PATCH 5/5] changelog --- ci/release/changelogs/next.md | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/release/changelogs/next.md b/ci/release/changelogs/next.md index 8b1b3c4f6..65594b6f9 100644 --- a/ci/release/changelogs/next.md +++ b/ci/release/changelogs/next.md @@ -15,3 +15,4 @@ - Fixes Markdown cropping last element in mixed-element blocks (e.g. em and strong) [#1543](https://github.com/terrastruct/d2/issues/1543) - Fixes missing compile error for non-blockstring empty labels [#1590](https://github.com/terrastruct/d2/issues/1590) - Fixes multiple constant nears overlapping in some cases [#1591](https://github.com/terrastruct/d2/issues/1591) +- Fixes error with an empty nested grid [#1594](https://github.com/terrastruct/d2/issues/1594)