Skip to content
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

Rx.Observable.ajax onProgress only set for Upload #2833

Closed
SntsDev opened this issue Sep 11, 2017 · 3 comments · Fixed by #6001
Closed

Rx.Observable.ajax onProgress only set for Upload #2833

SntsDev opened this issue Sep 11, 2017 · 3 comments · Fixed by #6001
Labels
bug Confirmed bug help wanted Issues we wouldn't mind assistance with.

Comments

@SntsDev
Copy link

SntsDev commented Sep 11, 2017

Hi there,

I hadn't been able to make progressSubscriber.next to fire on a Rx.Observable.ajax instance, when downloading. Checking the code and the log I found that you set onprogress for the upload but you removed the one for download.

As window.XDomainRequest is only available for IE 8 & 9, then on modern browsers onprogress is not assigned.

if (root.XDomainRequest) {
xhr.onprogress = xhrProgress;
} else {
xhr.upload.onprogress = xhrProgress;
}

Did you avoid setting xhr.onprogress = xhrProgress for any specific reason? Or, Should it be there next to xhr.upload.onprogress = xhrProgress ?

Thanks
SntsDev

@benlesh benlesh added help wanted Issues we wouldn't mind assistance with. bug Confirmed bug labels Oct 6, 2017
@PMPepper
Copy link

It seems to me that AjaxObservable needs to make a distinction between upload progress and download progress. You can get progress events by adding the line xhr.onprogress = xhrProgress, but you can't really tell the different between upload progress and download progress.

Would it make sense to split the progressSubscriber property into uploadProgressSubscriber and downloadProgressSubscriber? the existing progressSubscriber property could be deprecated, and left with the current behaviour to maintain compatibility.

@ankitbko
Copy link

It has been more than a year now. What is status of this?

benlesh added a commit to benlesh/rxjs that referenced this issue Feb 7, 2021
- Fixes a bunch of tests that were slightly off or had bad assumptions
- Adds two new features: `includeUploadProgress` and `includeDownloadProgress` that will add additional events to the output stream before the event that is usually emitted.

Resolved ReactiveX#2833
benlesh added a commit to benlesh/rxjs that referenced this issue Feb 7, 2021
- Fixes a bunch of tests that were slightly off or had bad assumptions
- Adds two new features: `includeUploadProgress` and `includeDownloadProgress` that will add additional events to the output stream before the event that is usually emitted.

Resolves ReactiveX#2833
@benlesh
Copy link
Member

benlesh commented Feb 7, 2021

Sorry this took a VERY long time. It wasn't really high-demand, and it took a while to get back to this.

There is now a PR #6001 that adds streaming updates for progress, rather than use the progressSubscriber option. Basically, the observable will emit more than one time, with type information you can check, along with with loaded and total properties.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bug help wanted Issues we wouldn't mind assistance with.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants