Merged
Conversation
Owner
Author
|
@brandoncc this is what I mentioned earlier in an email. |
excid3
commented
Jul 17, 2025
| header_template: FerrumPdf::DEFAULT_HEADER_TEMPLATE, | ||
| footer_template: FerrumPdf::DEFAULT_FOOTER_TEMPLATE | ||
| } | ||
| ) do |browser, page| |
Owner
Author
There was a problem hiding this comment.
This block is still possible, but only with FerrumPdf.render_pdf() calls directly.
brandoncc
reviewed
Jul 18, 2025
Contributor
brandoncc
left a comment
brandoncc
left a comment
There was a problem hiding this comment.
I found some typos, but mostly good. I remember using a custom renderer many years ago when I hadn't been using Rails very long. It was completely magical, and it was awesome to be able to break that magic down and understand it now!
Comment on lines
+29
to
+36
| class PdfsController < ApplicationController | ||
| def show | ||
| respond_to do |format| | ||
| format.pdf { render ferrum_pdf: {}, disposition: :inline, filename: "example.pdf" } | ||
| format.png { render ferrum_screenshot: {}, disposition: :inline, filename: "example.png" } | ||
| end | ||
| end | ||
| end |
Comment on lines
+13
to
+17
| send_data_options = options.extract!(:disposition, :filename, :status) | ||
| url = pdf_options.delete(:url) | ||
| html = render_to_string(**options.with_defaults(formats: [ :html ])) if url.blank? | ||
| pdf = FerrumPdf.render_pdf(html: html, base_url: request.base_url, url: url, pdf_options: pdf_options) | ||
| send_data(pdf, **send_data_options.with_defaults(type: :pdf)) |
Contributor
There was a problem hiding this comment.
It took me some time to figure this out, but it really is pretty simple. Nice work.
Co-authored-by: Brandon Conway <brandoncc@gmail.com>
Co-authored-by: Brandon Conway <brandoncc@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I never liked injecting the
render_pdfandrender_screenshotmethods into ActionController. We even had a config option to disable it. This got me thinking, what if we could just add a renderer instead?This turned out to be a much cleaner approach and also handles the
send_datacall for users. Plus, it won't conflict with any user code.Renderer options are passed directly to FerrumPDF and the other options are used for
render_to_stringandsend_data.With the new renderer:
More advanced example using Ferrum options, template and layout for
render_to_stringand options forsend_data.This also cleans up the AssetHelper so the classes aren't included in the main application.
Since this removes
render_pdfandrender_screenshotmethods from controllers, we'll want to cut this as a new major release.