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

fix "page-break: avoid;" in tables when the avoid blocks are bigger t… #2013

Open
wants to merge 5 commits into
base: development
Choose a base branch
from

Conversation

carli2
Copy link
Contributor

@carli2 carli2 commented May 24, 2024

…han the page size

@finwe
Copy link
Member

finwe commented May 24, 2024

Thanks for the PR. As per contribution guidelines, add a test running a minimal example HTML code affected by this fix.

@carli2
Copy link
Contributor Author

carli2 commented May 24, 2024

I added a unit test that demonstrates what goes wrong without the fix

@carli2
Copy link
Contributor Author

carli2 commented Sep 12, 2024

This is a new pull request with a new issue. Github is kinda buggy when the recent merge request is not completed yet. Important is the support for "%%" in SizeConverter since MS Outlook creates faulty CSS which we should tolerate.

@finwe
Copy link
Member

finwe commented Sep 12, 2024

If you have separate issues, create separate pull requests, each with its own tests.

To warn you, I will be hesitant to incorporate quickhacks for invalid input that can be solved on user side.

@carli2
Copy link
Contributor Author

carli2 commented Sep 12, 2024

I removed the latest commit from this pull request. Test case is present. Awaiting merge.

@carli2
Copy link
Contributor Author

carli2 commented Sep 18, 2024

What is the current status of the review process?

@finwe
Copy link
Member

finwe commented Sep 18, 2024

On hold. Will need to find time to consider how this might impact non-corner-case uses, as it seems hacky.

@carli2
Copy link
Contributor Author

carli2 commented Sep 18, 2024

Maybe I should give some more details what the commit actually does.

sometimes, the user provides big page-break:avoid blocks that do not fit a single page. The old algorithm was faulty because it just printed the first row of each "avoid" block, the second row on the next page and so on.

With the fix, the mpdf detects that the block cannot avoid a break and thus splits it up anyway.

I also added a test case via d60c359 which shows that the fix drastically reduces the amount of pages spilled with blocks being too big.

A general flaw of the current page break system is that mpdf takes the biggest row ($maxrowheight) to estimate the amount of rows to print. An exact break would be better especially in cases where rows have varying height (like in invoices with a lot of detail text in one of the rows)

@carli2
Copy link
Contributor Author

carli2 commented Sep 24, 2024

When will this pull request be merged? It is very important to not waste paper on many printouts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants