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

Break on breaks for View components #2411

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

Scong
Copy link

@Scong Scong commented Oct 11, 2023

Changes

  1. Add logic for node splitting to continue to split nodes while there is a nested break inside of attached to a View. Some adjustments to logic to avoid a possible infinite loop.

  2. Change to unwrappable fallback behavior for View / Text components to wrap when they don't fit within a page, to avoid content being lost. A warning is still logged, this behavior should likely be handled explicitly.

Notes:

Splitting on nested text breaks has some complexities, I believe, in that Lines are computed at the top level Text component (in the case of text components inside of text components). So a recomputation of lines would need to occur after doing a regular node split or such.

I believe nested text breaking is not really necessary, and should be able to be accomplished by adding break tags to view components. All though digging to top level text components with break tags could be reasonable to add. I added a test to demonstrate the relating behavior for nested text / view breaks.

This relies on the split logic and supporting logic behaving reasonably on deeply nested items. It appeared to, all though there my be other considerations here?

I published the fork to npm scong-react-fork-pdf-renderer for our use. (This is done through the distribution-2 branch in my fork repo)

Some related issues / enhancements

#1037

#1676

@changeset-bot
Copy link

changeset-bot bot commented Oct 11, 2023

🦋 Changeset detected

Latest commit: d535e61

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@react-pdf/layout Minor
@react-pdf/renderer Patch
@react-pdf/examples Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@Scong
Copy link
Author

Scong commented Oct 24, 2023

I believe there are additional considerations that need to be handled by the split function in order to handle this appropriately. All though the tests behave as expected, in more complicated reports the splitting still does not fully behave as desired in that some nested view page breaks do not fully break the page.

If someone could point me in the right direction here that would be helpful, there seems to be a lot going on here generally. I believe the digging behavior here is reasonable, but I wonder if a height adjustment or some other consideration is needed to get this working right.

@Scong
Copy link
Author

Scong commented Oct 30, 2023

@diegomura I know in the past you mentioned that nested page breaks is a tricky topic. Do you know where else I should be looking, I believe there is some consideration around height re-computation perhaps?

We really like the library, unfortunately we need a bit more nested pagination support for our report generation consistency. Appreciate any insight here!

@RichMatthews
Copy link

would like some more information here too!

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

2 participants