diff --git a/CHANGELOG.md b/CHANGELOG.md index 7906b65..ae4cd15 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/). ## [Unreleased] +### Fixed +- A multi-page table preceded by other rows no longer moves entirely to the next page, leaving the first page mostly blank. Since #24 every row was `BreakInside=BreakAvoid`, so a row containing a table that did not fit in the remaining space was deferred as a whole; the table now starts on the current page, repeats its header on each page, and renders every body row exactly once + - `template/grid.go`: `RowBuilder.build()` keeps `BreakAvoid` for ordinary rows but uses `BreakAuto` for rows that contain a table (tables paginate themselves) + - Example tests: `_examples/{builder,json,gotemplate}/39_table_after_content_test.go` with shared golden + ## [1.0.13] - 2026-09-04 ### Fixed diff --git a/_examples/builder/39_table_after_content_test.go b/_examples/builder/39_table_after_content_test.go new file mode 100644 index 0000000..d702130 --- /dev/null +++ b/_examples/builder/39_table_after_content_test.go @@ -0,0 +1,51 @@ +package builder_test + +import ( + "fmt" + "testing" + + "github.com/gpdf-dev/gpdf/_examples/testutil" + "github.com/gpdf-dev/gpdf/document" + "github.com/gpdf-dev/gpdf/template" +) + +// TestExample_39_TableAfterContent is a regression test: a multi-page table +// preceded by other rows must start on the first page and paginate from +// there (header repeated on each page), instead of the whole table being +// pushed to page 2 and leaving page 1 mostly blank. Rows were made atomic +// (BreakAvoid) for issue #24; rows containing a table must stay splittable. +func TestExample_39_TableAfterContent(t *testing.T) { + doc := template.New( + template.WithPageSize(document.A4), + template.WithMargins(document.UniformEdges(document.Mm(15))), + ) + + rows := make([][]string, 60) + ids := make([]string, len(rows)) + for i := range rows { + ids[i] = fmt.Sprintf("Item-%03d", i+1) + rows[i] = []string{ids[i], fmt.Sprintf("%d", (i+1)*100)} + } + + page := doc.AddPage() + page.AutoRow(func(r *template.RowBuilder) { + r.Col(12, func(c *template.ColBuilder) { + c.Text("Table After Content", template.FontSize(18), template.Bold()) + c.Text("Introductory paragraph before the table.") + c.Spacer(document.Mm(5)) + }) + }) + page.AutoRow(func(r *template.RowBuilder) { + r.Col(12, func(c *template.ColBuilder) { + c.Table([]string{"Item", "Amount"}, rows) + }) + }) + + testutil.GeneratePDFSharedGolden(t, "39_table_after_content.pdf", doc) + + data, err := doc.Generate() + if err != nil { + t.Fatalf("Generate failed: %v", err) + } + testutil.AssertTableSplitsFromFirstPage(t, data, "Table After Content", "Item", ids) +} diff --git a/_examples/gotemplate/39_table_after_content_test.go b/_examples/gotemplate/39_table_after_content_test.go new file mode 100644 index 0000000..1e2d3a5 --- /dev/null +++ b/_examples/gotemplate/39_table_after_content_test.go @@ -0,0 +1,56 @@ +package gotemplate_test + +import ( + "fmt" + "testing" + + "github.com/gpdf-dev/gpdf/_examples/testutil" + "github.com/gpdf-dev/gpdf/template" +) + +// TestTmpl_39_TableAfterContent mirrors the builder regression test: a +// multi-page table preceded by other rows must start on the first page. +func TestTmpl_39_TableAfterContent(t *testing.T) { + schema := []byte(`{ + "page": {"size": "A4", "margins": "15mm"}, + "body": [ + {"row": {"cols": [ + {"span": 12, "elements": [ + {"type": "text", "content": "{{.Title}}", "style": {"size": 18, "bold": true}}, + {"type": "text", "content": "{{.Intro}}"}, + {"type": "spacer", "height": "5mm"} + ]} + ]}}, + {"row": {"cols": [ + {"span": 12, "table": { + "header": ["Item", "Amount"], + "rows": {{toJSON .Rows}} + }} + ]}} + ] + }`) + + ids := make([]string, 60) + rows := make([][]string, len(ids)) + for i := range ids { + ids[i] = fmt.Sprintf("Item-%03d", i+1) + rows[i] = []string{ids[i], fmt.Sprintf("%d", (i+1)*100)} + } + data := map[string]any{ + "Title": "Table After Content", + "Intro": "Introductory paragraph before the table.", + "Rows": rows, + } + + doc, err := template.FromJSON(schema, data) + if err != nil { + t.Fatalf("FromJSON error: %v", err) + } + testutil.GeneratePDFSharedGolden(t, "39_table_after_content.pdf", doc) + + out, err := doc.Generate() + if err != nil { + t.Fatalf("Generate failed: %v", err) + } + testutil.AssertTableSplitsFromFirstPage(t, out, "Table After Content", "Item", ids) +} diff --git a/_examples/json/39_table_after_content_test.go b/_examples/json/39_table_after_content_test.go new file mode 100644 index 0000000..04168f5 --- /dev/null +++ b/_examples/json/39_table_after_content_test.go @@ -0,0 +1,52 @@ +package json_test + +import ( + "fmt" + "strings" + "testing" + + "github.com/gpdf-dev/gpdf/_examples/testutil" + "github.com/gpdf-dev/gpdf/template" +) + +// TestJSON_39_TableAfterContent mirrors the builder regression test: a +// multi-page table preceded by other rows must start on the first page. +func TestJSON_39_TableAfterContent(t *testing.T) { + ids := make([]string, 60) + rowJSON := make([]string, len(ids)) + for i := range ids { + ids[i] = fmt.Sprintf("Item-%03d", i+1) + rowJSON[i] = fmt.Sprintf(`["%s", "%d"]`, ids[i], (i+1)*100) + } + + schema := []byte(fmt.Sprintf(`{ + "page": {"size": "A4", "margins": "15mm"}, + "body": [ + {"row": {"cols": [ + {"span": 12, "elements": [ + {"type": "text", "content": "Table After Content", "style": {"size": 18, "bold": true}}, + {"type": "text", "content": "Introductory paragraph before the table."}, + {"type": "spacer", "height": "5mm"} + ]} + ]}}, + {"row": {"cols": [ + {"span": 12, "table": { + "header": ["Item", "Amount"], + "rows": [%s] + }} + ]}} + ] + }`, strings.Join(rowJSON, ","))) + + doc, err := template.FromJSON(schema, nil) + if err != nil { + t.Fatalf("FromJSON error: %v", err) + } + testutil.GeneratePDFSharedGolden(t, "39_table_after_content.pdf", doc) + + data, err := doc.Generate() + if err != nil { + t.Fatalf("Generate failed: %v", err) + } + testutil.AssertTableSplitsFromFirstPage(t, data, "Table After Content", "Item", ids) +} diff --git a/_examples/testdata/golden/39_table_after_content.pdf b/_examples/testdata/golden/39_table_after_content.pdf new file mode 100644 index 0000000..2a4093a Binary files /dev/null and b/_examples/testdata/golden/39_table_after_content.pdf differ diff --git a/_examples/testutil/pages.go b/_examples/testutil/pages.go new file mode 100644 index 0000000..0e7df8b --- /dev/null +++ b/_examples/testutil/pages.go @@ -0,0 +1,66 @@ +package testutil + +import ( + "regexp" + "strings" + "testing" +) + +var ( + pageContentsRef = regexp.MustCompile(`<< /Contents (\d+) 0 R [^>]*>> >> /Type /Page >>`) + streamObject = regexp.MustCompile(`(?s)(?:^|\n)(\d+) 0 obj\n<< /Length \d+ >>\nstream\n(.*?)endstream`) +) + +// PageContents returns the uncompressed content stream of each page in +// the generated PDF, in page order. It only understands the plain +// (unfiltered, unencrypted) output produced by gpdf's default writer. +func PageContents(t *testing.T, data []byte) []string { + t.Helper() + streams := map[string]string{} + for _, m := range streamObject.FindAllSubmatch(data, -1) { + streams[string(m[1])] = string(m[2]) + } + var pages []string + for _, m := range pageContentsRef.FindAllSubmatch(data, -1) { + content, ok := streams[string(m[1])] + if !ok { + t.Fatalf("content stream %s 0 obj not found", m[1]) + } + pages = append(pages, content) + } + if len(pages) == 0 { + t.Fatal("no pages found in PDF") + } + return pages +} + +// AssertTableSplitsFromFirstPage checks that a table preceded by other +// content starts on the first page (right after intro), that its header +// is repeated on every page, and that every body row is rendered exactly +// once across the whole document. +func AssertTableSplitsFromFirstPage(t *testing.T, data []byte, intro, header string, rows []string) { + t.Helper() + pages := PageContents(t, data) + if len(pages) < 2 { + t.Fatalf("expected the table to span multiple pages, got %d page(s)", len(pages)) + } + lit := func(s string) string { return "(" + s + ")" } + + if !strings.Contains(pages[0], lit(intro)) { + t.Errorf("expected %q on page 1", intro) + } + if !strings.Contains(pages[0], lit(rows[0])) { + t.Errorf("expected first table row %q on page 1 (table must start on the first page)", rows[0]) + } + for i, p := range pages { + if got := strings.Count(p, lit(header)); got != 1 { + t.Errorf("page %d: expected table header %q once, got %d", i+1, header, got) + } + } + all := strings.Join(pages, "\n") + for _, r := range rows { + if got := strings.Count(all, lit(r)); got != 1 { + t.Errorf("expected row %q exactly once, got %d", r, got) + } + } +} diff --git a/template/grid.go b/template/grid.go index 759156d..4d65183 100644 --- a/template/grid.go +++ b/template/grid.go @@ -135,14 +135,16 @@ func (r *RowBuilder) Col(span int, fn func(c *ColBuilder)) { // 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. +// +// Rows containing a table are the exception: tables paginate themselves +// (splitting between body rows and repeating the header), so such rows use +// BreakAuto and the table starts on the current page instead of being +// pushed to the next one. 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 { @@ -164,9 +166,30 @@ func (r *RowBuilder) build(height document.Value, auto bool) document.DocumentNo box.Content = append(box.Content, colBox) } + if !containsTable(box.Content) { + box.BreakPolicy.BreakInside = document.BreakAvoid + } + return box } +// containsTable reports whether any of the given nodes, or their +// descendants, is a table. +func containsTable(nodes []document.DocumentNode) bool { + for _, n := range nodes { + if n == nil { + continue + } + if n.NodeType() == document.NodeTable { + return true + } + if containsTable(n.Children()) { + return true + } + } + return false +} + // ColBuilder constructs content within a column. type ColBuilder struct { doc *Document diff --git a/template/template_test.go b/template/template_test.go index 2f32b2f..6f762e9 100644 --- a/template/template_test.go +++ b/template/template_test.go @@ -2038,3 +2038,35 @@ func TestRowBuildSetsBreakInsideAvoid(t *testing.T) { }) } } + +// TestRowBuildTableRowIsSplittable verifies that rows containing a table +// (directly or nested in a column box) use BreakInside=BreakAuto, so the +// table starts on the current page and paginates itself with a repeated +// header instead of the whole row being pushed to the next page. +func TestRowBuildTableRowIsSplittable(t *testing.T) { + tests := []struct { + name string + fn func(c *ColBuilder) + }{ + {"Direct", func(c *ColBuilder) { + c.Table([]string{"A"}, [][]string{{"1"}}) + }}, + {"Nested", func(c *ColBuilder) { + c.nodes = append(c.nodes, &document.Box{Content: []document.DocumentNode{&document.Table{}}}) + }}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + rb := &RowBuilder{} + rb.Col(4, func(c *ColBuilder) { c.Text("side") }) + rb.Col(8, tc.fn) + box, ok := rb.build(document.Mm(10), true).(*document.Box) + if !ok { + t.Fatal("expected *document.Box") + } + if box.BreakPolicy.BreakInside != document.BreakAuto { + t.Errorf("expected BreakInside=BreakAuto for a row containing a table, got %v", box.BreakPolicy.BreakInside) + } + }) + } +}