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

Control of PDPage creation via PDFCreationListener #334

Merged
merged 8 commits into from
Mar 13, 2019

Conversation

DSW-PS
Copy link
Contributor

@DSW-PS DSW-PS commented Mar 11, 2019

Hello,

I think it would be a good idea to give the user more control over how a PDPage is created and added to the PDDocument.

I modified the PdfBoxRenderer to delegate the page-creation to the PDFCreationListener (if one is set).

(In our case we already have PDPage with some data created and now need to print the rest - we could of course just print the html to pdf to import and place the document, but this way it is only a matter of setting where the html is to begin via some styling rules, much more simpler and streamlined)

Kind regards
Peter

DSW-PS and others added 5 commits March 8, 2019 09:18
Added onCreatePage to PDFCreationListener.
Added fireOnCreatePage to PdfBoxRenderer - by default this just creates
a PDPage and adds it to the PDDocument (old behavior).

This allows the user more control over the page that is created and also
control when (or if) a page is to be added to the PDDocument.
danfickle#8 Online sandbox now at RC18. [ci skip]
@DSW-PS DSW-PS changed the title Control fo PDPage creation via PDFCreationListener Control of PDPage creation via PDFCreationListener Mar 11, 2019
@danfickle
Copy link
Owner

Hi @DSW-PS,

Thanks a lot for the PR.

I think that allowing the user to control page creation is a good idea. However, I’m not sure that using PDFCreationListener is the way to go. Firstly, I’m not sure that its methods are all being called in the right place, so that would need analyzing before adding it to the official documented api. Secondly, the modern trend is towards lambda compatible interfaces for callbacks.

So I would prefer a specific lambda compatible interface for the callback and the ability to set it from the builder. Maybe usePageSupplier. Finally we need the page number and shadow page number as part of the arguments.

What do you think? If you think this is reasonable, I could do it when time permits, or it would be great if this pr or another implemented it!

Thanks again,
Daniel.

P.S. Anyone have any idea why Travis ci hasn’t picked this pr up?

@ieugen
Copy link
Contributor

ieugen commented Mar 11, 2019

P.S. Anyone have any idea why Travis ci hasn’t picked this pr up?
Some events may be lost ?

ps added 2 commits March 11, 2019 15:04
(See danfickle#334)

Based on the suggested by @danfickle, reverted the changes to
PDFCreationListener and PdfBoxRenderer, instead a PageSupplier can be
set via the PdfRendererBuilder.

The PageSupplier.requestPage allows the user to create a new page or
supply an existing one where the Html will be printed.

PdfRendererBuilder implements PageSupplier and simply creates a new page
and adds it to the PDDocument.
@DSW-PS
Copy link
Contributor Author

DSW-PS commented Mar 11, 2019

Hi @danfickle,

I tried to understand what the shadow-pages are for (or how they are formed) but I can't say I understand.

Nevertheless I took your advice and removed the changes to the PDFCreationListener and instead put the (possible) creation of the PDPage in the new interface PageSupplier - which can be written as a lambda.
Also added the pageNumer and shadowPageNumber as arguments as you suggested.

Tell me what you think,
Peter

@danfickle
Copy link
Owner

Thanks @DSW-PS,

Shadow pages are there for cut-off pages. The pr is looking good. Only minor concerns:

Page number should be zero based (use i) and continue to use i In the shadow page loop for the page number argument. Other than that, really good.

Added a brief explanation what "shadow-pages" are to the PageSupplier.
As @danfickle pointed out, the current-page index (i) should be used as
pageNumber argument for PageSupplier in PdfBoxRenderer when building the
shadow-pages and not the pdfPageIndex (which at this point has already
been upped by one).
Also first call to PageSupplier should (for the first page) should be
with pageNumber 0, not 1.
@DSW-PS
Copy link
Contributor Author

DSW-PS commented Mar 13, 2019

Hi @danfickle,

thanks for the explanation. I added a brief comment with link to the cut-off pages wiki page to the PageSupplier to help others.

Now using i for pageNumber in the shadow-page loop.

@danfickle
Copy link
Owner

Sensational. Thanks a lot @DSW-PS.

@danfickle danfickle merged commit 6eb8f44 into danfickle:open-dev-v1 Mar 13, 2019
danfickle added a commit that referenced this pull request Mar 17, 2019
I'm the worst offender with tabs but they are evil!
danfickle added a commit that referenced this pull request Mar 17, 2019
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.

3 participants