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

Add Support for disabling PageScaling in supported PDF Clients #448

Closed
wants to merge 9 commits into from

Conversation

welfare
Copy link
Contributor

@welfare welfare commented Feb 27, 2013

Most PDF clients automatically scales a document if it feels that the document won't fit within the page margins. However, some clients (such as the PDF client in iOS) does not give the user a possibility to disable the print scaling.

This commit adds support to Prawn for disabling print_scaling by supplying the option :print_scaling => :none

@plehoux
Copy link

plehoux commented May 25, 2013

👍

@ghost ghost assigned practicingruby Nov 16, 2013
@practicingruby
Copy link
Member

This needs documentation in order to be merged, but otherwise looks fine. Please re-open when it is documented, and I'll merge it.

@jlduran
Copy link

jlduran commented Dec 30, 2013

PrintScaling

(Optional; PDF 1.6) The page scaling option to be selected when a print dialog is displayed for this document. Valid values are None, which indicates that the print dialog should reflect no page scaling, and AppDefault, which indicates that applications should use the current print scaling. If this entry has an unrecognized value, applications should use the current print scaling. Default value: AppDefault.

Note: If the print dialog is suppressed and its parameters are provided directly by the application, the value of this entry should still be used.

@welfare @sandal

* Remove RDoc task now that we're using YARD
* Make gemfile read from gemspec for dependencies
@practicingruby practicingruby reopened this Jan 2, 2014
@practicingruby
Copy link
Member

If this is a feature that is only in PDF 1.6+, I think we'd need a min_version guard in this code. Is that correct @bradediger?

@bradediger
Copy link
Member

@sandal Correct. We'd need a min_version(1.6).

@practicingruby
Copy link
Member

Okay... so this patch needs a few things then, IMO.

  • Add a min_version call to make sure we're setting the right PDF version.
  • Wrap lines cleanly at 80 chars
  • Use the notes posted above from @jlduran to write appropriate API documentation
  • Fix manual example to actually use the option, and set it to not produce output as we do with some of the
    other examples.
  • Rewrite manual require to use require_relative
  • Resolve merge conflicts

Does someone want to take this up? If you've already got commit access, pushing this code to a branch on prawnpdf and then opening a fresh pull request is the way to go.

@practicingruby
Copy link
Member

I'm closing this out again, but will reopen if someone volunteers to work on the above feedback points.

@welfare
Copy link
Contributor Author

welfare commented Jan 3, 2014

Hey @sandal, I have fixed the following:

  • Added a min_version(1.6) if print_scaling is set to :none
  • Line wrap everywhere I found > 80 chars in my commits
  • Added the notes from @jlduran to the manual
  • Fixed the manual example to use the option
  • Rebased and resolved conflicts

I pushed the changes to my fork but I'm quite new to github so I have no clue what to do next. Please advise :)

@practicingruby practicingruby reopened this Jan 3, 2014
@practicingruby
Copy link
Member

What you did looks OK to me. I will probably rebase down to a single commit before merging, but I think I can take it from here. Thanks!

practicingruby pushed a commit that referenced this pull request Jan 15, 2014
This is #448, just squashed down into a single commit.
@practicingruby
Copy link
Member

Merged, thank you!

You've also been given commit access to Prawn. Please see this page for contribution guidelines:
https://github.com/prawnpdf/prawn/wiki/Contributor-welcome-notes

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

Successfully merging this pull request may close these issues.

5 participants