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

correction on page break when cell is too height #57

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

simon-feamzy
Copy link

No description provided.

@ELIONET
Copy link

ELIONET commented Dec 6, 2022

@natancabral Could you please accept this pull request ?

@fellmann
Copy link

@simon-feamzy

  1. I found the correct calculation to be:
    if (options.useSafelyMarginBottom && rowBottomY + rowHeight + columnSpacing + safelyMarginBottom >= maxY && !lockAddPage) onFirePageAdded();
    You were missing columnSpacing.
  2. You fixed the bug in line 661, same bug is in line 811

@natancabral This seems a critical bug to me. Is this library still maintained?

@fellmann
Copy link

Plus, if the calculation is fixed, I would propose to reduce the magic safelyMarginBottom. Either to 0, or a small fixed amount like

let safelyMarginBottom = 14 /* points = 0.5 cm */

Right now, it is half the top margin, which is a problem with a large top margin e.g. if there is a letterhead.

@fellmann
Copy link

fellmann commented Jun 28, 2023

P.S.: fixes #62

jjchiw added a commit to jjchiw/pdfkit-table that referenced this pull request Jul 7, 2023
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.

None yet

3 participants