Parameterize the cors and proxy options #5
Conversation
…-proxy-url Added proxy config to overcome cors issue for images
…enshot-utils � Conflicts: � dist/index.js � lib/index.ts
There was a problem hiding this comment.
Insatead of just exposing the 2 new options, we should expose an another parameter likecanvasOptions, where users can directly pass the options to html2canvas library. List of Possible options for html2canvas library is available at https://html2canvas.hertzen.com/configuration/. That way it can be easily customizable based on the requirement. Your thoughts?
@sudharsan-selvaraj Sounds good to me. I can update the pull request shortly if you are ok with it. |
|
@saikris12 Sure. Go ahead and update your pull request with the new implementation. |
|
@sudharsan-selvaraj I made the necessary updates. Please review and merge if good. |
There was a problem hiding this comment.
@saikris12 Thanks for the commit. The changes looks great. Just add the options to the constructor and then the commit is good to be merged.
| @@ -68,11 +98,13 @@ class ProtractorScreenShotUtils { | |||
| element:ElementFinder = options.element ? options.element : currentContext.$("body"), | |||
| dimensions = options.dimensions || {}, | |||
| outputPath = options.saveTo || null, | |||
| canvasOptions = options.canvasOptions || {}, | |||
There was a problem hiding this comment.
how about we include the canvasOptions in the constructor and use it as a default option. we can also override the default options when calling the takeScreenshot method?
| * Refer to https://html2canvas.hertzen.com/configuration for usage | ||
| */ | ||
|
|
||
| interface canvasOptions { |
Hi @sudharsan-selvaraj
Thanks for looking into the CORS issue. Since the issue demands different solutions for different scenarios, I think it's better to parameterize the CORS and proxy options instead of hard coding them in the code, so one can use them as per their need. These are not mandatory params, so it's fine even if they are not passed. Please review my pull request and merge if you agree. This is a great little utility and thank you for sharing it here.