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

Support custom 'Content-Disposition' options #40

Merged
merged 17 commits into from
Aug 7, 2019

Conversation

ATMartin
Copy link
Contributor

@ATMartin ATMartin commented Aug 7, 2019

Resolves #33 by allowing much more detailed control of the Content-Disposition header generated by prawn-rails:

  • If the header is already set, don't override it.
  • If the header isn't set, set it from the @filename ivar (preserves current functionality).
  • If there's no @filename set, use the :filename and :disposition option keys on the prawn_document helper.
  • No keys? Use sensible defaults (Content-Disposition=inline;)

Also: updated tests & README.

options = get_prawn_options

# Don't override the 'Content-Disposition' if we've chosen to set it elsewhere.
controller.response.headers['Content-Disposition'] ||= "#{options[:disposition]}; filename=\"#{options[:filename]}\""
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ||= was the biggest pain point we had - we set our own Content-Dispositions in our controllers, and having the gem totally scrap those was frustrating!

README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
@ATMartin
Copy link
Contributor Author

ATMartin commented Aug 7, 2019

@westonganger I've updated this to be prawn_document-centric. Let me know if we need to push in a different direction!

README.md Outdated
end
```

This uses the [`Content-Disposition` HTTP header](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Disposition#As_a_response_header_for_the_main_body). If you've already set this header, `prawn-rails` will *not* override it.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you maybe show an example usage/assignment of this header?

@westonganger
Copy link
Collaborator

Thanks! Could you also update the changelog?

@ATMartin
Copy link
Contributor Author

ATMartin commented Aug 7, 2019

@westonganger Updated. I bumped to 1.3.x since this could break someone using the old renderer with Prawn::Document.new. Thanks for the README tweak!

@westonganger westonganger merged commit 14a24a9 into cortiz:master Aug 7, 2019
@westonganger
Copy link
Collaborator

Merged! Thanks for your contribution!

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.

Add an option to control Content-Disposition
2 participants