knownasilya / ember-plupload

Ember component for handling uploads using plupload
MIT License
87 stars 53 forks source link

More updates and tests #137

Closed corrspt closed 5 years ago

corrspt commented 5 years ago

Hi @knownasilya

I've taken another stab at this, and although I couldn't make all tests pass. I did some more progress on this (and I've updated to Ember/CLI 3.12 and all dependencies are up to date, I think). Tests are currently passing on my machine (but there are several skipped tests and some assertions that are commented out on running tests)

I've had to make a couple of changes to the addFiles helper as removing the Ember.Map and replacing it with a native Map (which doesn't have a "lastObject" property, for instance) changes a few things.

I've also removed some deprecation warning.

I'm going to try and use this PR as-is in my app to see what happens and If I can upgrade to a new ember version after this.

Are you using this addon on Crash?

corrspt commented 5 years ago

Ok, self-note I've tried this in my app and the upload is failing on upload-queue.js. In the filesAdded function around here:

this.pushObject(file); Ember.get(this, 'target').onfileadd(file, { name: Ember.get(this, 'name'), uploader: uploader, queue: this }); Says the target does not have a onfileadd function, but it has the name (a string) of the action I have in the template (instead of being a function/action)

corrspt commented 5 years ago

Ok, so I got the time to look at this again and it appears the error was in my app now. The onfileadd property previously was expecting a string (to use sendAction) but now it expects a proper {{action 'actionName'). Probably needs to go to the changelog.

So for now, this PR allows me to upgrade my app to a new version of ember. I would like to improve it more, but not sure when I'll have the time.

knownasilya commented 5 years ago

All these changes will probably make a major release.

corrspt commented 5 years ago

Hi @knownasilya , with the major release you it be interested to bump the ember version required for this addon (so that internally we can use ES5 getters, instead of this.get)?

I'm not sure when I'll have the time, but I would like to help where possible.

knownasilya commented 5 years ago

totally 👍