diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml index 2a8d9fa..6477184 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.yml +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -63,7 +63,7 @@ body: id: gpdf-version attributes: label: gpdf Version - placeholder: "v1.0.8" + placeholder: "v1.0.9" validations: required: true diff --git a/CHANGELOG.md b/CHANGELOG.md index fcd3ab9..217ccb1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/). ## [Unreleased] +## [1.0.9] - 2026-05-06 + +### Fixed +- AutoRow/Row no longer splits across page boundaries — a row that does not fit in the remaining space on the current page now moves as a whole to the next page, instead of placing the columns that fit on the current page and re-rendering the full row on the next (#24) + - `template/grid.go`: `RowBuilder.build()` now sets `BreakInside=BreakAvoid` on the horizontal Box so rows are treated as atomic layout units + - `document/layout/block.go`: `layoutVerticalChild` falls back to a normal split when a `BreakAvoid` child is the first node on a fresh page, preventing an infinite loop of empty pages when a row is taller than a full page + ## [1.0.8] - 2026-05-06 ### Added @@ -159,7 +166,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/). - Reed-Solomon coefficient order in QR code encoder - binary.Write return value handling for errcheck lint -[Unreleased]: https://github.com/gpdf-dev/gpdf/compare/v1.0.8...HEAD +[Unreleased]: https://github.com/gpdf-dev/gpdf/compare/v1.0.9...HEAD +[1.0.9]: https://github.com/gpdf-dev/gpdf/compare/v1.0.8...v1.0.9 [1.0.8]: https://github.com/gpdf-dev/gpdf/compare/v1.0.7...v1.0.8 [1.0.7]: https://github.com/gpdf-dev/gpdf/compare/v1.0.6...v1.0.7 [1.0.6]: https://github.com/gpdf-dev/gpdf/compare/v1.0.5...v1.0.6 diff --git a/README.md b/README.md index 9e034ef..383e324 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ [![Go Reference](https://pkg.go.dev/badge/github.com/gpdf-dev/gpdf.svg)](https://pkg.go.dev/github.com/gpdf-dev/gpdf) [![CI](https://github.com/gpdf-dev/gpdf/actions/workflows/check-code.yml/badge.svg)](https://github.com/gpdf-dev/gpdf/actions/workflows/check-code.yml) -![coverage](https://img.shields.io/badge/coverage-84.1%25-green) +![coverage](https://img.shields.io/badge/coverage-84.3%25-green) [![Go Report Card](https://goreportcard.com/badge/github.com/gpdf-dev/gpdf)](https://goreportcard.com/report/github.com/gpdf-dev/gpdf) [![Go Version](https://img.shields.io/badge/Go-%3E%3D1.22-blue)](https://go.dev/) [![Website](https://img.shields.io/badge/Website-gpdf.dev-blue)](https://gpdf.dev/) diff --git a/_examples/builder/37_row_break_avoid_test.go b/_examples/builder/37_row_break_avoid_test.go new file mode 100644 index 0000000..cb54b41 --- /dev/null +++ b/_examples/builder/37_row_break_avoid_test.go @@ -0,0 +1,90 @@ +package builder_test + +import ( + "bytes" + "image/color" + "testing" + + "github.com/gpdf-dev/gpdf/_examples/testutil" + "github.com/gpdf-dev/gpdf/document" + "github.com/gpdf-dev/gpdf/pdf" + "github.com/gpdf-dev/gpdf/template" +) + +// TestExample_37_RowBreakAvoid is a regression test for issue #24: +// when an AutoRow's columns partially fit at the bottom of a page — +// some columns fit, others overflow — the entire row must move to the +// next page instead of being split between its columns. +// +// In the buggy behavior, the text column's content rendered at the +// bottom of page 1 (overlapping the footer) and the complete row +// rendered again at the top of page 2. +func TestExample_37_RowBreakAvoid(t *testing.T) { + // Tall image (1:4 aspect) so the image column is much taller than the + // text column and reliably overflows the remaining space at the bottom + // of the first page. + imgData := testutil.TestImagePNG(t, 50, 200, color.RGBA{R: 30, G: 136, B: 229, A: 255}) + + doc := template.New( + template.WithPageSize(document.A4), + template.WithMargins(document.Edges{ + Top: document.Mm(13), + Right: document.Mm(13), + Bottom: document.Mm(19), + Left: document.Mm(13), + }), + template.WithDefaultFont("Helvetica", 9), + ) + + doc.Footer(func(p *template.PageBuilder) { + p.AutoRow(func(r *template.RowBuilder) { + r.Col(12, func(c *template.ColBuilder) { + c.PageNumber(template.AlignRight(), template.FontSize(8), + template.TextColor(pdf.Gray(0.5))) + }) + }) + }) + + page := doc.AddPage() + + // Fill the page with fixed-height rows to deterministically push the last + // row near the bottom. Each filler is 13mm tall × 20 rows = 260mm, + // leaving roughly 5mm at the bottom of the body area — enough for the + // text column of the final row but not for the (taller) image column. + for range 20 { + page.Row(document.Mm(13), func(r *template.RowBuilder) { + r.Col(12, func(c *template.ColBuilder) { + c.Text("Filler row — some content here") + }) + }) + } + + // This row is tall enough that it partially overflows the remaining + // space on page 1: col 1 text fits, col 2 image does not. + page.AutoRow(func(r *template.RowBuilder) { + r.Col(9, func(c *template.ColBuilder) { + c.Text("GROUP HEADER", template.Bold(), template.FontSize(12)) + c.Text("Patient name", template.FontSize(10)) + c.Spacer(document.Mm(2)) + }) + r.Col(3, func(c *template.ColBuilder) { + c.Image(imgData, template.FitWidth(document.Mm(20))) + }) + }) + + testutil.GeneratePDFSharedGolden(t, "37_row_break_avoid.pdf", doc) + + // Behavioural assertion: the row must move as a whole to the next page, + // so each text in the final row appears exactly once in the document. + // The pre-fix behaviour would render col 1 on page 1 (overlapping the + // footer) AND the entire row on page 2 — duplicating both strings. + data, err := doc.Generate() + if err != nil { + t.Fatalf("Generate failed: %v", err) + } + for _, marker := range []string{"GROUP HEADER", "Patient name"} { + if got := bytes.Count(data, []byte(marker)); got != 1 { + t.Errorf("expected %q to appear once (row moved as a whole), got %d occurrences", marker, got) + } + } +} diff --git a/_examples/gotemplate/37_row_break_avoid_test.go b/_examples/gotemplate/37_row_break_avoid_test.go new file mode 100644 index 0000000..22b33c1 --- /dev/null +++ b/_examples/gotemplate/37_row_break_avoid_test.go @@ -0,0 +1,88 @@ +package gotemplate_test + +import ( + "bytes" + "encoding/base64" + "fmt" + "image/color" + "strings" + "testing" + + "github.com/gpdf-dev/gpdf/_examples/testutil" + "github.com/gpdf-dev/gpdf/document" + "github.com/gpdf-dev/gpdf/template" +) + +// TestTmpl_37_RowBreakAvoid mirrors the builder regression test for +// issue #24 — a row whose columns partially fit at the bottom of a +// page must move as a whole to the next page rather than splitting +// between its columns. +func TestTmpl_37_RowBreakAvoid(t *testing.T) { + imgB64 := base64.StdEncoding.EncodeToString( + testutil.TestImagePNG(t, 50, 200, color.RGBA{R: 30, G: 136, B: 229, A: 255}), + ) + + var fillerRows strings.Builder + for i := 0; i < 20; i++ { + fillerRows.WriteString(`, + {"row": {"height": "13mm", "cols": [ + {"span": 12, "text": "{{.Filler}}"} + ]}}`) + } + + schema := []byte(fmt.Sprintf(`{ + "page": {"size": "A4"}, + "footer": [ + {"row": {"cols": [ + {"span": 12, "elements": [ + {"type": "pageNumber", "style": {"align": "right", "size": 8, "color": "gray(0.5)"}} + ]} + ]}} + ], + "body": [ + {"row": {"cols": [ + {"span": 12, "text": ""} + ]}}%s, + {"row": {"cols": [ + {"span": 9, "elements": [ + {"type": "text", "content": "{{.GroupHeader}}", "style": {"bold": true, "size": 12}}, + {"type": "text", "content": "{{.PatientName}}", "style": {"size": 10}}, + {"type": "spacer", "height": "2mm"} + ]}, + {"span": 3, "elements": [ + {"type": "image", "image": {"src": "%s", "width": "20mm"}} + ]} + ]}} + ] + }`, fillerRows.String(), imgB64)) + + data := map[string]any{ + "Filler": "Filler row — some content here", + "GroupHeader": "GROUP HEADER", + "PatientName": "Patient name", + } + + doc, err := template.FromJSON(schema, data, + template.WithMargins(document.Edges{ + Top: document.Mm(13), + Right: document.Mm(13), + Bottom: document.Mm(19), + Left: document.Mm(13), + }), + template.WithDefaultFont("Helvetica", 9), + ) + if err != nil { + t.Fatalf("FromJSON error: %v", err) + } + testutil.GeneratePDFSharedGolden(t, "37_row_break_avoid.pdf", doc) + + out, err := doc.Generate() + if err != nil { + t.Fatalf("Generate failed: %v", err) + } + for _, marker := range []string{"GROUP HEADER", "Patient name"} { + if got := bytes.Count(out, []byte(marker)); got != 1 { + t.Errorf("expected %q to appear once (row moved as a whole), got %d occurrences", marker, got) + } + } +} diff --git a/_examples/json/37_row_break_avoid_test.go b/_examples/json/37_row_break_avoid_test.go new file mode 100644 index 0000000..ef590ae --- /dev/null +++ b/_examples/json/37_row_break_avoid_test.go @@ -0,0 +1,82 @@ +package json_test + +import ( + "bytes" + "encoding/base64" + "fmt" + "image/color" + "strings" + "testing" + + "github.com/gpdf-dev/gpdf/_examples/testutil" + "github.com/gpdf-dev/gpdf/document" + "github.com/gpdf-dev/gpdf/template" +) + +// TestJSON_37_RowBreakAvoid mirrors the builder regression test for +// issue #24 — a row whose columns partially fit at the bottom of a page +// must move as a whole to the next page rather than splitting between +// its columns. +func TestJSON_37_RowBreakAvoid(t *testing.T) { + imgB64 := base64.StdEncoding.EncodeToString( + testutil.TestImagePNG(t, 50, 200, color.RGBA{R: 30, G: 136, B: 229, A: 255}), + ) + + var fillerRows strings.Builder + for i := 0; i < 20; i++ { + fillerRows.WriteString(`, + {"row": {"height": "13mm", "cols": [ + {"span": 12, "text": "Filler row — some content here"} + ]}}`) + } + + schema := []byte(fmt.Sprintf(`{ + "page": {"size": "A4"}, + "footer": [ + {"row": {"cols": [ + {"span": 12, "elements": [ + {"type": "pageNumber", "style": {"align": "right", "size": 8, "color": "gray(0.5)"}} + ]} + ]}} + ], + "body": [ + {"row": {"cols": [ + {"span": 12, "text": ""} + ]}}%s, + {"row": {"cols": [ + {"span": 9, "elements": [ + {"type": "text", "content": "GROUP HEADER", "style": {"bold": true, "size": 12}}, + {"type": "text", "content": "Patient name", "style": {"size": 10}}, + {"type": "spacer", "height": "2mm"} + ]}, + {"span": 3, "elements": [ + {"type": "image", "image": {"src": "%s", "width": "20mm"}} + ]} + ]}} + ] + }`, fillerRows.String(), imgB64)) + + doc, err := template.FromJSON(schema, nil, + template.WithMargins(document.Edges{ + Top: document.Mm(13), + Right: document.Mm(13), + Bottom: document.Mm(19), + Left: document.Mm(13), + }), + template.WithDefaultFont("Helvetica", 9), + ) + if err != nil { + t.Fatalf("FromJSON error: %v", err) + } + testutil.GeneratePDFSharedGolden(t, "37_row_break_avoid.pdf", doc) + + data, err := doc.Generate() + if err != nil { + t.Fatalf("Generate failed: %v", err) + } + for _, marker := range []string{"GROUP HEADER", "Patient name"} { + if got := bytes.Count(data, []byte(marker)); got != 1 { + t.Errorf("expected %q to appear once (row moved as a whole), got %d occurrences", marker, got) + } + } +} diff --git a/_examples/testdata/golden/37_row_break_avoid.pdf b/_examples/testdata/golden/37_row_break_avoid.pdf new file mode 100644 index 0000000..d4f98e3 Binary files /dev/null and b/_examples/testdata/golden/37_row_break_avoid.pdf differ diff --git a/document/layout/block.go b/document/layout/block.go index e5d32cb..dd619f4 100644 --- a/document/layout/block.go +++ b/document/layout/block.go @@ -181,8 +181,12 @@ func (bl *BlockLayout) layoutVerticalChild(bc *blockContext, child document.Docu childResult := bl.layoutChild(child, childConstraints) // BreakInside=BreakAvoid: if the child overflowed, move the - // entire child to overflow instead of splitting it. - if bp.BreakInside == document.BreakAvoid && childResult.Overflow != nil { + // entire child to overflow instead of splitting it. Skip this + // guard when the child is the first node on a fresh page (nothing + // placed yet) — the child cannot be made to fit by deferring it, + // so fall back to the default split to avoid an infinite loop of + // empty pages. + if bp.BreakInside == document.BreakAvoid && childResult.Overflow != nil && (i > 0 || cursorY > 0 || len(placed) > 0) { return bc.overflowResult(placed, cursorY, children[i:]), true } diff --git a/document/layout/layout_test.go b/document/layout/layout_test.go index 7f22c95..2b502ef 100644 --- a/document/layout/layout_test.go +++ b/document/layout/layout_test.go @@ -2673,6 +2673,86 @@ func TestBreakInsideAvoidFits(t *testing.T) { } } +func TestBreakInsideAvoidFirstChildFallback(t *testing.T) { + // Regression for issue #24: when a BreakAvoid child is the first node on + // a fresh page and is taller than the available space, it must fall back + // to a normal split — otherwise the layout engine loops emitting empty + // pages forever. The child should be placed (partial fit) and the + // remainder returned as overflow. + bl := NewBlockLayout() + tooTall := &document.Box{ + BoxStyle: document.BoxStyle{Height: document.Pt(50)}, + BreakPolicy: document.BreakPolicy{BreakInside: document.BreakAvoid}, + Content: []document.DocumentNode{ + &document.Text{Content: strings.Repeat("x ", 200), TextStyle: document.DefaultStyle()}, + }, + } + parent := &document.Box{Content: []document.DocumentNode{tooTall}} + constraints := Constraints{ + AvailableWidth: 100, + AvailableHeight: 30, + FontResolver: &mockFontResolver{}, + } + result := bl.Layout(parent, constraints) + if result.Overflow == nil { + t.Fatal("Expected overflow when BreakAvoid child is taller than the page") + } + if len(result.Children) != 1 { + t.Errorf("Expected the BreakAvoid child to be placed (split fallback), got %d children", len(result.Children)) + } +} + +func TestRowBreakAvoidKeepsRowTogether(t *testing.T) { + // Regression for issue #24: a horizontal row with BreakAvoid whose + // columns partially fit must move the entire row to the next page, + // instead of placing the columns that fit on the current page. + bl := NewBlockLayout() + + // First child fills most of the page. + filler := &document.Box{ + BoxStyle: document.BoxStyle{Height: document.Pt(80)}, + Content: []document.DocumentNode{ + &document.Text{Content: "filler", TextStyle: document.DefaultStyle()}, + }, + } + + // Row whose right column is too tall to fit in the remaining space. + // Without BreakAvoid the row would split; with it, the whole row moves + // to the next page. + row := &document.Box{ + BoxStyle: document.BoxStyle{Direction: document.DirectionHorizontal}, + BreakPolicy: document.BreakPolicy{BreakInside: document.BreakAvoid}, + Content: []document.DocumentNode{ + &document.Box{ + BoxStyle: document.BoxStyle{Width: document.Pct(50)}, + Content: []document.DocumentNode{ + &document.Text{Content: "short", TextStyle: document.DefaultStyle()}, + }, + }, + &document.Box{ + BoxStyle: document.BoxStyle{Width: document.Pct(50)}, + Content: []document.DocumentNode{ + &document.Text{Content: strings.Repeat("y ", 200), TextStyle: document.DefaultStyle()}, + }, + }, + }, + } + + parent := &document.Box{Content: []document.DocumentNode{filler, row}} + constraints := Constraints{ + AvailableWidth: 200, + AvailableHeight: 100, + FontResolver: &mockFontResolver{}, + } + result := bl.Layout(parent, constraints) + if result.Overflow == nil { + t.Fatal("Expected overflow when row does not fit") + } + if len(result.Children) != 1 { + t.Errorf("Expected only the filler to be placed; got %d children (row should not be split)", len(result.Children)) + } +} + func TestBreakAutoDefault(t *testing.T) { // Default BreakPolicy (all BreakAuto) should behave normally. bl := NewBlockLayout() diff --git a/internal/buildinfo/version.go b/internal/buildinfo/version.go index 20b9107..bda1b21 100644 --- a/internal/buildinfo/version.go +++ b/internal/buildinfo/version.go @@ -3,4 +3,4 @@ package buildinfo // Version is the library version. It is the single source of truth used by // the public gpdf.Version constant and the default PDF Producer metadata. -const Version = "1.0.8" +const Version = "1.0.9" diff --git a/template/grid.go b/template/grid.go index 0c14f61..fa4ad22 100644 --- a/template/grid.go +++ b/template/grid.go @@ -130,11 +130,19 @@ func (r *RowBuilder) Col(span int, fn func(c *ColBuilder)) { // build converts the row into a Box document node. Each column becomes // a child Box with a width proportional to its grid span. +// +// Rows are treated as atomic units: BreakInside defaults to BreakAvoid so +// that a row which does not fit in the remaining vertical space on the +// current page is moved as a whole to the next page, rather than splitting +// between its columns. func (r *RowBuilder) build(height document.Value, auto bool) document.DocumentNode { box := &document.Box{ BoxStyle: document.BoxStyle{ Direction: document.DirectionHorizontal, }, + BreakPolicy: document.BreakPolicy{ + BreakInside: document.BreakAvoid, + }, } if !auto { diff --git a/template/template_test.go b/template/template_test.go index 8aff3c6..beb2508 100644 --- a/template/template_test.go +++ b/template/template_test.go @@ -1992,3 +1992,35 @@ func TestTotalPages(t *testing.T) { t.Fatal("generated PDF is empty") } } + +// TestRowBuildSetsBreakInsideAvoid verifies that rows produced by +// RowBuilder.build() default to BreakInside=BreakAvoid so they are +// treated as atomic units by the layout engine. Regression test for +// issue #24 where a partially fitting AutoRow split between its +// columns and produced duplicate content across page boundaries. +func TestRowBuildSetsBreakInsideAvoid(t *testing.T) { + rb := &RowBuilder{} + rb.Col(12, func(c *ColBuilder) { c.Text("hello") }) + + for _, tc := range []struct { + name string + auto bool + }{ + {"AutoRow", true}, + {"FixedRow", false}, + } { + t.Run(tc.name, func(t *testing.T) { + node := rb.build(document.Mm(10), tc.auto) + box, ok := node.(*document.Box) + if !ok { + t.Fatalf("expected *document.Box, got %T", node) + } + if box.BoxStyle.Direction != document.DirectionHorizontal { + t.Errorf("expected DirectionHorizontal, got %v", box.BoxStyle.Direction) + } + if box.BreakPolicy.BreakInside != document.BreakAvoid { + t.Errorf("expected BreakInside=BreakAvoid, got %v", box.BreakPolicy.BreakInside) + } + }) + } +}