Skip to content
This repository was archived by the owner on Mar 16, 2023. It is now read-only.

Conversation

@YoshiWalsh
Copy link

In EvaporateJs 2.0, the Evaporate.add() function now takes a second argument, overrideConfig. This allows for overriding the global config settings for each request.

This patch allows for specifying a property on AngularEvaporateUpload called overrideConfig. If specified, this will be passed in to Evaporate.add() as a second argument.

I know Angular Evaporate doesn't officially support EvaporateJs 2.0 yet, but I think this patch is a useful workaround until proper support can be added.

In EvaporateJs 2.0, the Evaporate.add() function now takes a second argument, overrideConfig. This allows for overriding the global config settings for each request.

This patch allows for specifying a property on AngularEvaporateUpload called overrideConfig. If specified, this will be passed in to Evaporate.add() as a second argument.

I know Angular Evaporate doesn't officially support EvaporateJs 2.0 yet, but I think this patch is a useful workaround until proper support can be added.
Copy link
Owner

@uqee uqee left a comment

Choose a reason for hiding this comment

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

Thanks for the effort to improve the lib, but I'm pretty positive it would be much better to make a complete support instead of local patches, which will confuse people.

If it helps you somehow, you can still link your own branch in a package.json file until someone (or me) pushes an upgrade. Sooner, rather than later, I hope.

AngularEvaporate.prototype.$start = function (upload) {

var args = Array.prototype.slice.call(arguments);
args[1] = upload.overrideConfig;
Copy link
Owner

Choose a reason for hiding this comment

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

And args being the shallow copy of arguments is not going to be used later, what's the point? Does this work for you?

Copy link
Author

Choose a reason for hiding this comment

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

My bad, I must've screwed up my commit somehow. args is meant to be passed in to this.add, instead of arguments. Ideally I'd like to just set arguments[1] instead of doing a shallow copy, but because arguments is a special type and not actually an array it seems that this doesn't work.

@YoshiWalsh
Copy link
Author

I understand, I rather expected that to be the response but I thought I'd try anyway. I thought this patch might be okay because it's completely backwards compatible, but I totally get what you're saying.

The main reason I tried to do a pull request is that I need this functionality in my own app and I didn't want to have to maintain a fork.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants