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

`Prawn::Document#on_page_create` in v 1.3.0 #797

Closed
BrentWheeldon opened this Issue Oct 27, 2014 · 15 comments

Comments

Projects
None yet
7 participants
@BrentWheeldon

BrentWheeldon commented Oct 27, 2014

Hi there,

We have some code that does, effectively:

document = Prawn::Document.new(...)
# stuff
document.on_page_create do |doc|
 # more stuff
end

Since upgrading to 1.3.0 we get: NoMethodError: undefined method 'on_page_create' for #<Prawn::Document:0x007f92f8a88fd0> when running this code. I checked the changelog, but didn't see anything about this method, and I can't see it in the source anymore. I can see it that it's been removed from lib/prawn/document/internals.rb in b48b206, but it's not clear if there's an alternative?

Any help would be greatly appreciated!

@practicingruby

This comment has been minimized.

Member

practicingruby commented Oct 27, 2014

@BrentWheeldon:

on_page_create is a low-level method that's not part of the public API. (repeat or number_pages are often used instead). If those support your use case, I'd recommend using them instead. If not, please let me know what you're using on_page_create for, so we can think about how to support your use case.

But an immediate fix for 1.3 would be to call renderer.on_page_create, just don't assume it's going to be a stable API.

@BrentWheeldon

This comment has been minimized.

BrentWheeldon commented Oct 27, 2014

Thanks for the quick reply, @sandal!

We're basically abusing that private API to keep track of which "section" we're in inside of a report to render that in a header. This is some inherited code that inherited so I've not dig in too deep. I'll see if we can do this in a supported way.

@practicingruby

This comment has been minimized.

Member

practicingruby commented Oct 27, 2014

@BrentWheeldon: This may be something we could offer support for (or build a tiny extension for) in the future. See this thread for what I think is a reasonably clean solution: https://groups.google.com/forum/#!topic/prawn-ruby/_KIg4Ih8TdU

@practicingruby

This comment has been minimized.

Member

practicingruby commented Oct 27, 2014

Here's a direct link to the relevant code sample: https://groups.google.com/d/msg/prawn-ruby/_KIg4Ih8TdU/MaHBtDu2F4sJ

@mojavelinux

This comment has been minimized.

Contributor

mojavelinux commented Oct 28, 2014

We also use this method in Asciidoctor PDF to paint the background color of the page.

https://github.com/asciidoctor/asciidoctor-pdf/blob/master/lib/asciidoctor-pdf/converter.rb#L96

Is there another way to accomplish this?

@practicingruby

This comment has been minimized.

Member

practicingruby commented Oct 28, 2014

@mojavelinux:

Maybe not, because repeaters run last, so the z-index is going to be wrong (your background would be rendered on top of the page contents rather than behind them).

I'm convinced that restoring on_page_create as a public method on Prawn::Document is reasonable. Anyone is welcome to prepare a patch. (It should simply delegate to renderer.on_page_create, which is the workaround for 1.3.x)

@practicingruby

This comment has been minimized.

Member

practicingruby commented Oct 28, 2014

@mojavelinux: Also, we should think about what first-order support for setting background colors might look like, since it's a tedious operation currently. A patch or feature-request ticket for that is welcome!

@mojavelinux

This comment has been minimized.

Contributor

mojavelinux commented Oct 28, 2014

@sandal Thanks for the clarification / input. I'll give it some thought when I upgrade Asciidoctor PDF to the latest Prawn.

@mikesax

This comment has been minimized.

mikesax commented Nov 11, 2014

We've also been using on_page_create to prepare a page and create background graphics. It would be nice to either have on_page_create back, or to have a feature that lets repeaters optionally run before the main content is rendered, so the z-order is correct. For example:

pdf.repeat(:all, before: true) do
   ...
end
@practicingruby

This comment has been minimized.

Member

practicingruby commented Nov 11, 2014

@mikesax. For the short term, I'd definitely like to bring back on_page_create. Controlling the order of when repeat calls get run is an interesting idea... I'll have to think on that some more.

@nathansamson

This comment has been minimized.

nathansamson commented Jan 2, 2015

document.on_page_create do |doc|
 # more stuff
end

Obviously only works for the second page and later, unless you do pass skip_page_creation: true on document creation...

So even if on_page_create is resurected it feels a bit clunky... Repeaters with different z-indexes sounds nice though

@practicingruby

This comment has been minimized.

Member

practicingruby commented Jan 4, 2015

@nathansamson: Yeah, in time for the next release I'm only going to restore the behavior because that's how it's been since basically forever, but that is a good wart to think about dealing with when we produce a better alternative in the future.

jessedoyle added a commit that referenced this issue Feb 19, 2015

@jessedoyle

This comment has been minimized.

Contributor

jessedoyle commented Feb 19, 2015

@practicingruby - I submitted PR #825 that delegates the method call to renderer. Hopefully we can get this in the 2.0.0 release!

practicingruby added a commit that referenced this issue Feb 20, 2015

Merge pull request #825 from prawnpdf/on-page-create
Delegate on_page_create method to renderer. #797
@practicingruby

This comment has been minimized.

Member

practicingruby commented Feb 26, 2015

Merged!

@bousquet

This comment has been minimized.

bousquet commented May 25, 2016

👍 Thanks. Used this today in combination with prawn-svg for crisp background image underneath flowing text.

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