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

Add fetch-progress examples to demos page #893

Merged
merged 1 commit into from
Mar 6, 2018
Merged

Add fetch-progress examples to demos page #893

merged 1 commit into from
Mar 6, 2018

Conversation

anthumchris
Copy link
Contributor

Adds link to the "fetch-progress" repository, which provides examples for showing progress indicators with Streams, Fetch, and Service Workers

Adds link to the "fetch-progress" repository, which provides examples for showing progress indicators with Streams, Fetch, and Service Workers
@ricea
Copy link
Collaborator

ricea commented Mar 5, 2018

Looks good.

The examples would be cleaner with TransformStream, but unfortunately no browser is shipping it yet. You could be really concise, something like

fetchEvent.respondWith(new Response(response.body.pipeThrough(new TransformStream({
  transform(chunk, controller) {
    loaded += chunk.length;
    progress({loaded, total});
    controller.enqueue(chunk);
  }
})), response));

(totally untested).

It would be great if you could add web-platform-tests for the incompatibilities between browsers that you found. This would allow the various browsers to track their compatibility issues. I think the issue with abort() not being called when a fetch is cancelled in Chrome is being tracked here: https://bugs.chromium.org/p/chromium/issues/detail?id=638494.

@anthumchris
Copy link
Contributor Author

TransformStream looks great. I made a note to come back to it:
anthumchris/fetch-progress-indicators#8

Haven't used web-platform-tests before, but I'm open to writing some.

@domenic domenic merged commit bb99f6a into whatwg:master Mar 6, 2018
@anthumchris anthumchris deleted the fetch-progress-demo-link branch March 6, 2018 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants