html2pdf.js icon indicating copy to clipboard operation
html2pdf.js copied to clipboard

Allow html2canvas options to be set across multiple .set() calls

Open brendon opened this issue 6 years ago • 3 comments

I am using Froala that allows one to save the editors contents as a PDF. It uses html2pdf.js to do this, and allows me to pass in the html2pdf function to use. I wanted to set some extra config (specifically html2canvas) but since Froala also sets the html2canvas key via .set(), my configuration got blown away.

This PR allows one to set html2canvas options across multiple set() calls by only setting the individual keys rather than replacing the whole object.

brendon avatar May 15 '19 04:05 brendon

Hi @eKoopmans, just wondering what you thought of this PR. Did you need any adjustments made? :)

brendon avatar Jun 04 '19 01:06 brendon

Hey @brendon, thanks for the PR! Sorry it took a while to get back to you.

This change looks good! I have two thoughts:

  1. There might be use cases where a user would want to completely replace the object, rather than merge. There should probably be a way to do both.
  2. The same sort of merge behaviour would be useful on the jsPDF setting as well.

A possible solution:

  • Keep the set() behaviour unchanged, and
  • Add an update() method which does the merge behaviour for html2canvas and jsPDF settings, and sends any other settings to set().

What do you think?

eKoopmans avatar Jul 07 '19 08:07 eKoopmans

Hi @eKoopmans, I thought that might have been the case (jsPDF settings) when I was working on this :D

The original problem that led me to this PR is that Froala was using set() to set html2canvas options after I had set my own. And their API didn't allow any further opportunity to interact with the settings object after that. Your suggestion would require that they also change the method they use to update() which is certainly feasible, but it just means updating two libraries.

Modifying html2pdf's set() to merge settings for html2canvas and jsPDF in the same way that the top level html2pdf settings are merged seems like a worthy special case for these two objects since they're configuration passed onto other libraries as opposed to local settings.

Perhaps we could modify set() as per the PR, then add another method that obliterates settings instead? since set() already merges at the top level anyway.

What do you think?

brendon avatar Jul 21 '19 22:07 brendon

It's been a while, so I'm not even sure if this is something I need anymore. Will close it for now.

brendon avatar Aug 30 '24 05:08 brendon