Skip to content

Fix File Progress Type #1140

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Apr 12, 2020
Merged

Fix File Progress Type #1140

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
933b797
Update RESTController.js
JeromeDeLeon Apr 7, 2020
0c2aa7e
Update RESTController-test.js
JeromeDeLeon Apr 7, 2020
df47223
Update ParseFile-test.js
JeromeDeLeon Apr 7, 2020
d7233f0
Move type at the end of the param
JeromeDeLeon Apr 9, 2020
5db75e5
Merge branch 'master' into fix-file-progress
JeromeDeLeon Apr 10, 2020
2c60e54
Update progress method's documentation
JeromeDeLeon Apr 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/ParseFile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,17 @@ class ParseFile {
* be used for this request.
* <li>sessionToken: A valid session token, used for making a request on
* behalf of a specific user.
* <li>progress: In Browser only, callback for upload progress
* <li>progress: In Browser only, callback for upload progress. For example:
* <pre>
* let parseFile = new Parse.File(name, file);
* parseFile.save({
* progress: (progressValue, loaded, total, { type }) => {
* if (type === "upload" && progressValue !== null) {
* // Update the UI using progressValue
* }
* }
* });
* </pre>
* </ul>
* @return {Promise} Promise that is resolved when the save finishes.
*/
Expand Down
11 changes: 6 additions & 5 deletions src/RESTController.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,27 +159,28 @@ const RESTController = {
headers[key] = customHeaders[key];
}

function handleProgress(event) {
function handleProgress(type, event) {
if (options && typeof options.progress === 'function') {
if (event.lengthComputable) {
options.progress(
Copy link
Contributor Author

@JeromeDeLeon JeromeDeLeon Apr 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I pushed it at the beginning, or

progress(event.loaded / event.total, {
  type,
  event
});

@dplewis , @ladigitale let me know your thoughts if you have a free time 😄

Copy link
Contributor Author

@JeromeDeLeon JeromeDeLeon Apr 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I do think there is. I was reluctant to just add it and the end, somehow, why not just enclose it in object to allow future values instead of

progress(progressValue, loaded, total, type, ...restArgs)

Copy link

Choose a reason for hiding this comment

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

it would be perfect.
In fact when we've got the original event i think we've got the type.
The targets of each event have a different type who gives the needed imformation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is that so? Have you tried it? haven't looked into values of event tho.

Copy link

Choose a reason for hiding this comment

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

I just logged the target propety one is XMLHttpRequest, and the other is XMLHttpRequestUpload

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Feel free to make suggestion on the PR using the suggestion feature of Github. 😄

event.loaded / event.total,
event.loaded,
event.total
event.total,
{ type }
);
} else {
options.progress(null);
options.progress(null, null, null, { type });
}
}
}

xhr.onprogress = (event) => {
handleProgress(event);
handleProgress('download', event);
};

if (xhr.upload) {
xhr.upload.onprogress = (event) => {
handleProgress(event);
handleProgress('upload', event);
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/__tests__/ParseFile-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ jest.setMock('../LocalDatastore', mockLocalDatastore);
function generateSaveMock(prefix) {
return function(name, payload, options) {
if (options && typeof options.progress === 'function') {
options.progress(0.5, 5, 10);
options.progress(0.5, 5, 10, { type: 'upload' });
}
return Promise.resolve({
name: name,
Expand Down Expand Up @@ -236,7 +236,7 @@ describe('ParseFile', () => {
jest.spyOn(options, 'progress');

return file.save(options).then(function(f) {
expect(options.progress).toHaveBeenCalledWith(0.5, 5, 10);
expect(options.progress).toHaveBeenCalledWith(0.5, 5, 10, { type: 'upload' });
expect(f).toBe(file);
expect(f.name()).toBe('progress.txt');
expect(f.url()).toBe('http://files.parsetfss.com/a/progress.txt');
Expand Down
6 changes: 3 additions & 3 deletions src/__tests__/RESTController-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -444,8 +444,8 @@ describe('RESTController', () => {
jest.spyOn(options, 'progress');

RESTController.ajax('POST', 'files/upload.txt', {}, {}, options).then(({ response, status }) => {
expect(options.progress).toHaveBeenCalledWith(0.5, 5, 10);
expect(options.progress).toHaveBeenCalledTimes(2);
expect(options.progress).toHaveBeenCalledWith(0.5, 5, 10, { type: 'download' });
expect(options.progress).toHaveBeenCalledWith(0.5, 5, 10, { type: 'upload' });
expect(response).toEqual({ success: true });
expect(status).toBe(200);
done();
Expand All @@ -468,7 +468,7 @@ describe('RESTController', () => {
jest.spyOn(options, 'progress');

RESTController.ajax('POST', 'files/upload.txt', {}, {}, options).then(({ response, status }) => {
expect(options.progress).toHaveBeenCalledWith(null);
expect(options.progress).toHaveBeenCalledWith(null, null, null, { type: 'upload' });
expect(response).toEqual({ success: true });
expect(status).toBe(200);
done();
Expand Down