johnfercher / maroto

A maroto way to create PDFs. Maroto is inspired in Bootstrap and uses gofpdf. Fast and simple.
https://maroto.io
MIT License
2.15k stars 206 forks source link

Minimum Top Marge 0 Acceptance #192

Open EddieSCJ opened 2 years ago

EddieSCJ commented 2 years ago

I can't define top margin as zero

To Reproduce Code to reproduce the behavior:

package main

import (
    "fmt"
    "fruitfulpdf/data"
    "github.com/johnfercher/maroto/pkg/color"
    "github.com/johnfercher/maroto/pkg/consts"
    "github.com/johnfercher/maroto/pkg/pdf"
    "github.com/johnfercher/maroto/pkg/props"
    "os"
)

func main() {
    m := pdf.NewMaroto(consts.Portrait, consts.A4)
    m.SetPageMargins(0, 0, 0)

    buildHeading(m)

    if err := m.OutputFileAndClose("pdfs/fruits.pdf"); err != nil {
        fmt.Println("Couldn't save PDF: ", err)
        os.Exit(1)
    }
    fmt.Println("PDF saved successfully")
}

func buildHeading(m pdf.Maroto) {
    m.SetBackgroundColor(color.Color{
        Red:   3,
        Green: 166,
        Blue:  166,
    })
    m.Row(10, func() {
        m.Col(12, func() {
            m.Text("Products", props.Text{
                Top:    2,
                Size:   13,
                Color:  color.NewWhite(),
                Family: consts.Arial,
                Style:  consts.Bold,
                Align:  consts.Center,
            })
        })
    })
}

I just think it's because inside this method it only changes the s.marginTop when the top value is greater than ten, so the minimum top margin will EVER be ten and it's not customizable follow the code.

The need to have it greater than ten is because in NewMarotoCustomSize the margin will be ten by default, and in the SetPageMargins it will not change at all for the top margin, so we need a marginTop in PdfMaroto to make some calculus.

// SetPageMargins overrides default margins (10,10,10)
// the new page margin will affect all PDF pages
func (s *PdfMaroto) SetPageMargins(left, top, right float64) {
    if top > 10 {
        s.marginTop = top - 10
    }

    s.Pdf.SetMargins(left, 10.0, right)
}

The SetPageMargins was supposed to override the default margins, but it does not override the top margin anyway, but it is calculated in someplace with a sum.

Sometimes, in part of the code as in the method GetPageMargins, the maroto top margin is added to the pdf top margin. Idk if it could occur in other pieces of code.

func (s *PdfMaroto) GetPageMargins() (left float64, top float64, right float64, bottom float64) {
    left, top, right, bottom = s.Pdf.GetMargins()
    top += s.marginTop

    return
}

My suggestion is to make have no condition to set the s.marginTop value in another method, so we can use this without break the current code

Something like

// SetPageMargins overrides default margins (10,10,10)
// the new page margin will affect all PDF pages
func (s *PdfMaroto) SetPageMargins(left, top, right float64) {
        s.marginTop = top - 10
    s.Pdf.SetMargins(left, 10.0, right)
}

func (s *PdfMaroto) SetTopPageMargin(top float64) {
    s.marginTop = top - 10
}
tg-mvooges commented 2 years ago

Two questions: I see that it is assigned;

johnferchermeli commented 2 years ago

There is a bug in gofpdf that doesn't allow customize margins completely, we don't know when it will be solved.

tg-mvooges commented 2 years ago

There is a bug in gofpdf that doesn't allow customize margins completely, we don't know when it will be solved.

Is there a ticket for this bug? I've looked around at https://github.com/go-pdf/fpdf/issues, but cant seem to find it. Could you make a ticket there? I could also do it, but I'm guessing you're more into the details then I am.

johnferchermeli commented 2 years ago

Actually is this repo here (https://github.com/jung-kurt/gofpdf), which maroto use. The main issue is that this repo is archived, so we would have to create a fork and use it. We don't know when we will start to work in this bug.

tg-mvooges commented 2 years ago

Hm thats understandable, but unfortunate. We're just getting started with Go, so I can't offer to help fix it :(

tg-mvooges commented 2 years ago

We're currently at the point where we need to decide wether or not we drop Maroto because of the margin issue, so I've spent a little extra time on research to find a possible solution.

First suggestion: I'm kind of assuming you are aware that there is more modern code: https://github.com/jung-kurt/gofpdf/tags I'm not sure wether that'll fix anything, but Maroto uses 1.16, there is a 2.17

Second suggestion: Based on this site, This package by phpdave11 appears to be maintained a bit better (at the moment of writing: last commit 9 days ago): https://github.com/phpdave11/gofpdf That package, as it is a fork, might be a drop-in replacement :) Could be worth investigating

EddieSCJ commented 2 years ago

Hey, that seems pretty interesting, on this weekend I'll play a little bit with the new versions of gofpdf to see what we can do about.

zejunlitg commented 2 years ago

Hey, that seems pretty interesting, on this weekend I'll play a little bit with the new versions of gofpdf to see what we can do about.

Hi any update on this? I just found the top margin still can't be set to 0..

EddieSCJ commented 2 years ago

@tgzejunli seems no one had time to see, so, what do you think to meet this weekend to study other libs which continued the original ?

tg-mvooges commented 2 years ago

While I'm actively following this thread, I do not have the time or experience (Im a programmer, but a Go Novice) I cant help much in that aspect. I can help think about more generic question, or help think about developing a feature. But in this case I dont think that is a very useful addition.

zejunlitg commented 2 years ago

I ended up changing the source code of this lib, not much but it provides what I need now.

Fernando-hub527 commented 6 months ago

Hello ! Doing some tests, I saw that with the code from example from the latest version of maroto it allows you to set the margin to 0. I'm not sure if I understood correctly, but in this case the problem has already been resolved ? @johnfercher