-
-
Notifications
You must be signed in to change notification settings - Fork 594
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 support for providing file upload progress. #373
Add support for providing file upload progress. #373
Conversation
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at [email protected]. Thanks! If you are contributing on behalf of someone else (eg your employer): the individual CLA is not sufficient - use https://developers.facebook.com/opensource/cla?type=company instead. Contact [email protected] if you have any questions. |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
Will this be merged? |
Just wondering when this will be merged? |
Please merge this!! |
@aaron-blondeau-dose Thanks for the PR, could you fix the conflicts so we can get that one in for the next release? |
@aaron-blondeau-dose ping? |
@aaron-blondeau-dose I'm doing some cleanup here, it seems that it diverged a lot since your PR, are you still interested at getting this one in? |
@flovilmart I can help out with this |
Codecov Report
@@ Coverage Diff @@
## master #373 +/- ##
==========================================
- Coverage 85.82% 85.69% -0.14%
==========================================
Files 48 48
Lines 3915 3928 +13
Branches 892 898 +6
==========================================
+ Hits 3360 3366 +6
- Misses 555 562 +7
Continue to review full report at Codecov.
|
@flovilmart @acinader Quick update on this. This will only work in the browser. I haven't tried it on IE8 / IE9. The xmlhttprequest node package seems to be missing the progress event. I would do a PR to add support for it but it seems like that package isn't well maintained. https://github.com/driverdan/node-XMLHttpRequest What do you think? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks good. I think its fine to add for just browser.
src/ParseFile.js
Outdated
* * Valid options are:<ul> | ||
* <li>useMasterKey: In Cloud Code and Node only, causes the Master Key to | ||
* be used for this request. | ||
* <li>progress: Callback for upload progress |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should add a note here that progress is only supported in the browser
@@ -6,17 +6,20 @@ | |||
* LICENSE file in the root directory of this source tree. An additional grant | |||
* of patent rights can be found in the PATENTS file in the same directory. | |||
*/ | |||
|
|||
/* global File */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's this?
@@ -183,6 +186,22 @@ describe('ParseFile', () => { | |||
expect(a.equals(b)).toBe(false); | |||
expect(b.equals(a)).toBe(false); | |||
}); | |||
|
|||
it('reports progress during save when source is a File', () => { | |||
const file = new ParseFile('progress.txt', new File(["Parse"], "progress.txt")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@acinader /* global File */ is for the new File(...)
so that it can find it.
@dplewis var parseFile = new Parse.File(file.name, file); |
@kartheeknagasuri That is correct. This feature is for the browser only. |
@dplewis I am trying through the browser, but not getting the call back, not sure what's the problem |
The progress calback seems to be ran just one time with value 1. |
Doesn't work, Any progress on this ? |
If there is an issue with this feature, please open a new issue (if it doesn't exist already) to report a bug. |
It would be very convenient if developers could get progress when uploading a file with Parse.File. The use case would look something like this:
See also https://www.parse.com/questions/file-upload-progress-javascript-api