Feature/repeat header on page breaks#567
Open
azamat7g wants to merge 8 commits into
Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdd repeat-on-page-break: extend the Row interface and implementation, update Maroto’s page-break logic to copy marked rows onto new pages, and add mocks, tests, an example, a JSON fixture, and documentation plus a sidebar entry. ChangesRepeat Header on Page Breaks
Sequence DiagramsequenceDiagram
participant Client as Client
participant Maroto as Maroto
participant Page as Page
participant Row as Row
Client->>Maroto: AddRow(row)
Maroto->>Row: IsRepeatOnPageBreak?
Maroto->>Page: fitsOnCurrentPage? (height check)
alt does not fit
Maroto->>Maroto: collectRepeatRows()
Maroto->>Page: fillPageToAddNew() / addHeader()
Maroto->>Maroto: addRepeatRows(repeatRows)
Maroto->>Maroto: recheck capacity (header+repeatRows)
alt still does not fit
Maroto->>Page: fillPageToAddNew() / addHeader()
end
Maroto->>Page: add overflowing row
else fits
Maroto->>Page: append row
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: Turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 👉 Get your free trial and get 200 agent minutes per Slack user (a $50 value). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/v2/features/repeatheader.md`:
- Around line 58-72: The example uses text.NewCol with the wrong signature and
nests it inside col.New(...).Add(...), causing compilation: change usages of
col.New(...).Add(text.NewCol(...)) and
row.New(...).Add(col.New(...).Add(text.NewCol(...))) to use
text.NewCol(gridSize, value) directly as the column component (e.g., replace the
col.New(...).Add(text.NewCol(...)) pattern with just text.NewCol(gridSize,
"value")) and update all similar data-row and header-row usages (referencing
text.NewCol, col.New, row.New, m.AddRow, m.AddRows); also ensure fmt is imported
in the example imports.
- Around line 130-207: Add missing language specifiers and blank lines around
fenced code blocks and ensure headings are surrounded by blank lines: add a
language tag (e.g., ```text) to the "Output Structure" example block and use
```go for Go examples (the blocks that contain "m.RegisterHeader(...)", "cfg :=
config.NewBuilder()", and the "header := row.New..." examples), and insert a
blank line before and after each fenced block and before/after the headings
"Single Repeat Header", "Multiple Repeat Headers (Section Headers)" and "Styled
Repeat Headers" so the markdownlint MD040, MD031 and MD022 violations are
resolved.
In `@maroto.go`:
- Around line 224-236: The code appends row r unconditionally after
m.addHeader() and m.addRepeatRows(), which can cause m.currentHeight + rowHeight
to exceed the page maxHeight; update the flow in the block that uses
collectRepeatRows(), fillPageToAddNew(), addHeader(), and addRepeatRows() to
re-check available space before appending r: compute or check if
m.currentHeight+rowHeight > m.maxHeight (or call the same page-break logic as
fillPageToAddNew()) and if so start a new page (adjust currentHeight, call
addHeader()/addRepeatRows() as needed) before doing m.currentHeight += rowHeight
and m.rows = append(m.rows, r) so r never overflows the page boundary.
- Around line 380-395: collectRepeatRows is including rows that were registered
via RegisterHeader/addHeader, causing header rows with WithRepeatOnPageBreak to
be duplicated; fix by filtering out header-managed rows when collecting repeats:
in collectRepeatRows build a lookup of rows that addHeader/RegisterHeader added
(e.g., use m.headerRows or create a temporary map keyed by row identity) and
skip any r present in that lookup or marked as header before appending; this
ensures collectRepeatRows only returns true repeat-on-page-break rows not
already handled by addHeader, leaving addRepeatRows unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8b5df5e7-030c-4b5e-b9c5-325cead857c1
📒 Files selected for processing (10)
docs/v2/features/_sidebar.mddocs/v2/features/repeatheader.mdmaroto.gomaroto_test.gomocks/Row.gopkg/components/row/example_test.gopkg/components/row/row.gopkg/components/row/row_test.gopkg/core/core.gotest/maroto/maroto_add_row_repeat.json
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
docs/v2/features/repeatheader.md (1)
46-52:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix non-compiling Go examples (
fmtimport andtext.NewColusage).
Line 69-71usesfmt.Sprintfbutfmtis not imported;Line 48importscolbut it’s unused in the first example.
Also,Line 95-105still usescol.New(...).Add(text.NewCol(...))with the oldtext.NewColcall style; this example will not compile as written.Suggested patch
import ( + "fmt" "github.com/johnfercher/maroto/v2" - "github.com/johnfercher/maroto/v2/pkg/components/col" "github.com/johnfercher/maroto/v2/pkg/components/row" "github.com/johnfercher/maroto/v2/pkg/components/text" "github.com/johnfercher/maroto/v2/pkg/config" )func (p *Product) GetHeader() core.Row { return row.New(8). - Add(col.New(3).Add(text.NewCol("Item ID"))). - Add(col.New(6).Add(text.NewCol("Product Name"))). - Add(col.New(3).Add(text.NewCol("Price"))). + Add(text.NewCol(3, "Item ID")). + Add(text.NewCol(6, "Product Name")). + Add(text.NewCol(3, "Price")). WithRepeatOnPageBreak() // Header repeats on page breaks } func (p *Product) GetContent(i int) core.Row { return row.New(6). - Add(col.New(3).Add(text.NewCol(p.ID))). - Add(col.New(6).Add(text.NewCol(p.Name))). - Add(col.New(3).Add(text.NewCol(fmt.Sprintf("$%.2f", p.Price)))) + Add(text.NewCol(3, p.ID)). + Add(text.NewCol(6, p.Name)). + Add(text.NewCol(3, fmt.Sprintf("$%.2f", p.Price))) }#!/bin/bash # Verify this docs file still contains problematic patterns. rg -n 'fmt\.Sprintf|pkg/components/col|col\.New\(|text\.NewCol\("[^"]+"\)' docs/v2/features/repeatheader.mdAlso applies to: 69-71, 95-105
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/v2/features/repeatheader.md` around lines 46 - 52, Add the missing "fmt" import, remove the unused "col" import in the first example, and update the old text.NewCol usage to the current struct form so examples compile: import fmt into the import block and delete the unused github.com/johnfercher/maroto/v2/pkg/components/col import from the first snippet; then replace any col.New(...).Add(text.NewCol("...")) calls with col.New(...).Add(text.Col{Text: fmt.Sprintf("...")}) (or text.Col{Text: "..."} when no formatting is needed) so the examples use the current text.Col symbol instead of the deprecated text.NewCol.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/v2/features/repeatheader.md`:
- Around line 130-135: The "## Output Structure" section is empty and should
either be completed with the promised example or removed; update the
docs/v2/features/repeatheader.md by either inserting the intended example
showing the new page structure when a page break occurs (diagram, prose, or code
block) immediately after the "## Output Structure" heading, or delete the entire
"## Output Structure" heading and any placeholder line so the flow jumps
directly to "## Related Features"; ensure the change keeps formatting consistent
with surrounding markdown.
---
Duplicate comments:
In `@docs/v2/features/repeatheader.md`:
- Around line 46-52: Add the missing "fmt" import, remove the unused "col"
import in the first example, and update the old text.NewCol usage to the current
struct form so examples compile: import fmt into the import block and delete the
unused github.com/johnfercher/maroto/v2/pkg/components/col import from the first
snippet; then replace any col.New(...).Add(text.NewCol("...")) calls with
col.New(...).Add(text.Col{Text: fmt.Sprintf("...")}) (or text.Col{Text: "..."}
when no formatting is needed) so the examples use the current text.Col symbol
instead of the deprecated text.NewCol.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 12fae359-3a04-4a6d-8131-77ea2451144b
📒 Files selected for processing (1)
docs/v2/features/repeatheader.md
There was a problem hiding this comment.
♻️ Duplicate comments (2)
docs/v2/features/repeatheader.md (2)
93-99:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winIncomplete fix: Same compilation error from previous review.
The fix from the earlier review (commit 1e0965b) corrected the first example but missed this second example. Lines 95-97 still use the broken pattern
col.New(3).Add(text.NewCol("Item ID")), which has two problems:
text.NewColrequires(gridSize int, value string)— the grid-size argument is missing.col.Addexpects...core.Component, buttext.NewColreturnscore.Col(incompatible interfaces).This pattern is also repeated in lines 103-105.
🐛 Proposed fix — use `text.NewCol` directly
func (p *Product) GetHeader() core.Row { return row.New(8). - Add(col.New(3).Add(text.NewCol("Item ID"))). - Add(col.New(6).Add(text.NewCol("Product Name"))). - Add(col.New(3).Add(text.NewCol("Price"))). + Add(text.NewCol(3, "Item ID")). + Add(text.NewCol(6, "Product Name")). + Add(text.NewCol(3, "Price")). WithRepeatOnPageBreak() // Header repeats on page breaks } func (p *Product) GetContent(i int) core.Row { return row.New(6). - Add(col.New(3).Add(text.NewCol(p.ID))). - Add(col.New(6).Add(text.NewCol(p.Name))). - Add(col.New(3).Add(text.NewCol(fmt.Sprintf("$%.2f", p.Price)))) + Add(text.NewCol(3, p.ID)). + Add(text.NewCol(6, p.Name)). + Add(text.NewCol(3, fmt.Sprintf("$%.2f", p.Price))) }Note: Ensure
fmtis also imported in the complete example.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/v2/features/repeatheader.md` around lines 93 - 99, The GetHeader example still uses the broken pattern col.New(...).Add(text.NewCol("...")) which omits the gridSize arg and mismatches types: text.NewCol(gridSize, value) returns core.Col and should be added directly to the row, not wrapped by col.New(...).Change Product.GetHeader to call text.NewCol with an explicit grid size and value (e.g. text.NewCol(3, "Item ID"), text.NewCol(6, "Product Name"), text.NewCol(3, "Price")) and pass those directly into row.New(...).Add(...); also ensure fmt is imported in the surrounding example as noted.
46-52:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winMissing
fmtimport breaks the example.Line 69 uses
fmt.Sprintf, but thefmtpackage is not imported. This example will not compile.🐛 Proposed fix
import ( + "fmt" + "github.com/johnfercher/maroto/v2" "github.com/johnfercher/maroto/v2/pkg/components/col" "github.com/johnfercher/maroto/v2/pkg/components/row" "github.com/johnfercher/maroto/v2/pkg/components/text" "github.com/johnfercher/maroto/v2/pkg/config" )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/v2/features/repeatheader.md` around lines 46 - 52, The example is missing the fmt import used by fmt.Sprintf, causing a compile error; update the import block in this file to include the "fmt" package so references to fmt.Sprintf compile successfully, locating the import list near the existing imports (github.com/johnfercher/maroto/v2, components/col, row, text, and config) and adding "fmt" alongside them.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@docs/v2/features/repeatheader.md`:
- Around line 93-99: The GetHeader example still uses the broken pattern
col.New(...).Add(text.NewCol("...")) which omits the gridSize arg and mismatches
types: text.NewCol(gridSize, value) returns core.Col and should be added
directly to the row, not wrapped by col.New(...).Change Product.GetHeader to
call text.NewCol with an explicit grid size and value (e.g. text.NewCol(3, "Item
ID"), text.NewCol(6, "Product Name"), text.NewCol(3, "Price")) and pass those
directly into row.New(...).Add(...); also ensure fmt is imported in the
surrounding example as noted.
- Around line 46-52: The example is missing the fmt import used by fmt.Sprintf,
causing a compile error; update the import block in this file to include the
"fmt" package so references to fmt.Sprintf compile successfully, locating the
import list near the existing imports (github.com/johnfercher/maroto/v2,
components/col, row, text, and config) and adding "fmt" alongside them.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f2e84b65-0cc7-48ba-895a-4d89f67ed717
📒 Files selected for processing (1)
docs/v2/features/repeatheader.md
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@maroto.go`:
- Around line 232-239: The current flow calls m.addRepeatRows(repeatRows) then,
if the row still doesn't fit, calls m.fillPageToAddNew() and m.addHeader(),
which can produce a page with only header/repeat rows and place the overflow row
on a page without repeats; change the logic so that when
m.currentHeight+rowHeight > maxHeight-m.footerHeight after calling
m.addRepeatRows(repeatRows) you perform the page break (m.fillPageToAddNew()),
call m.addHeader(), and then re-add the repeat rows (e.g., call
m.addRepeatRows(repeatRows) again or move the original addRepeatRows call to
after addHeader()) before placing the overflow row so the overflow row always
has its repeat rows on the same new page.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Implements row-level repeat-on-page-break functionality. Allows marking individual rows to be automatically re-injected at the top of new pages when content overflows.
Use Case: Multi-page tables (invoices, reports, data exports) where column headers must repeat on every new page.
How It Works
Comparison: Global Header vs Repeat Header
RegisterHeader()WithRepeatOnPageBreak()m.RegisterHeader(rows...)row.New().WithRepeatOnPageBreak()Checklist
func (<first letter of struct> *struct) method() {}name style.when,shouldnaming pattern.m := mocks.NewConstructor(t).m.EXPECT().MethodName()method to mock methods.example_test.go.make dodwith none issues pointed out bygolangci-lint