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 an option to control Content-Disposition #33

Closed
TylerRick opened this issue Aug 9, 2018 · 8 comments · Fixed by #40
Closed

Add an option to control Content-Disposition #33

TylerRick opened this issue Aug 9, 2018 · 8 comments · Fixed by #40

Comments

@TylerRick
Copy link

It looks like there's a way to control the filename (by setting @filename), but the Content-Disposition is hard-coded to be "inline":

          controller.response.headers['Content-Disposition'] = "inline; filename=\\\"\#{@filename}\\\""

Could we add an option for controlling whether Content-Disposition is inline or attachment like prawnto has?

      def set_disposition
        inline = options[:inline] ? 'inline' : 'attachment'
        filename = options[:filename] ? "filename=\"#{options[:filename]}\"" : nil
        @controller.headers["Content-Disposition"] = [inline,filename].compact.join(';')
      end

Maybe we need an options hash?

@westonganger
Copy link
Collaborator

Yes this feature would be much appreciated.

Could you possibly create a PR for this? If you do this please also consider if there's a better way we could be setting the filename instead of using an controller instance variable "@filename"

@westonganger
Copy link
Collaborator

@TylerRick any chance to take a stab at this?

@waghanza
Copy link

@cortiz I can make a PR if you want

@westonganger
Copy link
Collaborator

westonganger commented Dec 19, 2018

@waghanza go for it (P.S. I am the current maintainer)

@waghanza
Copy link

ok 😜, I'm on it

@ATMartin
Copy link
Contributor

ATMartin commented Aug 6, 2019

@waghanza Just checking in: are you still working on this feature? I'd like to take over if not. We got burned by this after a recent upgrade.

@westonganger
Copy link
Collaborator

@ATMartin go for it

@waghanza
Copy link

waghanza commented Aug 7, 2019

@ATMartin not working on this anymore, glad you take over

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 a pull request may close this issue.

4 participants