Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add custom page margins #122

Merged

Conversation

happsie
Copy link
Contributor

@happsie happsie commented Dec 20, 2019

Description

  • Added a new method on the Maroto PDF interface to allow backwards
    compatibility instead of adding extra parameter to NewMaroto

This PR makes it possible to set other margins than the standard (10, 10, 10)

Picture of sample1 modified with SetPageMargins. 30 left, 20 top and 0 right.
m.SetPageMargins(30, 20, 0)
Skärmavbild 2019-12-31 kl  11 14 07

Related Issue
resolve #123

Checklist

check with "x", if applied to your change

  • All methods associated with structs has func (s *struct) method() {} name style.
  • Wrote unit tests for new/changed features.
  • Updated docs/doc.go
  • Updated pkg/pdf/example_test.go
  • Updated README.md
  • Updated all examples inside internal/examples
  • New public methods has comments upside them explaining what it does
  • Executed go fmt github.com/johnfercher/maroto/... to format all files

johnfercher and others added 3 commits November 11, 2019 00:17
* Added a new method on the Maroto PDF interface to allow backwards
compatibility instead of adding extra parameter to NewMaroto
@codecov-io
Copy link

codecov-io commented Dec 20, 2019

Codecov Report

Merging #122 into dev will increase coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #122      +/-   ##
==========================================
+ Coverage   95.22%   95.25%   +0.04%     
==========================================
  Files           9        9              
  Lines         522      526       +4     
==========================================
+ Hits          497      501       +4     
  Misses         21       21              
  Partials        4        4
Impacted Files Coverage Δ
pkg/pdf/pdf.go 100% <100%> (ø) ⬆️
internal/signature.go 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 73e20ce...7a8d463. Read the comment docs.

@happsie happsie marked this pull request as ready for review December 20, 2019 20:59
pkg/pdf/pdf_test.go Outdated Show resolved Hide resolved
Copy link
Owner

@johnfercher johnfercher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think your feature is nice. I let some comments, could you see it?

Just one more thing, could you provide in your pull request a print screen of the effect of your modification? Maybe just run the sample1 and see if everything works properly with your modificati0on.

https://github.com/johnfercher/maroto/tree/master/internal/examples/sample1

pkg/pdf/pdf.go Outdated Show resolved Hide resolved
pkg/pdf/pdf_test.go Outdated Show resolved Hide resolved
@johnfercher
Copy link
Owner

Looks like the signature component doesn´t work well with this, I will take a look in this issue before merge your PR.

@happsie happsie force-pushed the feature/add-custom-page-margins branch from 94ae077 to 08dfd11 Compare December 20, 2019 22:17
@happsie
Copy link
Contributor Author

happsie commented Dec 20, 2019

Update PR with improvements from comments.

@johnfercher
Copy link
Owner

Have a look to https://github.com/Jepzter/maroto/pull/1/files, I fixed the issue in the signatures

@johnferchermeli
Copy link
Contributor

Could you merge my PR into your branch? So I will be able to merge your branch into dev.

@johnferchermeli
Copy link
Contributor

Niice, could you update the sample image with the fix?

@happsie
Copy link
Contributor Author

happsie commented Dec 31, 2019

Sorry for being late, updated PR with image.

@johnferchermeli
Copy link
Contributor

Hey, happy new year =) I will merge this PR today.

@happsie
Copy link
Contributor Author

happsie commented Jan 5, 2020

Happy new year to you to! Is there any reason this is still open I can fix?

@johnferchermeli johnferchermeli merged commit d2999b6 into johnfercher:dev Jan 7, 2020
This was referenced Jan 8, 2020
Closed
felix pushed a commit to felix/maroto that referenced this pull request May 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants